57048 - propertyIsEnumerable(): Ecmascript compliance bug

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[David Flanagan]

Section 15.2.4.7 of the ECMA3 spec says that propertyIsEnumerable 
"does not consider objects in the prototype chain".

Personally, I think this is a mistake in the specification, because
the function isn't as useful if it ignores that prototype chain.

However, spidermonkey does not conform to the spec as it is written.
Note that Rhino does conform.  Here is an illustration:

js> String.prototype.foo = "bar";
bar
js> s = "test"
test
js> s.propertyIsEnumerable("foo")
true
js> function Foo() { this.x = 1; }
js> Foo.prototype.y = 2;
2
js> var f = new Foo();
js> f.propertyIsEnumerable("x");
true
js> f.propertyIsEnumerable("y");
true

Comment 1

[Phil Schwartau]

From the ECMA3 spec:

15.2.4.7 Object.prototype.propertyIsEnumerable (V)

When the propertyIsEnumerable method is called with argument V, the following 
steps are taken:

1. Let O be this object.
2. Call ToString(V).
3. If O doesn’t have a property with the name given by Result(2), return false .
4. If the property has the DontEnum attribute, return false .
5. Return true.

NOTE This method does not consider objects in the prototype chain.

Comment 2

[Brendan Eich [:brendan]]

We believe the spec suffered a draft error that no one caught, and that the 
intention was for propertyIsEnumerable to match the for..in loop, and check the 
prototype chain as well as the object at hand.  I think we should WONTFIX this 
bug and fix the spec, but I'll confirm and reassign to waldemar for ultimate 
disposition.

/be

Comment 3

[David Flanagan]

A quick check of a newly downloaded copy of IE 5.5 makes it
look like MS implements the spec as written, rather than
as intended.

javascript:String.prototype.foo = 'bar';s='foo';s.propertyIsEnumerable('foo')

evaluates to false in IE5.5

Comment 4

[Waldemar Horwat]

I discussed this at the last ECMA meeting and the decision was to implement the 
spec as written, even if it's flawed in this case.  Spidermonkey is correct; 
Rhino should be updated.

Comment 5

[Phil Schwartau]

Reassigning to Rhino per Waldemar's comment above -

Comment 6

[Norris Boyd]

Reassigning back--

Rhino's matches the spec as written, SpiderMonkey does not.

Here's Rhino's output: 
js> String.prototype.foo = "bar";
bar
js> s = "test"
test
js> s.propertyIsEnumerable("foo")
false
js> function Foo() { this.x = 1; }
js> Foo.prototype.y = 2;
2
js> var f = new Foo();
js> f.propertyIsEnumerable("x");
true
js> f.propertyIsEnumerable("y");
false
js>

Comment 7

[Phil Schwartau]

[MID-AIR COLLISION] 

Norris is correct. As David reported, it's SpiderMonkey that goes up 
the prototype chain for propertyIsEnumerable(), against spec. 


Confirmed with Waldemar -

Comment 8

[Mike McCabe]

Attachment: Make propertyIsEnumerable do a hasOwnProeprty check

Comment 9

[rogerl (gone)]

r=rogerl

Comment 10

[Mike McCabe]

Fix checked in.

Comment 11

[Brendan Eich [:brendan]]

Who sr='d this change?

I still think ECMA is wrong, and should be fixed -- it's not as if anyone uses
this feature yet.

/be