Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(angular.equals): do not match keys defined in the prototype chain
Browse files Browse the repository at this point in the history
Merely testing for object[key] will give incorrect results on keys
defined in Object.prototype.
Note: IE8 is generally broken in this regard since `for...in` never returns
certain property keys even if they are defined directly on the object.

See #2141 - partially merges this PR
  • Loading branch information
mernen authored and petebacondarwin committed Jul 8, 2013
1 parent d88dc4a commit 7829c50
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 1 deletion.
12 changes: 11 additions & 1 deletion src/Angular.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,16 @@

////////////////////////////////////

/**
* hasOwnProperty may be overriden by a property of the same name, or entirely
* absent from an object that does not inherit Object.prototype; this copy is
* used instead
*/
var hasOwnPropertyFn = Object.prototype.hasOwnProperty;
var hasOwnPropertyLocal = function(obj, key) {
return hasOwnPropertyFn.call(obj, key);

This comment has been minimized.

Copy link
@rzschech

rzschech Jul 18, 2013

Contributor

Are these functions used? The code below doesn't seem to use them. Perhaps it should be?

This comment has been minimized.

Copy link
@petebacondarwin

petebacondarwin Jul 18, 2013

Member

They were there to be used but I was going to put it in a new PR

This comment has been minimized.

Copy link
@rzschech

rzschech Jul 18, 2013

Contributor

Ok thanks

};

/**
* @ngdoc function
* @name angular.lowercase
Expand Down Expand Up @@ -685,7 +695,7 @@ function equals(o1, o2) {
keySet[key] = true;
}
for(key in o2) {
if (!keySet[key] &&
if (!keySet.hasOwnProperty(key) &&
key.charAt(0) !== '$' &&
o2[key] !== undefined &&
!isFunction(o2[key])) return false;
Expand Down
8 changes: 8 additions & 0 deletions test/AngularSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,14 @@ describe('angular', function() {
expect(equals(new Date(0), 0)).toBe(false);
expect(equals(0, new Date(0))).toBe(false);
});

it('should correctly test for keys that are present on Object.prototype', function() {
// MS IE8 just doesn't work for this kind of thing, since "for ... in" doesn't return
// things like hasOwnProperty even if it is explicitly defined on the actual object!
if (msie<=8) return;
expect(equals({}, {hasOwnProperty: 1})).toBe(false);
expect(equals({}, {toString: null})).toBe(false);
});
});

describe('size', function() {
Expand Down

0 comments on commit 7829c50

Please sign in to comment.