fix(ngClass): add/remove classes which are properties of Object.prototype

caitp · caitp · commit 91a5f1f6f4bf · 2015-05-05T18:58:51.000-04:00
- fast paths for various internal functions when createMap() is used - Make createMap() safe for internal functions like copy/equals/forEach - Use createMap() in more places to avoid needing hasOwnProperty() Closes #11813
diff --git a/src/Angular.js b/src/Angular.js
@@ -267,9 +267,16 @@ function forEach(obj, iterator, context) {
       }
     } else if (obj.forEach && obj.forEach !== forEach) {
         obj.forEach(iterator, context, obj);
+    } else if (isObject(obj) && !Object.getPrototypeOf(obj)) {
+      // Fast path for createMap()
+      for (key in obj) {
+        iterator.call(context, obj[key], key, obj);
+      }
     } else {
+      // Slow path for ordinary objects
+      var hasOwn = typeof obj.hasOwnProperty === 'function';
       for (key in obj) {
-        if (obj.hasOwnProperty(key)) {
+        if (hasOwn ? obj.hasOwnProperty(key) : hasOwnProperty.call(obj, key)) {
           iterator.call(context, obj[key], key, obj);
         }
       }
@@ -820,14 +827,29 @@ function copy(source, destination, stackSource, stackDest) {
           delete destination[key];
         });
       }
-      for (var key in source) {
-        if (source.hasOwnProperty(key)) {
-          result = copy(source[key], null, stackSource, stackDest);
-          if (isObject(source[key])) {
-            stackSource.push(source[key]);
+      if (isObject(source) && !Object.getPrototypeOf(source)) {
+        // createMap() fast path
+        for (var key in source) {
+          var val = source[key];
+          result = copy(val, null, stackSource, stackDest);
+          if (isObject(val)) {
+            stackSource.push(val);
             stackDest.push(result);
           }
-          destination[key] = result;
+        }
+      } else {
+        // Slow path, which must rely on hasOwnProperty
+        var hasOwn = source && typeof source.hasOwnProperty === 'function';
+        for (var key in source) {
+          if (hasOwn ? source.hasOwnProperty(key) : hasOwnProperty.call(source, key)) {
+            var val = source[key];
+            result = copy(val, null, stackSource, stackDest);
+            if (isObject(val)) {
+              stackSource.push(val);
+              stackDest.push(result);
+            }
+            destination[key] = result;
+          }
         }
       }
       setHashKey(destination,h);
@@ -915,14 +937,14 @@ function equals(o1, o2) {
       } else {
         if (isScope(o1) || isScope(o2) || isWindow(o1) || isWindow(o2) ||
           isArray(o2) || isDate(o2) || isRegExp(o2)) return false;
-        keySet = {};
+        keySet = createMap();
         for (key in o1) {
           if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
           if (!equals(o1[key], o2[key])) return false;
           keySet[key] = true;
         }
         for (key in o2) {
-          if (!keySet.hasOwnProperty(key) &&
+          if (!(key in keySet) &&
               key.charAt(0) !== '$' &&
               o2[key] !== undefined &&
               !isFunction(o2[key])) return false;
diff --git a/src/ng/animate.js b/src/ng/animate.js
@@ -26,7 +26,9 @@ function splitClasses(classes) {
     classes = classes.split(' ');
   }
 
-  var obj = {};
+  // Use createMap() to prevent class assumptions involving property names in
+  // Object.prototype
+  var obj = createMap();
   forEach(classes, function(klass) {
     // sometimes the split leaves empty string values
     // incase extra spaces were applied to the options
diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js
@@ -39,7 +39,9 @@ function classDirective(name, selector) {
         }
 
         function digestClassCounts(classes, count) {
-          var classCounts = element.data('$classCounts') || {};
+          // Use createMap() to prevent class assumptions involving property
+          // names in Object.prototype
+          var classCounts = element.data('$classCounts') || createMap();
           var classesToUpdate = [];
           forEach(classes, function(className) {
             if (count > 0 || classCounts[className]) {
diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js
@@ -33,6 +33,31 @@ describe('ngClass', function() {
   }));
 
 
+  it('should add new and remove old classes with same names as Object.prototype properties dynamically', inject(function($rootScope, $compile) {
+    element = $compile('<div class="existing" ng-class="dynClass"></div>')($rootScope);
+    $rootScope.dynClass = { watch: true, hasOwnProperty: true, isPrototypeOf: true };
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(true);
+    expect(element.hasClass('hasOwnProperty')).toBe(true);
+    expect(element.hasClass('isPrototypeOf')).toBe(true);
+
+    $rootScope.dynClass.watch = false;
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(false);
+    expect(element.hasClass('hasOwnProperty')).toBe(true);
+    expect(element.hasClass('isPrototypeOf')).toBe(true);
+
+    delete $rootScope.dynClass;
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(false);
+    expect(element.hasClass('hasOwnProperty')).toBe(false);
+    expect(element.hasClass('isPrototypeOf')).toBe(false);
+  }));
+
+
   it('should support adding multiple classes via an array', inject(function($rootScope, $compile) {
     element = $compile('<div class="existing" ng-class="[\'A\', \'B\']"></div>')($rootScope);
     $rootScope.$digest();
