fix(ngAnimate): ensure that only string-based addClass/removeClass va…

…lues are applied Related #11268 Closes #12458 Closes #12459
angular · Jul 29, 2015 · 0d6fc2d · 0d6fc2d
1 parent addb1ae
commit 0d6fc2d
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 26 deletions.
diff --git a/src/ng/animate.js b/src/ng/animate.js
@@ -106,31 +106,31 @@ var $$CoreAnimateQueueProvider = function() {
     };
 
     function addRemoveClassesPostDigest(element, add, remove) {
-      var data = postDigestQueue.get(element);
-      var classVal;
+      var classVal, data = postDigestQueue.get(element);
 
       if (!data) {
         postDigestQueue.put(element, data = {});
         postDigestElements.push(element);
       }
 
-      if (add) {
-        forEach(add.split(' '), function(className) {
-          if (className) {
-            data[className] = true;
-          }
-        });
-      }
-
-      if (remove) {
-        forEach(remove.split(' '), function(className) {
-          if (className) {
-            data[className] = false;
-          }
-        });
-      }
+      var updateData = function(classes, value) {
+        var changed = false;
+        if (classes) {
+          classes = isString(classes) ? classes.split(' ') :
+                    isArray(classes) ? classes : [];
+          forEach(classes, function(className) {
+            if (className) {
+              changed = true;
+              data[className] = value;
+            }
+          });
+        }
+        return changed;
+      };
 
-      if (postDigestElements.length > 1) return;
+      var classesAdded = updateData(add, true);
+      var classesRemoved = updateData(remove, false);
+      if ((!classesAdded && !classesRemoved) || postDigestElements.length > 1) return;
 
       $rootScope.$$postDigest(function() {
         forEach(postDigestElements, function(element) {

diff --git a/src/ngAnimate/animateQueue.js b/src/ngAnimate/animateQueue.js
@@ -239,22 +239,22 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
       // These methods will become available after the digest has passed
       var runner = new $$AnimateRunner();
 
-      // there are situations where a directive issues an animation for
-      // a jqLite wrapper that contains only comment nodes... If this
-      // happens then there is no way we can perform an animation
-      if (!node) {
-        close();
-        return runner;
-      }
-
       if (isArray(options.addClass)) {
         options.addClass = options.addClass.join(' ');
       }
 
+      if (options.addClass && !isString(options.addClass)) {
+        options.addClass = null;
+      }
+
       if (isArray(options.removeClass)) {
         options.removeClass = options.removeClass.join(' ');
       }
 
+      if (options.removeClass && !isString(options.removeClass)) {
+        options.removeClass = null;
+      }
+
       if (options.from && !isObject(options.from)) {
         options.from = null;
       }
@@ -263,6 +263,14 @@ var $$AnimateQueueProvider = ['$animateProvider', function($animateProvider) {
         options.to = null;
       }
 
+      // there are situations where a directive issues an animation for
+      // a jqLite wrapper that contains only comment nodes... If this
+      // happens then there is no way we can perform an animation
+      if (!node) {
+        close();
+        return runner;
+      }
+
       var className = [node.className, options.addClass, options.removeClass].join(' ');
       if (!isAnimatableClassName(className)) {
         close();

diff --git a/test/ng/animateSpec.js b/test/ng/animateSpec.js
@@ -320,6 +320,49 @@ describe("$animate", function() {
     });
   });
 
+  it('should not issue a call to addClass if the provided class value is not a string or array', function() {
+    inject(function($animate, $rootScope, $rootElement) {
+      var spy = spyOn(window, 'jqLiteAddClass').andCallThrough();
+
+      var element = jqLite('<div></div>');
+      var parent = $rootElement;
+
+      $animate.enter(element, parent, null, { addClass: noop });
+      $rootScope.$digest();
+      expect(spy).not.toHaveBeenCalled();
+
+      $animate.leave(element, { addClass: true });
+      $rootScope.$digest();
+      expect(spy).not.toHaveBeenCalled();
+
+      $animate.enter(element, parent, null, { addClass: 'fatias' });
+      $rootScope.$digest();
+      expect(spy).toHaveBeenCalled();
+    });
+  });
+
+  it('should not issue a call to removeClass if the provided class value is not a string or array', function() {
+    inject(function($animate, $rootScope, $rootElement) {
+      var spy = spyOn(window, 'jqLiteRemoveClass').andCallThrough();
+
+      var element = jqLite('<div></div>');
+      var parent = $rootElement;
+
+      $animate.enter(element, parent, null, {removeClass: noop});
+      $rootScope.$digest();
+      expect(spy).not.toHaveBeenCalled();
+
+      $animate.leave(element, {removeClass: true});
+      $rootScope.$digest();
+      expect(spy).not.toHaveBeenCalled();
+
+      element.addClass('fatias');
+      $animate.enter(element, parent, null, { removeClass: 'fatias' });
+      $rootScope.$digest();
+      expect(spy).toHaveBeenCalled();
+    });
+  });
+
   describe('CSS class DOM manipulation', function() {
     var element;
     var addClass;

diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -148,6 +148,35 @@ describe("animations", function() {
       });
     });
 
+    they('should nullify both options.$prop when passed into an animation if it is not a string or an array', ['addClass', 'removeClass'], function(prop) {
+      inject(function($animate, $rootScope) {
+        var options1 = {};
+        options1[prop] = function() {};
+        $animate.enter(element, parent, null, options1);
+
+        expect(options1[prop]).toBeFalsy();
+        $rootScope.$digest();
+
+        var options2 = {};
+        options2[prop] = true;
+        $animate.leave(element, options2);
+
+        expect(options2[prop]).toBeFalsy();
+        $rootScope.$digest();
+
+        capturedAnimation = null;
+
+        var options3 = {};
+        if (prop === 'removeClass') {
+          element.addClass('fatias');
+        }
+
+        options3[prop] = ['fatias'];
+        $animate.enter(element, parent, null, options3);
+        expect(options3[prop]).toBe('fatias');
+      });
+    });
+
     it('should throw a minErr if a regex value is used which partially contains or fully matches the `ng-animate` CSS class', function() {
       module(function($animateProvider) {
         assertError(/ng-animate/, true);