fix($animate): prevent race conditions for class-based animations when animating on the same CSS class

Closes #5588

2 files changed, 70 insertions, 5 deletions

diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
index 26fe982d..f3a86f82 100644
--- a/src/ngAnimate/animate.js
+++ b/src/ngAnimate/animate.js
@@ -659,12 +659,23 @@ angular.module('ngAnimate', ['ng'])
           cleanup(element);
           cancelAnimations(ngAnimateState.animations);
 
+          //in the event that the CSS is class is quickly added and removed back
+          //then we don't want to wait until after the reflow to add/remove the CSS
+          //class since both class animations may run into a race condition.
+          //The code below will check to see if that is occurring and will
+          //immediately remove the former class before the reflow so that the
+          //animation can snap back to the original animation smoothly
+          var isFullyClassBasedAnimation = isClassBased && !ngAnimateState.structural;
+          var isRevertingClassAnimation = isFullyClassBasedAnimation &&
+                                          ngAnimateState.className == className &&
+                                          animationEvent != ngAnimateState.event;
+
           //if the class is removed during the reflow then it will revert the styles temporarily
           //back to the base class CSS styling causing a jump-like effect to occur. This check
           //here ensures that the domOperation is only performed after the reflow has commenced
-          if(ngAnimateState.beforeComplete) {
+          if(ngAnimateState.beforeComplete || isRevertingClassAnimation) {
             (ngAnimateState.done || noop)(true);
-          } else if(isClassBased && !ngAnimateState.structural) {
+          } else if(isFullyClassBasedAnimation) {
             //class-based animations will compare element className values after cancelling the
             //previous animation to see if the element properties already contain the final CSS
             //class and if so then the animation will be skipped. Since the domOperation will
@@ -812,10 +823,10 @@ angular.module('ngAnimate', ['ng'])
       function cancelAnimations(animations) {
         var isCancelledFlag = true;
         forEach(animations, function(animation) {
-          if(!animations.beforeComplete) {
+          if(!animation.beforeComplete) {
             (animation.beforeEnd || noop)(isCancelledFlag);
           }
-          if(!animations.afterComplete) {
+          if(!animation.afterComplete) {
             (animation.afterEnd || noop)(isCancelledFlag);
           }
         });
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
index 99527cc4..e74049be 100644
--- a/test/ngAnimate/animateSpec.js
+++ b/test/ngAnimate/animateSpec.js
@@ -2673,10 +2673,16 @@ describe("ngAnimate", function() {
             beforeAddClass : function(element, className, done) {
               currentAnimation = 'addClass';
               currentFn = done;
+              return function(cancelled) {
+                currentAnimation = cancelled ? null : currentAnimation;
+              }
             },
             beforeRemoveClass : function(element, className, done) {
               currentAnimation = 'removeClass';
               currentFn = done;
+              return function(cancelled) {
+                currentAnimation = cancelled ? null : currentAnimation;
+              }
             }
           };
         });
@@ -2690,10 +2696,12 @@ describe("ngAnimate", function() {
         expect(currentAnimation).toBe('addClass');
         currentFn();
 
+        currentAnimation = null;
+
         $animate.removeClass(element, 'on');
         $animate.addClass(element, 'on');
 
-        expect(currentAnimation).toBe('addClass');
+        expect(currentAnimation).toBe(null);
       });
     });
 
@@ -3113,5 +3121,51 @@ describe("ngAnimate", function() {
       $timeout.flush(1);
       expect(ready).toBe(true);
     }));
+
+    it('should avoid skip animations if the same CSS class is added / removed synchronously before the reflow kicks in',
+      inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {
+
+      if (!$sniffer.transitions) return;
+
+      ss.addRule('.water-class', '-webkit-transition:2s linear all;' +
+                                         'transition:2s linear all;');
+
+      $animate.enabled(true);
+
+      var element = $compile('<div class="water-class on"></div>')($rootScope);
+      $rootElement.append(element);
+      jqLite($document[0].body).append($rootElement);
+
+      var signature = '';
+      $animate.removeClass(element, 'on', function() {
+        signature += 'A';
+      });
+      $animate.addClass(element, 'on', function() {
+        signature += 'B';
+      });
+
+      $timeout.flush(1);
+      expect(signature).toBe('AB');
+
+      signature = '';
+      $animate.removeClass(element, 'on', function() {
+        signature += 'A';
+      });
+      $animate.addClass(element, 'on', function() {
+        signature += 'B';
+      });
+      $animate.removeClass(element, 'on', function() {
+        signature += 'C';
+      });
+
+      $timeout.flush(1);
+      expect(signature).toBe('AB');
+
+      $timeout.flush(10);
+      browserTrigger(element, 'transitionend', { timeStamp: Date.now(), elapsedTime: 2000 });
+      $timeout.flush(1);
+
+      expect(signature).toBe('ABC');
+    }));
   });
 });