fix($animate): avoid accidentally matching substrings when resolving …

…the presence of className tokens
angular · Jan 14, 2014 · 524650a · 524650a
1 parent 02a4582
commit 524650a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 4 deletions.
diff --git a/src/ngAnimate/animate.js b/src/ngAnimate/animate.js
@@ -647,10 +647,11 @@ angular.module('ngAnimate', ['ng'])
           return;
         }
 
+        var ONE_SPACE = ' ';
         //this value will be searched for class-based CSS className lookup. Therefore,
         //we prefix and suffix the current className value with spaces to avoid substring
         //lookups of className tokens
-        var futureClassName = ' ' + currentClassName + ' ';
+        var futureClassName = ONE_SPACE + currentClassName + ONE_SPACE;
         if(ngAnimateState.running) {
           //if an animation is currently running on the element then lets take the steps
           //to cancel that animation and fire any required callbacks
@@ -671,16 +672,16 @@ angular.module('ngAnimate', ['ng'])
             //will be invalid. Therefore the same string manipulation that would occur within the
             //DOM operation will be performed below so that the class comparison is valid...
             futureClassName = ngAnimateState.event == 'removeClass' ?
-              futureClassName.replace(ngAnimateState.className, '') :
-              futureClassName + ngAnimateState.className + ' ';
+              futureClassName.replace(ONE_SPACE + ngAnimateState.className + ONE_SPACE, ONE_SPACE) :
+              futureClassName + ngAnimateState.className + ONE_SPACE;
           }
         }
 
         //There is no point in perform a class-based animation if the element already contains
         //(on addClass) or doesn't contain (on removeClass) the className being animated.
         //The reason why this is being called after the previous animations are cancelled
         //is so that the CSS classes present on the element can be properly examined.
-        var classNameToken = ' ' + className + ' ';
+        var classNameToken = ONE_SPACE + className + ONE_SPACE;
         if((animationEvent == 'addClass'    && futureClassName.indexOf(classNameToken) >= 0) ||
            (animationEvent == 'removeClass' && futureClassName.indexOf(classNameToken) == -1)) {
           fireDOMOperation();

diff --git a/test/ngAnimate/animateSpec.js b/test/ngAnimate/animateSpec.js
@@ -2665,6 +2665,37 @@ describe("ngAnimate", function() {
       expect(element.hasClass('red')).toBe(true);
     }));
 
+    it("should avoid mixing up substring classes during add and remove operations", function() {
+      var currentAnimation, currentFn;
+      module(function($animateProvider) {
+        $animateProvider.register('.on', function() {
+          return {
+            beforeAddClass : function(element, className, done) {
+              currentAnimation = 'addClass';
+              currentFn = done;
+            },
+            beforeRemoveClass : function(element, className, done) {
+              currentAnimation = 'removeClass';
+              currentFn = done;
+            }
+          };
+        });
+      });
+      inject(function($compile, $rootScope, $animate, $sniffer, $timeout) {
+        var element = $compile('<div class="animation-enabled only"></div>')($rootScope);
+        $rootElement.append(element);
+        jqLite($document[0].body).append($rootElement);
+
+        $animate.addClass(element, 'on');
+        expect(currentAnimation).toBe('addClass');
+        currentFn();
+
+        $animate.removeClass(element, 'on');
+        $animate.addClass(element, 'on');
+
+        expect(currentAnimation).toBe('addClass');
+      });
+    });
 
     it('should enable and disable animations properly on the root element', function() {
       var count = 0;