fix($animateCss): only (de)register listeners when events have been added

Narretz · Narretz · commit 959f2bbb2d12 · 2016-01-04T21:01:45.000+01:00
Previously, when an animation was closed because no animation styles where found, it would call .off() with an empty string as the argument. For both jquery/jqlite this is the same as calling .off() without any argument, which deregisters all event listeners on an element. Closes angular#13514
diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js
@@ -749,8 +749,8 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
           options.onDone();
         }
 
-        // Remove the transitionend / animationend listener(s)
-        if (events) {
+        if (events && events.length) {
+          // Remove the transitionend / animationend listener(s)
           element.off(events.join(' '), onAnimationProgress);
         }
 
@@ -960,7 +960,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
             element.data(ANIMATE_TIMER_KEY, animationsData);
           }
 
-          element.on(events.join(' '), onAnimationProgress);
+          if (events.length) {
+            element.on(events.join(' '), onAnimationProgress);
+          }
+
           if (options.to) {
             if (options.cleanupStyles) {
               registerRestorableStyles(restoreStyles, node, Object.keys(options.to));
diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js
@@ -1502,6 +1502,39 @@ describe("ngAnimate $animateCss", function() {
               expect(elementOffSpy.mostRecentCall.args[0]).toBe(event);
             });
         });
+
+        they("should not add or remove $prop event listeners when no animation styles are detected",
+          [TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
+            inject(function($animateCss, $timeout) {
+
+              progress = event === TRANSITIONEND_EVENT ? transitionProgress : keyframeProgress;
+
+              // Make sure other event listeners are not affected
+              var otherEndSpy = jasmine.createSpy('otherEndSpy');
+              element.on(event, otherEndSpy);
+
+              expect(elementOnSpy).toHaveBeenCalledOnce();
+              elementOnSpy.reset();
+
+              var animator = $animateCss(element, {
+                event: 'enter',
+                structural: true
+              });
+
+              expect(animator.$$willAnimate).toBeFalsy();
+
+              // This will close the animation because no styles have been detected
+              var runner = animator.start();
+              triggerAnimationStartFrame();
+
+              expect(elementOnSpy).not.toHaveBeenCalled();
+              expect(elementOffSpy).not.toHaveBeenCalled();
+
+              progress(element, 10);
+              expect(otherEndSpy).toHaveBeenCalledOnce();
+            });
+        });
+
       });
     });
 

Original file line number	Diff line number	Diff line change
`@@ -749,8 +749,8 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {`
`749`	`749`	`options.onDone();`
`750`	`750`	`}`
`751`	`751`
`752`		`- // Remove the transitionend / animationend listener(s)`
`753`		`- if (events) {`
	`752`	`+ if (events && events.length) {`
	`753`	`+ // Remove the transitionend / animationend listener(s)`
`754`	`754`	`element.off(events.join(' '), onAnimationProgress);`
`755`	`755`	`}`
`756`	`756`
`@@ -960,7 +960,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {`
`960`	`960`	`element.data(ANIMATE_TIMER_KEY, animationsData);`
`961`	`961`	`}`
`962`	`962`
`963`		`- element.on(events.join(' '), onAnimationProgress);`
	`963`	`+ if (events.length) {`
	`964`	`+ element.on(events.join(' '), onAnimationProgress);`
	`965`	`+ }`
	`966`	`+`
`964`	`967`	`if (options.to) {`
`965`	`968`	`if (options.cleanupStyles) {`
`966`	`969`	`registerRestorableStyles(restoreStyles, node, Object.keys(options.to));`