Closed Bug 167910 Opened 22 years ago Closed 22 years ago

enumeration of string characters fails; propertyIsEnumerable says false

Categories

(Core :: JavaScript Engine, defect, P3)

x86
Windows 2000
defect

Tracking

()

VERIFIED FIXED
mozilla1.2beta

People

(Reporter: georg, Assigned: brendan)

Details

(Keywords: js1.5)

Attachments

(1 file, 1 obsolete file)

Tested with Mozilla 1.0rc2 and Mozilla1.1 Gecko/20020826

s='test';
alert(s.propertyIsEnumerable(2));

This alerts false.

for(i in s) alert(i+': '+s[i]);

does nothing.

This is not compatible with earlier implementations of JavaScript 1.5 and also
breaks compatibility with JavaScript 1.2 to 1.4.

In NN4 you could enumerate the characters of a string. In earlier Mozillas and
also in earlier implementations of the stand alone JavaScript shell (like
js1.5r3) you could enumerate them too.

js> s='test';
test
js> print(s.propertyIsEnumerable(2));
true
js> for(i in s) alert(i+': '+s[i]+'\n');
0: t
1: e
2: s
3: t

(results known from the js-shell js1.5r3)

This should be re enabled for backward compatibility. Also
alert(s.propertyIsEnumerable(2)); then should alert true.

Both enabling enumeration and disabling it are ecma conform. Supporting the
index operator is simply an extension to ecma. As long as this useful extension
is supported also enumeration for the characters should be supported especially
because it has been supported in JavaScript 1.2 until earlier builds of
JavaScript 1.5
Confirming report:

JS1.5 RC3 (07 Mar 2001)  js> 'test'.propertyIsEnumerable(2)
                         true
JS1.5 RC4 (01 Jan 2002)  js> 'test'.propertyIsEnumerable(2)
                         false


Reassigning to Kenton; cc'ing Brendan. I will query Bugzilla
to find why this was done -
Assignee: rogerl → khanson
Status: UNCONFIRMED → NEW
Ever confirmed: true
This may help explain things:

JS1.5 RC3 (07 Mar 2001)  js> 'test'.hasOwnProperty(2)
                         true
JS1.5 RC4 (01 Jan 2002)  js> 'test'.hasOwnProperty(2)
                         false


Now recall bug 57048,
"propertyIsEnumerable(): EcmaScript compliance bug"

---
david@oreilly.com:

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. 
---


The fix for that bug was:


------- Additional Comment_ #8 From Mike McCabe 2000-11-21 18:17 ------- 

Created an attachment (id=19598)
Make propertyIsEnumerable do a hasOwnProperty check
This is a non-ECMA extension, but still, I'd rather not change things if it can
be helped.

/be
Assignee: khanson → brendan
Keywords: js1.5, mozilla1.2
Priority: -- → P3
Target Milestone: --- → mozilla1.2beta
Bug 56940 made a number of changes, one of which broke compatibility here with
our ECMA extension.  Phil, the mccabe change predates that fix by quite a bit,
and is superceded by other changes in jsobj.c -- FYI, no worries.

Patch in a few.

/be
Status: NEW → ASSIGNED
Attached patch proposed fix (obsolete) — Splinter Review
Give up the perf win of bug 56940's patch, which removed str_resolve.  It's not
worth the incompatibility and ugliness to script authors.

Can I get an r= for 1.2beta?  Thanks.

/be
Comment on attachment 98816 [details] [diff] [review]
proposed fix

r=rogerl. Also ran test suite fine.
Attachment #98816 - Flags: review+
I'm gonna check this one in.

/be
Attachment #98816 - Attachment is obsolete: true
Fixed.

/be
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED
Verified FIXED in current trunk debug and optimized JS shell:

js> 'test'.hasOwnProperty(2)
true

js> 'test'.propertyIsEnumerable(2)
true

js> var s = 'test'
js> for (var i in s) {print(i + ': ' + s[i]);}
0: t
1: e
2: s
3: t
Status: RESOLVED → VERIFIED
Thanks for repairing this so fast.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: