string.length should not be generic

VERIFIED FIXED in mozilla1.9alpha1

Status

()

Core
JavaScript Engine
P2
normal
VERIFIED FIXED
13 years ago
12 years ago

People

(Reporter: Igor Bukanov, Assigned: brendan)

Tracking

({verified1.8.1})

Trunk
mozilla1.9alpha1
verified1.8.1
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Reporter)

Description

13 years ago
From ECMA-262:

--------------------------------------------
15.5.5    Properties of String Instances
...
The [[Value]] property is the string value represented by this String object.
 
15.5.5.1    length

The number of characters in the String value represented by this String object. Once a String object is created, this property is unchanging. It has the attributes { DontEnum, DontDelete, ReadOnly }.
--------------------------------------------

AFAICS this text implies that "length" property is the length of the string passed to "String" constructor and in particular for any string instance "s" s.valueOf().length == s.length unless valueOf is overwritten.

On the other hand in SpiderMonkey "length" property is generic and is defined to be the length of the string resulted from conversion of the object to string. The latter can be overwritten,for example, with custom toString method.

Here is a test case that demonstrates the difference between SpiderMonkey and ECMA required behavior:

var s = new String("1");
s.toString = function() {
    return "22";
}
var ecma_expected_length = 1;
var actual = s.length;
print("ecma_expected_length="+ecma_expected_length+" actual="+actual);

Currentl in js shell it prints 
ecma_expected_length=1 actual=2

where actual=2 comes from "22" returned by custom toString method.
(Assignee)

Comment 1

13 years ago
Created attachment 200617 [details] [diff] [review]
possible fix

The jsobj.c change fixes a bug that was supposed to have been fixed already (old unreported bug, see rev 3.167 of jsobj.c).

The effect of the jsstr.c changes is to allow this to work:

var o = {__proto__: new String("Hello"), toString: function(){return "Bye"}};

print(o[0], o[1]);  // H e
print(o.length);    // 3
print(o);           // Bye

Of course, uneval(o) throws TypeError: String.prototype.toSource called on incompatible Object.

I'm trying to balance backward compatibility with following the spec.  Always a ton of fun.  Comments?

/be
Attachment #200617 - Flags: superreview?(shaver)
Attachment #200617 - Flags: review?(igor.bukanov)
(Assignee)

Comment 2

13 years ago
Comment on attachment 200617 [details] [diff] [review]
possible fix

The JSVAL_IS_VOID(v) tests on the private data value are not necessary.

/be
Comment on attachment 200617 [details] [diff] [review]
possible fix

sr=shaver
Attachment #200617 - Flags: superreview?(shaver) → superreview+
(Assignee)

Updated

13 years ago
Assignee: general → brendan
OS: Linux → All
Priority: -- → P2
Hardware: PC → All
Target Milestone: --- → mozilla1.9alpha
(Assignee)

Comment 4

13 years ago
Created attachment 200661 [details] [diff] [review]
possible fix, v2
Attachment #200617 - Attachment is obsolete: true
Attachment #200661 - Flags: superreview+
Attachment #200661 - Flags: review?(igor.bukanov)
Attachment #200617 - Flags: review?(igor.bukanov)
(Reporter)

Updated

13 years ago
Attachment #200661 - Flags: review?(igor.bukanov) → review+
(Assignee)

Comment 5

13 years ago
Fixed on trunk.  I'm trying not to fix this for 1.8/fx1.5/js1.6 -- if someone thinks it should be fixed on that branch, please nominate.

/be
Status: NEW → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Checking in regress-313567.js;
/cvsroot/mozilla/js/tests/ecma_3/String/regress-313567.js,v  <--  regress-313567.js
initial revision: 1.1
Flags: testcase+
verified fixed cvs debug trunk builds windows/linux
Status: RESOLVED → VERIFIED
(Assignee)

Comment 8

12 years ago
*** Bug 340940 has been marked as a duplicate of this bug. ***
(Assignee)

Comment 9

12 years ago
*** Bug 340534 has been marked as a duplicate of this bug. ***
fixed by Bug 336373 on the 1.8.1 branch. 
verified fixed 1.8.1 with windows/macppc/linux 20060707
Keywords: verified1.8.1
You need to log in before you can comment on or make changes to this bug.