If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

enumeration of string characters fails; propertyIsEnumerable says false

VERIFIED FIXED in mozilla1.2beta

Status

()

Core
JavaScript Engine
P3
normal
VERIFIED FIXED
15 years ago
15 years ago

People

(Reporter: Georg Maaß, Assigned: brendan)

Tracking

({js1.5})

Trunk
mozilla1.2beta
x86
Windows 2000
js1.5
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

15 years ago
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

Comment 1

15 years ago
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

Comment 2

15 years ago
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
(Assignee)

Comment 3

15 years ago
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
(Assignee)

Comment 4

15 years ago
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
(Assignee)

Comment 5

15 years ago
Created attachment 98816 [details] [diff] [review]
proposed fix

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 6

15 years ago
Comment on attachment 98816 [details] [diff] [review]
proposed fix

r=rogerl. Also ran test suite fine.
Attachment #98816 - Flags: review+
(Assignee)

Comment 7

15 years ago
Created attachment 98919 [details] [diff] [review]
common macro: SPROP_IS_SHARED_PERMANENT; obj2 fix in propertyIsEnumerable

I'm gonna check this one in.

/be
Attachment #98816 - Attachment is obsolete: true
(Assignee)

Comment 8

15 years ago
Fixed.

/be
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 9

15 years ago
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
(Reporter)

Comment 10

15 years ago
Thanks for repairing this so fast.
You need to log in before you can comment on or make changes to this bug.