Skip to content

Commit 91a5f1f

Browse files
committed
fix(ngClass): add/remove classes which are properties of Object.prototype
- 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 angular#11813
1 parent b5a9053 commit 91a5f1f

File tree

4 files changed

+62
-11
lines changed

4 files changed

+62
-11
lines changed

src/Angular.js

+31-9
Original file line numberDiff line numberDiff line change
@@ -267,9 +267,16 @@ function forEach(obj, iterator, context) {
267267
}
268268
} else if (obj.forEach && obj.forEach !== forEach) {
269269
obj.forEach(iterator, context, obj);
270+
} else if (isObject(obj) && !Object.getPrototypeOf(obj)) {
271+
// Fast path for createMap()
272+
for (key in obj) {
273+
iterator.call(context, obj[key], key, obj);
274+
}
270275
} else {
276+
// Slow path for ordinary objects
277+
var hasOwn = typeof obj.hasOwnProperty === 'function';
271278
for (key in obj) {
272-
if (obj.hasOwnProperty(key)) {
279+
if (hasOwn ? obj.hasOwnProperty(key) : hasOwnProperty.call(obj, key)) {
273280
iterator.call(context, obj[key], key, obj);
274281
}
275282
}
@@ -820,14 +827,29 @@ function copy(source, destination, stackSource, stackDest) {
820827
delete destination[key];
821828
});
822829
}
823-
for (var key in source) {
824-
if (source.hasOwnProperty(key)) {
825-
result = copy(source[key], null, stackSource, stackDest);
826-
if (isObject(source[key])) {
827-
stackSource.push(source[key]);
830+
if (isObject(source) && !Object.getPrototypeOf(source)) {
831+
// createMap() fast path
832+
for (var key in source) {
833+
var val = source[key];
834+
result = copy(val, null, stackSource, stackDest);
835+
if (isObject(val)) {
836+
stackSource.push(val);
828837
stackDest.push(result);
829838
}
830-
destination[key] = result;
839+
}
840+
} else {
841+
// Slow path, which must rely on hasOwnProperty
842+
var hasOwn = source && typeof source.hasOwnProperty === 'function';
843+
for (var key in source) {
844+
if (hasOwn ? source.hasOwnProperty(key) : hasOwnProperty.call(source, key)) {
845+
var val = source[key];
846+
result = copy(val, null, stackSource, stackDest);
847+
if (isObject(val)) {
848+
stackSource.push(val);
849+
stackDest.push(result);
850+
}
851+
destination[key] = result;
852+
}
831853
}
832854
}
833855
setHashKey(destination,h);
@@ -915,14 +937,14 @@ function equals(o1, o2) {
915937
} else {
916938
if (isScope(o1) || isScope(o2) || isWindow(o1) || isWindow(o2) ||
917939
isArray(o2) || isDate(o2) || isRegExp(o2)) return false;
918-
keySet = {};
940+
keySet = createMap();
919941
for (key in o1) {
920942
if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
921943
if (!equals(o1[key], o2[key])) return false;
922944
keySet[key] = true;
923945
}
924946
for (key in o2) {
925-
if (!keySet.hasOwnProperty(key) &&
947+
if (!(key in keySet) &&
926948
key.charAt(0) !== '$' &&
927949
o2[key] !== undefined &&
928950
!isFunction(o2[key])) return false;

src/ng/animate.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ function splitClasses(classes) {
2626
classes = classes.split(' ');
2727
}
2828

29-
var obj = {};
29+
// Use createMap() to prevent class assumptions involving property names in
30+
// Object.prototype
31+
var obj = createMap();
3032
forEach(classes, function(klass) {
3133
// sometimes the split leaves empty string values
3234
// incase extra spaces were applied to the options

src/ng/directive/ngClass.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ function classDirective(name, selector) {
3939
}
4040

4141
function digestClassCounts(classes, count) {
42-
var classCounts = element.data('$classCounts') || {};
42+
// Use createMap() to prevent class assumptions involving property
43+
// names in Object.prototype
44+
var classCounts = element.data('$classCounts') || createMap();
4345
var classesToUpdate = [];
4446
forEach(classes, function(className) {
4547
if (count > 0 || classCounts[className]) {

test/ng/directive/ngClassSpec.js

+25
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,31 @@ describe('ngClass', function() {
3333
}));
3434

3535

36+
it('should add new and remove old classes with same names as Object.prototype properties dynamically', inject(function($rootScope, $compile) {
37+
element = $compile('<div class="existing" ng-class="dynClass"></div>')($rootScope);
38+
$rootScope.dynClass = { watch: true, hasOwnProperty: true, isPrototypeOf: true };
39+
$rootScope.$digest();
40+
expect(element.hasClass('existing')).toBe(true);
41+
expect(element.hasClass('watch')).toBe(true);
42+
expect(element.hasClass('hasOwnProperty')).toBe(true);
43+
expect(element.hasClass('isPrototypeOf')).toBe(true);
44+
45+
$rootScope.dynClass.watch = false;
46+
$rootScope.$digest();
47+
expect(element.hasClass('existing')).toBe(true);
48+
expect(element.hasClass('watch')).toBe(false);
49+
expect(element.hasClass('hasOwnProperty')).toBe(true);
50+
expect(element.hasClass('isPrototypeOf')).toBe(true);
51+
52+
delete $rootScope.dynClass;
53+
$rootScope.$digest();
54+
expect(element.hasClass('existing')).toBe(true);
55+
expect(element.hasClass('watch')).toBe(false);
56+
expect(element.hasClass('hasOwnProperty')).toBe(false);
57+
expect(element.hasClass('isPrototypeOf')).toBe(false);
58+
}));
59+
60+
3661
it('should support adding multiple classes via an array', inject(function($rootScope, $compile) {
3762
element = $compile('<div class="existing" ng-class="[\'A\', \'B\']"></div>')($rootScope);
3863
$rootScope.$digest();

0 commit comments

Comments
 (0)