revert: fix($animateCss): remove animation end event listeners on close

src/ng/directive/select.js

@@ -154,7 +154,7 @@ var SelectController =
  *
  * The `select` directive is used together with {@link ngModel `ngModel`} to provide data-binding
  * between the scope and the `<select>` control (including setting default values).
- * Ìt also handles dynamic `<option>` elements, which can be added using the {@link ngRepeat `ngRepeat}` or
+ * It also handles dynamic `<option>` elements, which can be added using the {@link ngRepeat `ngRepeat}` or
  * {@link ngOptions `ngOptions`} directives.
  *
  * When an item in the `<select>` menu is selected, the value of the selected option will be bound

src/ngAnimate/animateCss.js

@@ -474,8 +474,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
       var maxDelayTime;
       var maxDuration;
       var maxDurationTime;
-      var startTime;
-      var events = [];
 
       if (options.duration === 0 || (!$sniffer.animations && !$sniffer.transitions)) {
         return closeAndReturnNoopAnimator();
@@ -749,11 +747,6 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
           options.onDone();
         }
 
-        // Remove the transitionend / animationend listener(s)
-        if (events) {
-          element.off(events.join(' '), onAnimationProgress);
-        }
-
         // if the preparation function fails then the promise is not setup
         if (runner) {
           runner.complete(!rejected);
@@ -789,37 +782,15 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
         };
       }
 
-      function onAnimationProgress(event) {
-        event.stopPropagation();
-        var ev = event.originalEvent || event;
-        var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();
-
-        /* Firefox (or possibly just Gecko) likes to not round values up
-         * when a ms measurement is used for the animation */
-        var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));
-
-        /* $manualTimeStamp is a mocked timeStamp value which is set
-         * within browserTrigger(). This is only here so that tests can
-         * mock animations properly. Real events fallback to event.timeStamp,
-         * or, if they don't, then a timeStamp is automatically created for them.
-         * We're checking to see if the timeStamp surpasses the expected delay,
-         * but we're using elapsedTime instead of the timeStamp on the 2nd
-         * pre-condition since animationPauseds sometimes close off early */
-        if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
-          // we set this flag to ensure that if the transition is paused then, when resumed,
-          // the animation will automatically close itself since transitions cannot be paused.
-          animationCompleted = true;
-          close();
-        }
-      }
-
       function start() {
         if (animationClosed) return;
         if (!node.parentNode) {
           close();
           return;
         }
 
+        var startTime, events = [];
+
         // even though we only pause keyframe animations here the pause flag
         // will still happen when transitions are used. Only the transition will
         // not be paused since that is not possible. If the animation ends when
@@ -982,6 +953,30 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
             element.removeData(ANIMATE_TIMER_KEY);
           }
         }
+
+        function onAnimationProgress(event) {
+          event.stopPropagation();
+          var ev = event.originalEvent || event;
+          var timeStamp = ev.$manualTimeStamp || ev.timeStamp || Date.now();
+
+          /* Firefox (or possibly just Gecko) likes to not round values up
+           * when a ms measurement is used for the animation */
+          var elapsedTime = parseFloat(ev.elapsedTime.toFixed(ELAPSED_TIME_MAX_DECIMAL_PLACES));
+
+          /* $manualTimeStamp is a mocked timeStamp value which is set
+           * within browserTrigger(). This is only here so that tests can
+           * mock animations properly. Real events fallback to event.timeStamp,
+           * or, if they don't, then a timeStamp is automatically created for them.
+           * We're checking to see if the timeStamp surpasses the expected delay,
+           * but we're using elapsedTime instead of the timeStamp on the 2nd
+           * pre-condition since animations sometimes close off early */
+          if (Math.max(timeStamp - startTime, 0) >= maxDelayTime && elapsedTime >= maxDuration) {
+            // we set this flag to ensure that if the transition is paused then, when resumed,
+            // the animation will automatically close itself since transitions cannot be paused.
+            animationCompleted = true;
+            close();
+          }
+        }
       }
     };
   }];

src/ngAnimate/animateQueue.js

@@ -586,6 +586,7 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
             parentHost = parentElement.data(NG_ANIMATE_PIN_DATA);
             if (parentHost) {
               parentElement = parentHost;
+              rootElementDetected = true;
             }
           }
         }

test/ng/directive/ngOptionsSpec.js

@@ -1,6 +1,6 @@
 'use strict';
 
-ddescribe('ngOptions', function() {
+describe('ngOptions', function() {
 
   var scope, formElement, element, $compile, linkLog;
 

test/ngAnimate/.jshintrc

@@ -8,10 +8,6 @@
     "applyAnimationStyles": false,
     "applyAnimationFromStyles": false,
     "applyAnimationToStyles": false,
-    "applyAnimationClassesFactory": false,
-    "TRANSITIONEND_EVENT": false,
-    "TRANSITION_PROP": false,
-    "ANIMATION_PROP": false,
-    "ANIMATIONEND_EVENT": false
+    "applyAnimationClassesFactory": false
   }
 }

test/ngAnimate/animateCssSpec.js

@@ -12,16 +12,6 @@ describe("ngAnimate $animateCss", function() {
         : expect(className).not.toMatch(regex);
   }
 
-  function keyframeProgress(element, duration, delay) {
-    browserTrigger(element, 'animationend',
-      { timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
-  }
-
-  function transitionProgress(element, duration, delay) {
-    browserTrigger(element, 'transitionend',
-      { timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
-  }
-
   var fakeStyle = {
     color: 'blue'
   };
@@ -365,6 +355,16 @@ describe("ngAnimate $animateCss", function() {
           assert.toHaveClass('ng-enter-active');
         }
 
+        function keyframeProgress(element, duration, delay) {
+          browserTrigger(element, 'animationend',
+            { timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
+        }
+
+        function transitionProgress(element, duration, delay) {
+          browserTrigger(element, 'transitionend',
+            { timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
+        }
+
         beforeEach(inject(function($rootElement, $document) {
           element = jqLite('<div></div>');
           $rootElement.append(element);
@@ -1404,105 +1404,6 @@ describe("ngAnimate $animateCss", function() {
           expect(count.stagger).toBe(2);
         }));
       });
-
-      describe('transitionend/animationend event listeners', function() {
-        var element, elementOnSpy, elementOffSpy, progress;
-
-        function setStyles(event) {
-          switch (event) {
-            case TRANSITIONEND_EVENT:
-              ss.addRule('.ng-enter', 'transition: 10s linear all;');
-              progress = transitionProgress;
-              break;
-            case ANIMATIONEND_EVENT:
-              ss.addRule('.ng-enter', '-webkit-animation: animation 10s;' +
-                                              'animation: animation 10s;');
-              progress = keyframeProgress;
-              break;
-          }
-        }
-
-        beforeEach(inject(function($rootElement, $document) {
-          element = jqLite('<div></div>');
-          $rootElement.append(element);
-          jqLite($document[0].body).append($rootElement);
-
-          elementOnSpy = spyOn(element, 'on').andCallThrough();
-          elementOffSpy = spyOn(element, 'off').andCallThrough();
-        }));
-
-        they('should remove the $prop event listeners on cancel',
-          [TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
-            inject(function($animateCss) {
-
-              setStyles(event);
-
-              var animator = $animateCss(element, {
-                event: 'enter',
-                structural: true
-              });
-
-              var runner = animator.start();
-              triggerAnimationStartFrame();
-
-              expect(elementOnSpy).toHaveBeenCalledOnce();
-              expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
-
-              runner.cancel();
-
-              expect(elementOffSpy).toHaveBeenCalledOnce();
-              expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
-            });
-        });
-
-        they("should remove the $prop event listener when the animation is closed",
-          [TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
-            inject(function($animateCss) {
-
-              setStyles(event);
-
-              var animator = $animateCss(element, {
-                event: 'enter',
-                structural: true
-              });
-
-              var runner = animator.start();
-              triggerAnimationStartFrame();
-
-              expect(elementOnSpy).toHaveBeenCalledOnce();
-              expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
-
-              progress(element, 10);
-
-              expect(elementOffSpy).toHaveBeenCalledOnce();
-              expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
-            });
-        });
-
-        they("should remove the $prop event listener when the closing timeout occurs",
-          [TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
-            inject(function($animateCss, $timeout) {
-
-              setStyles(event);
-
-              var animator = $animateCss(element, {
-                event: 'enter',
-                structural: true
-              });
-
-              animator.start();
-              triggerAnimationStartFrame();
-
-              expect(elementOnSpy).toHaveBeenCalledOnce();
-              expect(elementOnSpy.mostRecentCall.args[0]).toBe(event);
-
-              $timeout.flush(15000);
-
-              expect(elementOffSpy).toHaveBeenCalledOnce();
-              expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
-            });
-        });
-      });
     });
 
     it('should avoid applying the same cache to an element a follow-up animation is run on the same element',

test/ngAnimate/animateSpec.js

@@ -1448,28 +1448,46 @@ describe("animations", function() {
     }));
 
 
-    it('should allow an element to be pinned elsewhere and still be available in animations',
-      inject(function($animate, $compile, $document, $rootElement, $rootScope) {
+    they('should animate an element inside a pinned element that is the $prop element',
+      ['same', 'parent', 'grandparent'],
+      function(elementRelation) {
+        inject(function($animate, $compile, $document, $rootElement, $rootScope) {
 
-      var innerParent = jqLite('<div></div>');
-      jqLite($document[0].body).append(innerParent);
-      innerParent.append($rootElement);
+        var pinElement, animateElement;
 
-      var element = jqLite('<div></div>');
-      jqLite($document[0].body).append(element);
+        var innerParent = jqLite('<div></div>');
+        jqLite($document[0].body).append(innerParent);
+        innerParent.append($rootElement);
 
-      $animate.addClass(element, 'red');
-      $rootScope.$digest();
-      expect(capturedAnimation).toBeFalsy();
+        switch (elementRelation) {
+          case 'same':
+            pinElement = jqLite('<div id="animate"></div>');
+            break;
+          case 'parent':
+            pinElement = jqLite('<div><div id="animate"></div></div>');
+            break;
+          case 'grandparent':
+            pinElement = jqLite('<div><div><div id="animate"></div></div></div>');
+            break;
+        }
 
-      $animate.pin(element, $rootElement);
+        jqLite($document[0].body).append(pinElement);
+        animateElement = jqLite($document[0].getElementById('animate'));
 
-      $animate.addClass(element, 'blue');
-      $rootScope.$digest();
-      expect(capturedAnimation).toBeTruthy();
+        $animate.addClass(animateElement, 'red');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeFalsy();
 
-      dealoc(element);
-    }));
+        // Pin the element to the app root to enable animations
+        $animate.pin(pinElement, $rootElement);
+
+        $animate.addClass(animateElement, 'blue');
+        $rootScope.$digest();
+        expect(capturedAnimation).toBeTruthy();
+
+        dealoc(pinElement);
+      });
+    });
 
     it('should adhere to the disabled state of the hosted parent when an element is pinned',
       inject(function($animate, $compile, $document, $rootElement, $rootScope) {