Skip to content

Commit 2c7bdb2

Browse files
committed
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 angular#11975 Closes angular#12338
1 parent 4cef752 commit 2c7bdb2

File tree

5 files changed

+96
-6
lines changed

5 files changed

+96
-6
lines changed

src/ngAnimate/animateCss.js

+8
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
497497

498498
function init(element, options) {
499499
var node = getDomNode(element);
500+
if (!node || !node.parentNode) {
501+
return closeAndReturnNoopAnimator();
502+
}
503+
500504
options = prepareAnimationOptions(options);
501505

502506
var temporaryStyles = [];
@@ -782,6 +786,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {
782786

783787
function start() {
784788
if (animationClosed) return;
789+
if (!node.parentNode) {
790+
close();
791+
return;
792+
}
785793

786794
var startTime, events = [];
787795

src/ngAnimate/animation.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -138,7 +138,7 @@ var $$AnimationProvider = ['$animateProvider', function($animateProvider) {
138138
? (animationEntry.from.element || animationEntry.to.element)
139139
: animationEntry.element;
140140

141-
if (getRunner(targetElement)) {
141+
if (getRunner(targetElement) && getDomNode(targetElement).parentNode) {
142142
var operation = invokeFirstDriver(animationEntry);
143143
if (operation) {
144144
startAnimationFn = operation.start;

test/ngAnimate/animateCssSpec.js

+37
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,43 @@ describe("ngAnimate $animateCss", function() {
5151
describe('when active', function() {
5252
if (!browserSupportsCssAnimations()) return;
5353

54+
it("should silently quit the animation and not throw when an element is not attached to a parent during preparation",
55+
inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) {
56+
57+
var element = jqLite('<div></div>');
58+
expect(function() {
59+
$animateCss(element, {
60+
duration: 1000,
61+
event: 'fake',
62+
to: fakeStyle
63+
}).start();
64+
}).not.toThrow();
65+
66+
expect(element).not.toHaveClass('fake');
67+
triggerAnimationStartFrame();
68+
expect(element).not.toHaveClass('fake-active');
69+
}));
70+
71+
it("should silently quit the animation and not throw when an element is not attached to a parent before starting",
72+
inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) {
73+
74+
var element = jqLite('<div></div>');
75+
jqLite($document[0].body).append($rootElement);
76+
$rootElement.append(element);
77+
78+
$animateCss(element, {
79+
duration: 1000,
80+
addClass: 'wait-for-it',
81+
to: fakeStyle
82+
}).start();
83+
84+
element.remove();
85+
86+
expect(function() {
87+
triggerAnimationStartFrame();
88+
}).not.toThrow();
89+
}));
90+
5491
describe("rAF usage", function() {
5592
it("should buffer all requests into a single requestAnimationFrame call",
5693
inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) {

test/ngAnimate/animationSpec.js

+23-5
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,10 @@ describe('$$animation', function() {
8080
};
8181
});
8282

83-
inject(function($$animation, $rootScope) {
83+
inject(function($$animation, $rootScope, $rootElement) {
8484
element = jqLite('<div></div>');
85+
$rootElement.append(element);
86+
8587
$$animation(element, 'enter');
8688
$rootScope.$digest();
8789

@@ -109,7 +111,8 @@ describe('$$animation', function() {
109111
}));
110112

111113
it("should obtain the element, event, the provided options and the domOperation",
112-
inject(function($$animation, $rootScope) {
114+
inject(function($$animation, $rootScope, $rootElement) {
115+
$rootElement.append(element);
113116

114117
var options = {};
115118
options.foo = 'bar';
@@ -132,9 +135,11 @@ describe('$$animation', function() {
132135
}));
133136

134137
it("should obtain the classes string which is a combination of className, addClass and removeClass",
135-
inject(function($$animation, $rootScope) {
138+
inject(function($$animation, $rootScope, $rootElement) {
136139

137140
element.addClass('blue red');
141+
$rootElement.append(element);
142+
138143
$$animation(element, 'enter', {
139144
addClass: 'green',
140145
removeClass: 'orange',
@@ -165,8 +170,9 @@ describe('$$animation', function() {
165170
});
166171
});
167172

168-
inject(function($$animation, $rootScope) {
173+
inject(function($$animation, $rootScope, $rootElement) {
169174
element = jqLite('<div></div>');
175+
$rootElement.append(element);
170176
$$animation(element, 'enter');
171177
$rootScope.$digest();
172178
expect(log).toEqual(['second', 'first']);
@@ -237,8 +243,10 @@ describe('$$animation', function() {
237243
});
238244
});
239245

240-
inject(function($$animation, $rootScope) {
246+
inject(function($$animation, $rootScope, $rootElement) {
241247
element = jqLite('<div></div>');
248+
$rootElement.append(element);
249+
242250
var runner = $$animation(element, 'enter');
243251
$rootScope.$digest();
244252

@@ -791,6 +799,8 @@ describe('$$animation', function() {
791799
it('should temporarily assign the provided CSS class for the duration of the animation',
792800
inject(function($rootScope, $$animation) {
793801

802+
parent.append(element);
803+
794804
$$animation(element, 'enter', {
795805
tempClasses: 'temporary fudge'
796806
});
@@ -809,6 +819,8 @@ describe('$$animation', function() {
809819
it('should add and remove the ng-animate CSS class when the animation is active',
810820
inject(function($$animation, $rootScope) {
811821

822+
parent.append(element);
823+
812824
$$animation(element, 'enter');
813825
$rootScope.$digest();
814826
expect(element).toHaveClass('ng-animate');
@@ -823,6 +835,8 @@ describe('$$animation', function() {
823835
it('should apply the `ng-animate` and temporary CSS classes before the driver is invoked', function() {
824836
var capturedElementClasses;
825837

838+
parent.append(element);
839+
826840
module(function($provide) {
827841
$provide.factory('mockedTestDriver', function() {
828842
return function(details) {
@@ -832,6 +846,8 @@ describe('$$animation', function() {
832846
});
833847

834848
inject(function($$animation, $rootScope) {
849+
parent.append(element);
850+
835851
$$animation(element, 'enter', {
836852
tempClasses: 'temp-class-name'
837853
});
@@ -845,6 +861,8 @@ describe('$$animation', function() {
845861
it('should perform the DOM operation at the end of the animation if the driver doesn\'t run it already',
846862
inject(function($$animation, $rootScope) {
847863

864+
parent.append(element);
865+
848866
var domOperationFired = false;
849867
$$animation(element, 'enter', {
850868
domOperation: function() {

test/ngAnimate/integrationSpec.js

+27
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,33 @@ describe('ngAnimate integration tests', function() {
105105
expect(animationCompleted).toBe(true);
106106
});
107107
});
108+
109+
it('should not throw an error if the element is orphaned before the CSS animation starts',
110+
inject(function($rootScope, $rootElement, $animate, $$rAF) {
111+
112+
ss.addRule('.animate-me', 'transition:2s linear all;');
113+
114+
var parent = jqLite('<div></div>');
115+
html(parent);
116+
117+
var element = jqLite('<div class="animate-me">DOING</div>');
118+
parent.append(element);
119+
120+
$animate.addClass(parent, 'on');
121+
$animate.addClass(element, 'on');
122+
$rootScope.$digest();
123+
124+
// this will run the first class-based animation
125+
$$rAF.flush();
126+
127+
element.remove();
128+
129+
expect(function() {
130+
$$rAF.flush();
131+
}).not.toThrow();
132+
133+
dealoc(element);
134+
}));
108135
});
109136

110137
describe('JS animations', function() {

0 commit comments

Comments
 (0)