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

matsko · petebacondarwin · commit e4aeae0c7303 · 2015-07-14T21:06:55.000+01:00
This fix ensures that both `$animateCss` and `$animate` swallow the error when an animation takes place in the sitation that the element is removed from the parent element sometime before or during the preparation stages of the animation. Closes angular#11975 Closes angular#12338
diff --git 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
@@ -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
@@ -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 has no parent during preparation",
+      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 has no 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) {
diff --git 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('<div></div>');
+        $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('<div></div>');
+        $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('<div></div>');
+        $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
@@ -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() {
