Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 7b50582

Browse files
committed
fix($animateCss): remove animation end event listeners on close
Previously the transition/animation end events were not removed when the animation was closed. This normally didn't matter, because the close function knows the animation are closed and won't do work twice. However, the listeners themselves do computation that could fail when the event was missing some data, for example when the event was triggered instead of natural. Closes #10387
1 parent f50b0cb commit 7b50582

File tree

3 files changed

+120
-11
lines changed

3 files changed

+120
-11
lines changed

src/ngAnimate/animateCss.js

+6
Original file line numberDiff line numberDiff line change
@@ -950,6 +950,7 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
950950
for (var i = 1; i < animationsData.length; i++) {
951951
animationsData[i]();
952952
}
953+
removeEventListeners();
953954
element.removeData(ANIMATE_TIMER_KEY);
954955
}
955956
}
@@ -975,8 +976,13 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
975976
// the animation will automatically close itself since transitions cannot be paused.
976977
animationCompleted = true;
977978
close();
979+
removeEventListeners();
978980
}
979981
}
982+
983+
function removeEventListeners() {
984+
element.off(events.join(' '), onAnimationProgress);
985+
}
980986
}
981987
};
982988
}];

test/ngAnimate/.jshintrc

+5-1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
"applyAnimationStyles": false,
99
"applyAnimationFromStyles": false,
1010
"applyAnimationToStyles": false,
11-
"applyAnimationClassesFactory": false
11+
"applyAnimationClassesFactory": false,
12+
"TRANSITIONEND_EVENT": false,
13+
"TRANSITION_PROP": false,
14+
"ANIMATION_PROP": false,
15+
"ANIMATIONEND_EVENT": false,
1216
}
1317
}

test/ngAnimate/animateCssSpec.js

+109-10
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,16 @@ describe("ngAnimate $animateCss", function() {
1212
: expect(className).not.toMatch(regex);
1313
}
1414

15+
function keyframeProgress(element, duration, delay) {
16+
browserTrigger(element, 'animationend',
17+
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
18+
}
19+
20+
function transitionProgress(element, duration, delay) {
21+
browserTrigger(element, 'transitionend',
22+
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
23+
}
24+
1525
var fakeStyle = {
1626
color: 'blue'
1727
};
@@ -355,16 +365,6 @@ describe("ngAnimate $animateCss", function() {
355365
assert.toHaveClass('ng-enter-active');
356366
}
357367

358-
function keyframeProgress(element, duration, delay) {
359-
browserTrigger(element, 'animationend',
360-
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
361-
}
362-
363-
function transitionProgress(element, duration, delay) {
364-
browserTrigger(element, 'transitionend',
365-
{ timeStamp: Date.now() + ((delay || 1) * 1000), elapsedTime: duration });
366-
}
367-
368368
beforeEach(inject(function($rootElement, $document) {
369369
element = jqLite('<div></div>');
370370
$rootElement.append(element);
@@ -463,6 +463,62 @@ describe("ngAnimate $animateCss", function() {
463463
});
464464
});
465465

466+
they("should remove the $prop event listener when the animation is closed",
467+
[TRANSITIONEND_EVENT, ANIMATIONEND_EVENT], function(event) {
468+
469+
inject(function($animateCss, $rootScope) {
470+
471+
var callLog = {},
472+
origOn = element.on,
473+
origOff = element.off,
474+
origListener,
475+
progress;
476+
477+
callLog[event] = 0;
478+
479+
var proxyListener = function() {
480+
callLog[event]++;
481+
origListener.apply(this, arguments);
482+
};
483+
484+
// Overwrite the .on function to log calls to the event listeners
485+
element.on = function() {
486+
origListener = arguments[1];
487+
origOn.call(this, arguments[0], proxyListener);
488+
};
489+
490+
// Make sure the .off function removes the proxyListener
491+
element.off = function() {
492+
origOff.call(this, arguments[0], proxyListener);
493+
};
494+
495+
switch (event) {
496+
case TRANSITIONEND_EVENT:
497+
ss.addRule('.ng-enter', TRANSITION_PROP + '1s linear all;' +
498+
TRANSITION_PROP + '-duration:10s;');
499+
500+
progress = bind(this, transitionProgress, element, 10);
501+
break;
502+
case ANIMATIONEND_EVENT:
503+
ss.addRule('.ng-enter', ANIMATION_PROP + ':animation 10s;');
504+
progress = bind(this, keyframeProgress, element, 10);
505+
break;
506+
}
507+
508+
var animator = $animateCss(element, options);
509+
var runner = animator.start();
510+
triggerAnimationStartFrame();
511+
512+
progress();
513+
assertAnimationComplete(true);
514+
515+
// Trigger another end event
516+
progress();
517+
518+
expect(callLog[event]).toBe(1);
519+
});
520+
});
521+
466522
it("should use the highest transition duration value detected in the CSS class", inject(function($animateCss) {
467523
ss.addRule('.ng-enter', 'transition:1s linear all;' +
468524
'transition-duration:10s, 15s, 20s;');
@@ -1074,6 +1130,49 @@ describe("ngAnimate $animateCss", function() {
10741130
expect(element).not.toHaveClass('ng-enter-active');
10751131
}));
10761132

1133+
it("should remove the transitionend event listener when the closing timeout closes the animation",
1134+
inject(function($animateCss, $document, $rootElement, $timeout) {
1135+
1136+
var element = jqLite('<div></div>'),
1137+
callLog = 0,
1138+
origOn = element.on,
1139+
origOff = element.off,
1140+
origListener,
1141+
progress;
1142+
1143+
var proxyListener = function() {
1144+
callLog++;
1145+
origListener.apply(this, arguments);
1146+
};
1147+
1148+
// Overwrite the .on function to log calls to the event listeners
1149+
element.on = function() {
1150+
origListener = arguments[1];
1151+
origOn.call(this, arguments[0], proxyListener);
1152+
};
1153+
1154+
// Make sure the .off function removes the proxyListener
1155+
element.off = function() {
1156+
origOff.call(this, arguments[0], proxyListener);
1157+
};
1158+
1159+
ss.addRule('.ng-enter', 'transition:10s linear all;');
1160+
1161+
$rootElement.append(element);
1162+
jqLite($document[0].body).append($rootElement);
1163+
1164+
var animator = $animateCss(element, { event: 'enter', structural: true });
1165+
animator.start();
1166+
triggerAnimationStartFrame();
1167+
$timeout.flush(15000);
1168+
1169+
// Force an transitionend event
1170+
transitionProgress(element, 10);
1171+
1172+
expect(callLog).toBe(0);
1173+
1174+
}));
1175+
10771176
it("should close off the animation after 150% of the animation time has passed and consider the detected delay value",
10781177
inject(function($animateCss, $document, $rootElement, $timeout) {
10791178

0 commit comments

Comments
 (0)