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
This commit is contained in:
Matias Niemelä 2013-11-14 15:36:07 -05:00
parent c47abd0dd7
commit 7067a8fb0b
2 changed files with 48 additions and 4 deletions

View file

@ -564,7 +564,7 @@ angular.module('ngAnimate', ['ng'])
//the animation if any matching animations are not found at all.
//NOTE: IE8 + IE9 should close properly (run closeAnimation()) in case a NO animation is not found.
if (animationsDisabled(element, parentElement) || matches.length === 0) {
domOperation();
fireDOMOperation();
closeAnimation();
return;
}
@ -597,7 +597,7 @@ angular.module('ngAnimate', ['ng'])
//this would mean that an animation was not allowed so let the existing
//animation do it's thing and close this one early
if(animations.length === 0) {
domOperation();
fireDOMOperation();
fireDoneCallbackAsync();
return;
}
@ -617,7 +617,7 @@ angular.module('ngAnimate', ['ng'])
//is so that the CSS classes present on the element can be properly examined.
if((animationEvent == 'addClass' && element.hasClass(className)) ||
(animationEvent == 'removeClass' && !element.hasClass(className))) {
domOperation();
fireDOMOperation();
fireDoneCallbackAsync();
return;
}
@ -628,6 +628,7 @@ angular.module('ngAnimate', ['ng'])
element.data(NG_ANIMATE_STATE, {
running:true,
className:className,
structural:!isClassBased,
animations:animations,
done:onBeforeAnimationsComplete
@ -638,7 +639,7 @@ angular.module('ngAnimate', ['ng'])
invokeRegisteredAnimationFns(animations, 'before', onBeforeAnimationsComplete);
function onBeforeAnimationsComplete(cancelled) {
domOperation();
fireDOMOperation();
if(cancelled === true) {
closeAnimation();
return;
@ -696,6 +697,15 @@ angular.module('ngAnimate', ['ng'])
doneCallback && $timeout(doneCallback, 0, false);
}
//it is less complicated to use a flag than managing and cancelling
//timeouts containing multiple callbacks.
function fireDOMOperation() {
if(!fireDOMOperation.hasBeenRun) {
fireDOMOperation.hasBeenRun = true;
domOperation();
}
}
function closeAnimation() {
if(!closeAnimation.hasBeenRun) {
closeAnimation.hasBeenRun = true;

View file

@ -2599,4 +2599,38 @@ describe("ngAnimate", function() {
});
});
it('should only perform the DOM operation once',
inject(function($sniffer, $compile, $rootScope, $rootElement, $animate, $timeout) {
if (!$sniffer.transitions) return;
ss.addRule('.base-class', '-webkit-transition:1s linear all;' +
'transition:1s linear all;');
$animate.enabled(true);
var element = $compile('<div class="base-class one two"></div>')($rootScope);
$rootElement.append(element);
jqLite($document[0].body).append($rootElement);
$animate.removeClass(element, 'base-class one two');
//still true since we're before the reflow
expect(element.hasClass('base-class')).toBe(true);
//this will cancel the remove animation
$animate.addClass(element, 'base-class one two');
//the cancellation was a success and the class was added right away
//since there was no successive animation for the after animation
expect(element.hasClass('base-class')).toBe(true);
//the reflow...
$timeout.flush();
//the reflow DOM operation was commenced but it ran before so it
//shouldn't run agaun
expect(element.hasClass('base-class')).toBe(true);
}));
});