From 2c7bdb2e1c3b129eb0dc01706e75494558264f8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matias=20Niemel=C3=A4?= Date: Fri, 3 Jul 2015 09:09:17 -0400 Subject: [PATCH] fix(ngAnimate): ensure that orphaned elements do not throw errors when animated This fix ensures that for both `$animateCss` and `$animate` capture the error when an animation takes place where the element is removed from the parent element sometime before or during the preparation stages of the animation. Closes #11975 Closes #12338 --- src/ngAnimate/animateCss.js | 8 +++++++ src/ngAnimate/animation.js | 2 +- test/ngAnimate/animateCssSpec.js | 37 +++++++++++++++++++++++++++++++ test/ngAnimate/animationSpec.js | 28 ++++++++++++++++++----- test/ngAnimate/integrationSpec.js | 27 ++++++++++++++++++++++ 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/src/ngAnimate/animateCss.js b/src/ngAnimate/animateCss.js index 50a842717f29..b92c36c70089 100644 --- a/src/ngAnimate/animateCss.js +++ b/src/ngAnimate/animateCss.js @@ -497,6 +497,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { function init(element, options) { var node = getDomNode(element); + if (!node || !node.parentNode) { + return closeAndReturnNoopAnimator(); + } + options = prepareAnimationOptions(options); var temporaryStyles = []; @@ -782,6 +786,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) { function start() { if (animationClosed) return; + if (!node.parentNode) { + close(); + return; + } var startTime, events = []; diff --git a/src/ngAnimate/animation.js b/src/ngAnimate/animation.js index 169b8150a400..b7ec28b5742e 100644 --- a/src/ngAnimate/animation.js +++ b/src/ngAnimate/animation.js @@ -138,7 +138,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) { ? (animationEntry.from.element || animationEntry.to.element) : animationEntry.element; - if (getRunner(targetElement)) { + if (getRunner(targetElement) && getDomNode(targetElement).parentNode) { var operation = invokeFirstDriver(animationEntry); if (operation) { startAnimationFn = operation.start; diff --git a/test/ngAnimate/animateCssSpec.js b/test/ngAnimate/animateCssSpec.js index 6bed68a9b26e..9f333f1a44a7 100644 --- a/test/ngAnimate/animateCssSpec.js +++ b/test/ngAnimate/animateCssSpec.js @@ -51,6 +51,43 @@ describe("ngAnimate $animateCss", function() { describe('when active', function() { if (!browserSupportsCssAnimations()) return; + it("should silently quit the animation and not throw when an element is not attached to a parent during preparation", + inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) { + + var element = jqLite('
'); + expect(function() { + $animateCss(element, { + duration: 1000, + event: 'fake', + to: fakeStyle + }).start(); + }).not.toThrow(); + + expect(element).not.toHaveClass('fake'); + triggerAnimationStartFrame(); + expect(element).not.toHaveClass('fake-active'); + })); + + it("should silently quit the animation and not throw when an element is not attached to a parent before starting", + inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) { + + var element = jqLite('
'); + jqLite($document[0].body).append($rootElement); + $rootElement.append(element); + + $animateCss(element, { + duration: 1000, + addClass: 'wait-for-it', + to: fakeStyle + }).start(); + + element.remove(); + + expect(function() { + triggerAnimationStartFrame(); + }).not.toThrow(); + })); + describe("rAF usage", function() { it("should buffer all requests into a single requestAnimationFrame call", inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) { diff --git a/test/ngAnimate/animationSpec.js b/test/ngAnimate/animationSpec.js index a7ffb8eb4235..dcda42f1d76f 100644 --- a/test/ngAnimate/animationSpec.js +++ b/test/ngAnimate/animationSpec.js @@ -80,8 +80,10 @@ describe('$$animation', function() { }; }); - inject(function($$animation, $rootScope) { + inject(function($$animation, $rootScope, $rootElement) { element = jqLite('
'); + $rootElement.append(element); + $$animation(element, 'enter'); $rootScope.$digest(); @@ -109,7 +111,8 @@ describe('$$animation', function() { })); it("should obtain the element, event, the provided options and the domOperation", - inject(function($$animation, $rootScope) { + inject(function($$animation, $rootScope, $rootElement) { + $rootElement.append(element); var options = {}; options.foo = 'bar'; @@ -132,9 +135,11 @@ describe('$$animation', function() { })); it("should obtain the classes string which is a combination of className, addClass and removeClass", - inject(function($$animation, $rootScope) { + inject(function($$animation, $rootScope, $rootElement) { element.addClass('blue red'); + $rootElement.append(element); + $$animation(element, 'enter', { addClass: 'green', removeClass: 'orange', @@ -165,8 +170,9 @@ describe('$$animation', function() { }); }); - inject(function($$animation, $rootScope) { + inject(function($$animation, $rootScope, $rootElement) { element = jqLite('
'); + $rootElement.append(element); $$animation(element, 'enter'); $rootScope.$digest(); expect(log).toEqual(['second', 'first']); @@ -237,8 +243,10 @@ describe('$$animation', function() { }); }); - inject(function($$animation, $rootScope) { + inject(function($$animation, $rootScope, $rootElement) { element = jqLite('
'); + $rootElement.append(element); + var runner = $$animation(element, 'enter'); $rootScope.$digest(); @@ -791,6 +799,8 @@ describe('$$animation', function() { it('should temporarily assign the provided CSS class for the duration of the animation', inject(function($rootScope, $$animation) { + parent.append(element); + $$animation(element, 'enter', { tempClasses: 'temporary fudge' }); @@ -809,6 +819,8 @@ describe('$$animation', function() { it('should add and remove the ng-animate CSS class when the animation is active', inject(function($$animation, $rootScope) { + parent.append(element); + $$animation(element, 'enter'); $rootScope.$digest(); expect(element).toHaveClass('ng-animate'); @@ -823,6 +835,8 @@ describe('$$animation', function() { it('should apply the `ng-animate` and temporary CSS classes before the driver is invoked', function() { var capturedElementClasses; + parent.append(element); + module(function($provide) { $provide.factory('mockedTestDriver', function() { return function(details) { @@ -832,6 +846,8 @@ describe('$$animation', function() { }); inject(function($$animation, $rootScope) { + parent.append(element); + $$animation(element, 'enter', { tempClasses: 'temp-class-name' }); @@ -845,6 +861,8 @@ describe('$$animation', function() { it('should perform the DOM operation at the end of the animation if the driver doesn\'t run it already', inject(function($$animation, $rootScope) { + parent.append(element); + var domOperationFired = false; $$animation(element, 'enter', { domOperation: function() { diff --git a/test/ngAnimate/integrationSpec.js b/test/ngAnimate/integrationSpec.js index 5e01abc25800..8b4cf1cf9ba9 100644 --- a/test/ngAnimate/integrationSpec.js +++ b/test/ngAnimate/integrationSpec.js @@ -105,6 +105,33 @@ describe('ngAnimate integration tests', function() { expect(animationCompleted).toBe(true); }); }); + + it('should not throw an error if the element is orphaned before the CSS animation starts', + inject(function($rootScope, $rootElement, $animate, $$rAF) { + + ss.addRule('.animate-me', 'transition:2s linear all;'); + + var parent = jqLite('
'); + html(parent); + + var element = jqLite('
DOING
'); + parent.append(element); + + $animate.addClass(parent, 'on'); + $animate.addClass(element, 'on'); + $rootScope.$digest(); + + // this will run the first class-based animation + $$rAF.flush(); + + element.remove(); + + expect(function() { + $$rAF.flush(); + }).not.toThrow(); + + dealoc(element); + })); }); describe('JS animations', function() {