From 28caba7428d7a4d586b73b3afa3fa1e83278d622 Mon Sep 17 00:00:00 2001 From: Martin Staffa Date: Sun, 12 Jun 2016 17:16:07 +0200 Subject: [PATCH] fix(ngAnimate): allow removal of class that is scheduled to be added with requestAnimationFrame The following case can happen when ngClass updates an element's classes in very quick order in the following way: - First animation adds class "a" - A digest passes, but "a" is not yet added to the element - Second animation adds class "b" - No digest passes, and "a" is still not added to the element, because requestAnimationFrame hasn't been flushed yet - Third animation removes class "a" - the third animation gets merged into the second animation Before this change: - Because the element doesn't have class "a" yet, ngAnimate resolves that it cannot remove class "a". However, the first animation is still running, and finally adds "a" After this change: - ngAnimate reacts to the temporary class "add-a", which indicates that "a" is about to be added and decides that "a" can be removed after all. This is a very rare case where setting the element's class is not fast enough, and subsequent animations operate on incorrect assumptions. "In the wild", this is caused by rapidly updating ngClass, which uses inidvidual addClass and removeClass calls when both operations happen in a single digest. Fixes #14582 --- src/ngAnimate/shared.js | 4 +-- test/ngAnimate/integrationSpec.js | 45 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/src/ngAnimate/shared.js b/src/ngAnimate/shared.js index 54d3c95412ae..259895f5b549 100644 --- a/src/ngAnimate/shared.js +++ b/src/ngAnimate/shared.js @@ -285,10 +285,10 @@ function resolveElementClasses(existing, toAdd, toRemove) { var prop, allow; if (val === ADD_CLASS) { prop = 'addClass'; - allow = !existing[klass]; + allow = !existing[klass] || existing[klass + REMOVE_CLASS_SUFFIX]; } else if (val === REMOVE_CLASS) { prop = 'removeClass'; - allow = existing[klass]; + allow = existing[klass] || existing[klass + ADD_CLASS_SUFFIX]; } if (allow) { if (classes[prop].length) { diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index 25553298f1bb..a84c3eb88a2a 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -51,6 +51,51 @@ describe('ngAnimate integration tests', function() { expect(doneHandler).toHaveBeenCalled(); })); + it('should remove a class that is currently being added by a running animation when another class is added in before in the same digest', + inject(function($animate, $rootScope, $$rAF, $document, $rootElement) { + + jqLite($document[0].body).append($rootElement); + element = jqLite('
'); + $rootElement.append(element); + + var runner = $animate.addClass(element, 'red'); + + $rootScope.$digest(); + + $animate.addClass(element, 'blue'); + $animate.removeClass(element, 'red'); + $rootScope.$digest(); + + $$rAF.flush(); + + expect(element).not.toHaveClass('red'); + expect(element).toHaveClass('blue'); + })); + + + it('should add a class that is currently being removed by a running animation when another class is removed before in the same digest', + inject(function($animate, $rootScope, $$rAF, $document, $rootElement) { + + jqLite($document[0].body).append($rootElement); + element = jqLite('
'); + $rootElement.append(element); + element.addClass('red blue'); + + var runner = $animate.removeClass(element, 'red'); + + $rootScope.$digest(); + + $animate.removeClass(element, 'blue'); + $animate.addClass(element, 'red'); + $rootScope.$digest(); + + $$rAF.flush(); + + expect(element).not.toHaveClass('blue'); + expect(element).toHaveClass('red'); + })); + + describe('CSS animations', function() { if (!browserSupportsCssAnimations()) return;