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

Commit 7067a8f

Browse files
committed
fix($animate): ensure the DOM operation isn't run twice
Depending on the animations placed on ngClass, the DOM operation may run twice causing a race condition between addClass and removeClass. Depending on what classes are removed and added via $compile this may cause all CSS classes to be removed accidentally from the element being animated. Closes #4949
1 parent c47abd0 commit 7067a8f

File tree

2 files changed

+48
-4
lines changed

2 files changed

+48
-4
lines changed

src/ngAnimate/animate.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -564,7 +564,7 @@ angular.module('ngAnimate', ['ng'])
564564
//the animation if any matching animations are not found at all.
565565
//NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case a NO animation is not found.
566566
if (animationsDisabled(element, parentElement) || matches.length === 0) {
567-
domOperation();
567+
fireDOMOperation();
568568
closeAnimation();
569569
return;
570570
}
@@ -597,7 +597,7 @@ angular.module('ngAnimate', ['ng'])
597597
//this would mean that an animation was not allowed so let the existing
598598
//animation do it's thing and close this one early
599599
if(animations.length === 0) {
600-
domOperation();
600+
fireDOMOperation();
601601
fireDoneCallbackAsync();
602602
return;
603603
}
@@ -617,7 +617,7 @@ angular.module('ngAnimate', ['ng'])
617617
//is so that the CSS classes present on the element can be properly examined.
618618
if((animationEvent == 'addClass' && element.hasClass(className)) ||
619619
(animationEvent == 'removeClass' && !element.hasClass(className))) {
620-
domOperation();
620+
fireDOMOperation();
621621
fireDoneCallbackAsync();
622622
return;
623623
}
@@ -628,6 +628,7 @@ angular.module('ngAnimate', ['ng'])
628628

629629
element.data(NG_ANIMATE_STATE, {
630630
running:true,
631+
className:className,
631632
structural:!isClassBased,
632633
animations:animations,
633634
done:onBeforeAnimationsComplete
@@ -638,7 +639,7 @@ angular.module('ngAnimate', ['ng'])
638639
invokeRegisteredAnimationFns(animations, 'before', onBeforeAnimationsComplete);
639640

640641
function onBeforeAnimationsComplete(cancelled) {
641-
domOperation();
642+
fireDOMOperation();
642643
if(cancelled === true) {
643644
closeAnimation();
644645
return;
@@ -696,6 +697,15 @@ angular.module('ngAnimate', ['ng'])
696697
doneCallback && $timeout(doneCallback, 0, false);
697698
}
698699

700+
//it is less complicated to use a flag than managing and cancelling
701+
//timeouts containing multiple callbacks.
702+
function fireDOMOperation() {
703+
if(!fireDOMOperation.hasBeenRun) {
704+
fireDOMOperation.hasBeenRun = true;
705+
domOperation();
706+
}
707+
}
708+
699709
function closeAnimation() {
700710
if(!closeAnimation.hasBeenRun) {
701711
closeAnimation.hasBeenRun = true;

test/ngAnimate/animateSpec.js

+34
Original file line numberDiff line numberDiff line change
@@ -2599,4 +2599,38 @@ describe("ngAnimate", function() {
25992599
});
26002600
});
26012601

2602+
it('should only perform the DOM operation once',
2603+
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {
2604+
2605+
if (!$sniffer.transitions) return;
2606+
2607+
ss.addRule('.base-class', '-webkit-transition:1s linear all;' +
2608+
'transition:1s linear all;');
2609+
2610+
$animate.enabled(true);
2611+
2612+
var element = $compile('<div class="base-class one two"></div>')($rootScope);
2613+
$rootElement.append(element);
2614+
jqLite($document[0].body).append($rootElement);
2615+
2616+
$animate.removeClass(element, 'base-class one two');
2617+
2618+
//still true since we're before the reflow
2619+
expect(element.hasClass('base-class')).toBe(true);
2620+
2621+
//this will cancel the remove animation
2622+
$animate.addClass(element, 'base-class one two');
2623+
2624+
//the cancellation was a success and the class was added right away
2625+
//since there was no successive animation for the after animation
2626+
expect(element.hasClass('base-class')).toBe(true);
2627+
2628+
//the reflow...
2629+
$timeout.flush();
2630+
2631+
//the reflow DOM operation was commenced but it ran before so it
2632+
//shouldn't run agaun
2633+
expect(element.hasClass('base-class')).toBe(true);
2634+
}));
2635+
26022636
});

0 commit comments

Comments
 (0)