fix(ngAnimate.$animate): remove animation callbacks when the element is removed

Narretz · Narretz · commit ce7f400011e1 · 2016-03-18T17:10:26.000+01:00
The test for this didn't actually test the listener removal. The addClass animation after
the element removal didn't start because the enter animation was still in progress.
diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js
@@ -194,14 +194,19 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
       return matches;
     }
 
-    return {
+    var $animate = {
       on: function(event, container, callback) {
         var node = extractElementNode(container);
         callbackRegistry[event] = callbackRegistry[event] || [];
         callbackRegistry[event].push({
           node: node,
           callback: callback
         });
+
+        // Remove the callback when the element is removed from the DOM
+        jqLite(container).on('$destroy', function() {
+          $animate.off(event, container, callback);
+        });
       },
 
       off: function(event, container, callback) {
@@ -269,6 +274,8 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
       }
     };
 
+    return $animate;
+
     function queueAnimation(element, event, initialOptions) {
       // we always make a copy of the options since
       // there should never be any side effects on
diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -1912,33 +1912,62 @@ describe("animations", function() {
       expect(capturedElement).toBe(element);
     }));
 
-    it('should remove the event listener if the element is removed',
+    it('should remove all event listeners when the element is removed',
       inject(function($animate, $rootScope, $rootElement) {
 
       element = jqLite('<div></div>');
 
       var count = 0;
+      var runner;
+
       $animate.on('enter', element, counter);
-      $animate.on('addClass', element, counter);
+      $animate.on('addClass', element[0], counter);
 
       function counter(element, phase) {
         if (phase === 'start') {
           count++;
         }
       }
 
-      $animate.enter(element, $rootElement);
+      runner = $animate.enter(element, $rootElement);
       $rootScope.$digest();
-      $animate.flush();
+      expect(capturedAnimation).toBeTruthy();
 
+      $animate.flush();
+      runner.end(); // Otherwise the class animation won't run because enter is still in progress
       expect(count).toBe(1);
+
+      capturedAnimation = null;
+
+      $animate.addClass(element, 'blue');
+      $rootScope.$digest();
+      expect(capturedAnimation).toBeTruthy();
+
+      $animate.flush();
+      expect(count).toBe(2);
+
+      capturedAnimation = null;
+
       element.remove();
 
-      $animate.addClass(element, 'viljami');
+      runner = $animate.enter(element, $rootElement);
       $rootScope.$digest();
 
+      expect(capturedAnimation).toBeTruthy();
+
       $animate.flush();
-      expect(count).toBe(1);
+      runner.end(); // Otherwise the class animation won't run because enter is still in progress
+
+      expect(count).toBe(2);
+
+      capturedAnimation = null;
+
+      $animate.addClass(element, 'red');
+      $rootScope.$digest();
+      expect(capturedAnimation).toBeTruthy();
+
+      $animate.flush();
+      expect(count).toBe(2);
     }));
 
     it('should always detect registered callbacks after one postDigest has fired',
@@ -2116,7 +2145,6 @@ describe("animations", function() {
 
           var callbackTriggered = false;
 
-
           $animate.on($event, $document[0], function() {
             callbackTriggered = true;
           });
