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

fix(ngAnimate): ensure that orphaned elements do not throw errors when animated #12338

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/ngAnimate/animateCss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = [];
Expand Down Expand Up @@ -782,6 +786,10 @@ var $AnimateCssProvider = ['$animateProvider', function($animateProvider) {

function start() {
if (animationClosed) return;
if (!node.parentNode) {
close();
return;
}

var startTime, events = [];

Expand Down
2 changes: 1 addition & 1 deletion src/ngAnimate/animation.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
37 changes: 37 additions & 0 deletions test/ngAnimate/animateCssSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when an element has no parent removes the potential confusion of a double negative.

inject(function($animateCss, $$rAF, $rootScope, $document, $rootElement) {

var element = jqLite('<div></div>');
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('<div></div>');
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) {
Expand Down
28 changes: 23 additions & 5 deletions test/ngAnimate/animationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,10 @@ describe('$$animation', function() {
};
});

inject(function($$animation, $rootScope) {
inject(function($$animation, $rootScope, $rootElement) {
element = jqLite('<div></div>');
$rootElement.append(element);

$$animation(element, 'enter');
$rootScope.$digest();

Expand Down Expand Up @@ -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';
Expand All @@ -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',
Expand Down Expand Up @@ -165,8 +170,9 @@ describe('$$animation', function() {
});
});

inject(function($$animation, $rootScope) {
inject(function($$animation, $rootScope, $rootElement) {
element = jqLite('<div></div>');
$rootElement.append(element);
$$animation(element, 'enter');
$rootScope.$digest();
expect(log).toEqual(['second', 'first']);
Expand Down Expand Up @@ -237,8 +243,10 @@ describe('$$animation', function() {
});
});

inject(function($$animation, $rootScope) {
inject(function($$animation, $rootScope, $rootElement) {
element = jqLite('<div></div>');
$rootElement.append(element);

var runner = $$animation(element, 'enter');
$rootScope.$digest();

Expand Down Expand Up @@ -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'
});
Expand All @@ -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');
Expand All @@ -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) {
Expand All @@ -832,6 +846,8 @@ describe('$$animation', function() {
});

inject(function($$animation, $rootScope) {
parent.append(element);

$$animation(element, 'enter', {
tempClasses: 'temp-class-name'
});
Expand All @@ -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() {
Expand Down
27 changes: 27 additions & 0 deletions test/ngAnimate/integrationSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('<div></div>');
html(parent);

var element = jqLite('<div class="animate-me">DOING</div>');
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() {
Expand Down