fix($animate): use a scheduled timeout in favor of a fallback property to close transitions

With ngAnimate, CSS transitions, that are not properlty triggered, are forceably closed off
by appling a fallback property. The fallback property approach works, however, its styling
itself may effect CSS inheritance or cause the element to render improperly. Therefore, its
best to stick to using a scheduled timeout to run sometime after the highest animation time
has passed.

Closes #5255
Closes #5241
Closes #5405

3 files changed, 128 insertions, 171 deletions

diff --git a/css/angular.css b/css/angular.css
index 3e20a999..b88e61e4 100644
--- a/css/angular.css
+++ b/css/angular.css
@@ -9,14 +9,3 @@
 ng\:form {
   display: block;
 }
-
-/* The styles below ensure that the CSS transition will ALWAYS
- * animate and close. A nasty bug occurs with CSS transitions where
- * when the active class isn't set, or if the active class doesn't
- * contain any styles to transition to, then, if ngAnimate is used,
- * it will appear as if the webpage is broken due to the forever hanging
- * animations. The border-spacing (!ie) and zoom (ie) CSS properties are
- * used below since they trigger a transition without making the browser
- * animate anything and they're both highly underused CSS properties */
-.ng-animate-start { border-spacing:1px 1px; -ms-zoom:1.0001; }
-.ng-animate-active { border-spacing:0px 0px; -ms-zoom:1; }
diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
index aeb6e32e..16d0aa0d 100644
--- a/src/ngAnimate/animate.js
+++ b/src/ngAnimate/animate.js
@@ -881,27 +881,73 @@ angular.module('ngAnimate', ['ng'])
       var ANIMATION_ITERATION_COUNT_KEY = 'IterationCount';
       var NG_ANIMATE_PARENT_KEY = '$$ngAnimateKey';
       var NG_ANIMATE_CSS_DATA_KEY = '$$ngAnimateCSS3Data';
-      var NG_ANIMATE_FALLBACK_CLASS_NAME = 'ng-animate-start';
-      var NG_ANIMATE_FALLBACK_ACTIVE_CLASS_NAME = 'ng-animate-active';
       var ELAPSED_TIME_MAX_DECIMAL_PLACES = 3;
+      var CLOSING_TIME_BUFFER = 1.5;
+      var ONE_SECOND = 1000;
 
+      var animationCounter = 0;
       var lookupCache = {};
       var parentCounter = 0;
+      var animationReflowQueue = [];
+      var animationElementQueue = [];
+      var animationTimer;
+      var closingAnimationTime = 0;
+      var timeOut = false;
+      function afterReflow(element, callback) {
+        $timeout.cancel(animationTimer);
 
-      var animationReflowQueue = [], animationTimer, timeOut = false;
-      function afterReflow(callback) {
         animationReflowQueue.push(callback);
-        $timeout.cancel(animationTimer);
+
+        var node = extractElementNode(element);
+        element = angular.element(node);
+        animationElementQueue.push(element);
+
+        var elementData = element.data(NG_ANIMATE_CSS_DATA_KEY);
+        closingAnimationTime = Math.max(closingAnimationTime,
+          (elementData.maxDelay + elementData.maxDuration) * CLOSING_TIME_BUFFER * ONE_SECOND);
+
+        //by placing a counter we can avoid an accidental
+        //race condition which may close an animation when
+        //a follow-up animation is midway in its animation
+        elementData.animationCount = animationCounter;
+
         animationTimer = $timeout(function() {
           forEach(animationReflowQueue, function(fn) {
             fn();
           });
+
+          //copy the list of elements so that successive
+          //animations won't conflict if they're added before
+          //the closing animation timeout has run
+          var elementQueueSnapshot = [];
+          var animationCounterSnapshot = animationCounter;
+          forEach(animationElementQueue, function(elm) {
+            elementQueueSnapshot.push(elm);
+          });
+
+          $timeout(function() {
+            closeAllAnimations(elementQueueSnapshot, animationCounterSnapshot);
+            elementQueueSnapshot = null;
+          }, closingAnimationTime, false);
+
           animationReflowQueue = [];
+          animationElementQueue = [];
           animationTimer = null;
           lookupCache = {};
+          closingAnimationTime = 0;
+          animationCounter++;
         }, 10, false);
       }
 
+      function closeAllAnimations(elements, count) {
+        forEach(elements, function(element) {
+          var elementData = element.data(NG_ANIMATE_CSS_DATA_KEY);
+          if(elementData && elementData.animationCount == count) {
+            (elementData.closeAnimationFn || noop)();
+          }
+        });
+      }
+
       function getElementAnimationDetails(element, cacheKey) {
         var data = cacheKey ? lookupCache[cacheKey] : null;
         if(!data) {
@@ -1007,6 +1053,7 @@ angular.module('ngAnimate', ['ng'])
            timeout is empty (this would cause a flicker bug normally
            in the page. There is also no point in performing an animation
            that only has a delay and no duration */
+        var maxDelay = Math.max(timings.transitionDelay, timings.animationDelay);
         var maxDuration = Math.max(timings.transitionDuration, timings.animationDuration);
         if(maxDuration === 0) {
           element.removeClass(className);
@@ -1016,13 +1063,9 @@ angular.module('ngAnimate', ['ng'])
         //temporarily disable the transition so that the enter styles
         //don't animate twice (this is here to avoid a bug in Chrome/FF).
         var activeClassName = '';
-        if(timings.transitionDuration > 0) {
-          element.addClass(NG_ANIMATE_FALLBACK_CLASS_NAME);
-          activeClassName += NG_ANIMATE_FALLBACK_ACTIVE_CLASS_NAME + ' ';
-          blockTransitions(element);
-        } else {
+        timings.transitionDuration > 0 ?
+          blockTransitions(element) :
           blockKeyframeAnimations(element);
-        }
 
         forEach(className.split(' '), function(klass, i) {
           activeClassName += (i > 0 ? ' ' : '') + klass + '-active';
@@ -1032,6 +1075,7 @@ angular.module('ngAnimate', ['ng'])
           className : className,
           activeClassName : activeClassName,
           maxDuration : maxDuration,
+          maxDelay : maxDelay,
           classes : className + ' ' + activeClassName,
           timings : timings,
           stagger : stagger,
@@ -1066,30 +1110,28 @@ angular.module('ngAnimate', ['ng'])
       }
 
       function animateRun(element, className, activeAnimationComplete) {
-        var data = element.data(NG_ANIMATE_CSS_DATA_KEY);
+        var elementData = element.data(NG_ANIMATE_CSS_DATA_KEY);
         var node = extractElementNode(element);
-        if(node.className.indexOf(className) == -1 || !data) {
+        if(node.className.indexOf(className) == -1 || !elementData) {
           activeAnimationComplete();
           return;
         }
 
-        var timings = data.timings;
-        var stagger = data.stagger;
-        var maxDuration = data.maxDuration;
-        var activeClassName = data.activeClassName;
-        var maxDelayTime = Math.max(timings.transitionDelay, timings.animationDelay) * 1000;
+        var timings = elementData.timings;
+        var stagger = elementData.stagger;
+        var maxDuration = elementData.maxDuration;
+        var activeClassName = elementData.activeClassName;
+        var maxDelayTime = Math.max(timings.transitionDelay, timings.animationDelay) * ONE_SECOND;
         var startTime = Date.now();
         var css3AnimationEvents = ANIMATIONEND_EVENT + ' ' + TRANSITIONEND_EVENT;
-        var ii = data.ii;
+        var ii = elementData.ii;
 
-        var applyFallbackStyle, style = '', appliedStyles = [];
+        var style = '', appliedStyles = [];
         if(timings.transitionDuration > 0) {
           var propertyStyle = timings.transitionPropertyStyle;
           if(propertyStyle.indexOf('all') == -1) {
-            applyFallbackStyle = true;
-            var fallbackProperty = $sniffer.msie ? '-ms-zoom' : 'border-spacing';
-            style += CSS_PREFIX + 'transition-property: ' + propertyStyle + ', ' + fallbackProperty + '; ';
-            style += CSS_PREFIX + 'transition-duration: ' + timings.transitionDurationStyle + ', ' + timings.transitionDuration + 's; ';
+            style += CSS_PREFIX + 'transition-property: ' + propertyStyle + ';';
+            style += CSS_PREFIX + 'transition-duration: ' + timings.transitionDurationStyle + 's;';
             appliedStyles.push(CSS_PREFIX + 'transition-property');
             appliedStyles.push(CSS_PREFIX + 'transition-duration');
           }
@@ -1098,10 +1140,6 @@ angular.module('ngAnimate', ['ng'])
         if(ii > 0) {
           if(stagger.transitionDelay > 0 && stagger.transitionDuration === 0) {
             var delayStyle = timings.transitionDelayStyle;
-            if(applyFallbackStyle) {
-              delayStyle += ', ' + timings.transitionDelay + 's';
-            }
-
             style += CSS_PREFIX + 'transition-delay: ' +
                      prepareStaggerDelay(delayStyle, stagger.transitionDelay, ii) + '; ';
             appliedStyles.push(CSS_PREFIX + 'transition-delay');
@@ -1124,11 +1162,16 @@ angular.module('ngAnimate', ['ng'])
 
         element.on(css3AnimationEvents, onAnimationProgress);
         element.addClass(activeClassName);
+        elementData.closeAnimationFn = function() {
+          onEnd();
+          activeAnimationComplete();
+        };
+        return onEnd;
 
         // This will automatically be called by $animate so
         // there is no need to attach this internally to the
         // timeout done method.
-        return function onEnd(cancelled) {
+        function onEnd(cancelled) {
           element.off(css3AnimationEvents, onAnimationProgress);
           element.removeClass(activeClassName);
           animateClose(element, className);
@@ -1136,7 +1179,7 @@ angular.module('ngAnimate', ['ng'])
           for (var i in appliedStyles) {
             node.style.removeProperty(appliedStyles[i]);
           }
-        };
+        }
 
         function onAnimationProgress(event) {
           event.stopPropagation();
@@ -1202,7 +1245,7 @@ angular.module('ngAnimate', ['ng'])
         //data from the element which will not make the 2nd animation
         //happen in the first place
         var cancel = preReflowCancellation;
-        afterReflow(function() {
+        afterReflow(element, function() {
           unblockTransitions(element);
           unblockKeyframeAnimations(element);
           //once the reflow is complete then we point cancel to
@@ -1218,7 +1261,6 @@ angular.module('ngAnimate', ['ng'])
 
       function animateClose(element, className) {
         element.removeClass(className);
-        element.removeClass(NG_ANIMATE_FALLBACK_CLASS_NAME);
         element.removeData(NG_ANIMATE_CSS_DATA_KEY);
       }
 
@@ -1268,7 +1310,7 @@ angular.module('ngAnimate', ['ng'])
         beforeAddClass : function(element, className, animationCompleted) {
           var cancellationMethod = animateBefore(element, suffixClasses(className, '-add'));
           if(cancellationMethod) {
-            afterReflow(function() {
+            afterReflow(element, function() {
               unblockTransitions(element);
               unblockKeyframeAnimations(element);
               animationCompleted();
@@ -1285,7 +1327,7 @@ angular.module('ngAnimate', ['ng'])
         beforeRemoveClass : function(element, className, animationCompleted) {
           var cancellationMethod = animateBefore(element, suffixClasses(className, '-remove'));
           if(cancellationMethod) {
-            afterReflow(function() {
+            afterReflow(element, function() {
               unblockTransitions(element);
               unblockKeyframeAnimations(element);
               animationCompleted();
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
index 01589da1..705004d4 100644
--- a/test/ngAnimate/animateSpec.js
+++ b/test/ngAnimate/animateSpec.js
@@ -645,30 +645,6 @@ describe("ngAnimate", function() {
             expect(element).toBeShown();
           }));
 
-          it("should fallback to the animation duration if an infinite iteration is provided",
-            inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
-
-            var style = '-webkit-animation-duration: 2s;' +
-                        '-webkit-animation-iteration-count: infinite;' +
-                                'animation-duration: 2s;' +
-                                'animation-iteration-count: infinite;';
-
-            ss.addRule('.ng-hide-add', style);
-            ss.addRule('.ng-hide-remove', style);
-
-            element = $compile(html('<div>1</div>'))($rootScope);
-
-            element.addClass('ng-hide');
-            expect(element).toBeHidden();
-
-            $animate.removeClass(element, 'ng-hide');
-            if ($sniffer.animations) {
-              $timeout.flush();
-              browserTrigger(element,'animationend', { timeStamp: Date.now() + 2000, elapsedTime: 2 });
-            }
-            expect(element).toBeShown();
-          }));
-
           it("should not consider the animation delay is provided",
             inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
 
@@ -838,109 +814,10 @@ describe("ngAnimate", function() {
             expect(elements[2].attr('style')).toMatch(/animation-delay: 1\.2\d*s,\s*2\.2\d*s/);
             expect(elements[3].attr('style')).toMatch(/animation-delay: 1\.3\d*s,\s*2\.3\d*s/);
           }));
+
         });
 
         describe("Transitions", function() {
-          it("should only apply the fallback transition property unless all properties are being animated",
-            inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
-
-            if (!$sniffer.animations) return;
-
-            ss.addRule('.all.ng-enter',  '-webkit-transition:1s linear all;' +
-                                                 'transition:1s linear all');
-
-            ss.addRule('.one.ng-enter',  '-webkit-transition:1s linear color;' +
-                                                 'transition:1s linear color');
-
-            var element = $compile('<div></div>')($rootScope);
-            var child = $compile('<div class="all">...</div>')($rootScope);
-            $rootElement.append(element);
-            var body = jqLite($document[0].body);
-            body.append($rootElement);
-
-            $animate.enter(child, element);
-            $rootScope.$digest();
-            $timeout.flush();
-
-            expect(child.attr('style') || '').not.toContain('transition-property');
-            expect(child.hasClass('ng-animate-start')).toBe(true);
-            expect(child.hasClass('ng-animate-active')).toBe(true);
-
-            browserTrigger(child,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 });
-            $timeout.flush();
-
-            expect(child.hasClass('ng-animate')).toBe(false);
-            expect(child.hasClass('ng-animate-active')).toBe(false);
-
-            child.remove();
-
-            var child2 = $compile('<div class="one">...</div>')($rootScope);
-
-            $animate.enter(child2, element);
-            $rootScope.$digest();
-            $timeout.flush();
-
-            //IE removes the -ms- prefix when placed on the style
-            var fallbackProperty = $sniffer.msie ? 'zoom' : 'border-spacing';
-            var regExp = new RegExp("transition-property:\\s+color\\s*,\\s*" + fallbackProperty + "\\s*;");
-            expect(child2.attr('style') || '').toMatch(regExp);
-            expect(child2.hasClass('ng-animate')).toBe(true);
-            expect(child2.hasClass('ng-animate-active')).toBe(true);
-
-            browserTrigger(child2,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1000 });
-            $timeout.flush();
-
-            expect(child2.hasClass('ng-animate')).toBe(false);
-            expect(child2.hasClass('ng-animate-active')).toBe(false);
-          }));
-
-          it("should not apply the fallback classes if no animations are going on or if CSS animations are going on",
-            inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
-
-            if (!$sniffer.animations) return;
-
-            ss.addRule('.transitions',  '-webkit-transition:1s linear all;' +
-                                                'transition:1s linear all');
-
-            ss.addRule('.keyframes',  '-webkit-animation:my_animation 1s;' +
-                                              'animation:my_animation 1s');
-
-            var element = $compile('<div class="transitions">...</div>')($rootScope);
-            $rootElement.append(element);
-            jqLite($document[0].body).append($rootElement);
-
-            $animate.enabled(false);
-
-            $animate.addClass(element, 'klass');
-
-            expect(element.hasClass('ng-animate-start')).toBe(false);
-
-            element.removeClass('klass');
-
-            $animate.enabled(true);
-
-            $animate.addClass(element, 'klass');
-
-            $timeout.flush();
-
-            expect(element.hasClass('ng-animate-start')).toBe(true);
-            expect(element.hasClass('ng-animate-active')).toBe(true);
-
-            browserTrigger(element,'transitionend', { timeStamp: Date.now() + 1000, elapsedTime: 1 });
-
-            expect(element.hasClass('ng-animate-start')).toBe(false);
-            expect(element.hasClass('ng-animate-active')).toBe(false);
-
-            element.attr('class', 'keyframes');
-
-            $animate.addClass(element, 'klass2');
-
-            $timeout.flush();
-
-            expect(element.hasClass('ng-animate-start')).toBe(false);
-            expect(element.hasClass('ng-animate-active')).toBe(false);
-          }));
-
           it("should skip transitions if disabled and run when enabled",
             inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
 
@@ -1083,9 +960,9 @@ describe("ngAnimate", function() {
               }
               expect(element).toBeShown();
               if ($sniffer.transitions) {
-                expect(element.hasClass('ng-animate-active')).toBe(true);
+                expect(element.hasClass('ng-hide-remove-active')).toBe(true);
                 browserTrigger(element,'animationend', { timeStamp: Date.now() + 11000, elapsedTime: 11 });
-                expect(element.hasClass('ng-animate-active')).toBe(false);
+                expect(element.hasClass('ng-hide-remove-active')).toBe(false);
               }
           }));
 
@@ -1214,6 +1091,55 @@ describe("ngAnimate", function() {
             expect(elements[2].attr('style')).toMatch(/transition-delay: 2\.2\d*s,\s*4\.2\d*s/);
             expect(elements[3].attr('style')).toMatch(/transition-delay: 2\.3\d*s,\s*4\.3\d*s/);
           }));
+
+          it("apply a closing timeout to close all pending transitions",
+            inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
+
+            if (!$sniffer.transitions) return;
+
+            ss.addRule('.animated-element', '-webkit-transition:5s linear all;' +
+                                                    'transition:5s linear all;');
+
+            element = $compile(html('<div class="animated-element">foo</div>'))($rootScope);
+
+            $animate.addClass(element, 'some-class');
+
+            $timeout.flush(10); //reflow
+            expect(element.hasClass('some-class-add-active')).toBe(true);
+
+            $timeout.flush(7500); //closing timeout
+            expect(element.hasClass('some-class-add-active')).toBe(false);
+          }));
+
+          it("should not allow the closing animation to close off a successive animation midway",
+            inject(function($animate, $rootScope, $compile, $sniffer, $timeout) {
+          
+            if (!$sniffer.transitions) return;
+
+            ss.addRule('.some-class-add', '-webkit-transition:5s linear all;' +
+                                                  'transition:5s linear all;');
+            ss.addRule('.some-class-remove', '-webkit-transition:10s linear all;' +
+                                                     'transition:10s linear all;');
+
+            element = $compile(html('<div>foo</div>'))($rootScope);
+
+            $animate.addClass(element, 'some-class');
+
+            $timeout.flush(10); //reflow
+            expect(element.hasClass('some-class-add-active')).toBe(true);
+
+            $animate.removeClass(element, 'some-class');
+
+            $timeout.flush(10); //second reflow
+
+            $timeout.flush(7500); //closing timeout for the first animation
+            expect(element.hasClass('some-class-remove-active')).toBe(true);
+
+            $timeout.flush(15000); //closing timeout for the second animation
+            expect(element.hasClass('some-class-remove-active')).toBe(false);
+
+            $timeout.verifyNoPendingTasks();
+          }));
         });
 
         it("should apply staggering to both transitions and keyframe animations when used within the same animation",