Bug 11529 - under js1.2, Array object should pop the last element as Number object's argument
Summary: under js1.2, Array object should pop the last element as Number object's argu...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Macintosh OS X 10.4
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-11-06 09:22 PST by digdog
Modified: 2007-09-30 03:40 PDT (History)
0 users

See Also:


Attachments
identifier.h (703 bytes, patch)
2006-11-06 09:26 PST, digdog
no flags Details | Formatted Diff | Diff
object.cpp (863 bytes, patch)
2006-11-06 09:26 PST, digdog
no flags Details | Formatted Diff | Diff
Patch (1.99 KB, patch)
2006-11-06 17:08 PST, digdog
mjs: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description digdog 2006-11-06 09:22:46 PST
Currently, the result will be correct if Number object uses Array object as argument, and the Array object has only one element.

If array length is larger than one, the argument will become NaN. This is due to Array object will be first converted into ValueOf and later ToString, e.g. "element1, element2".

The correct way based on js1.2 is to pop the last element if Array elements if more than one.

Reference:
/WebKit/JavaScriptCore/tests/mozilla/js1_2/function/Number.js Ln65
Comment 1 digdog 2006-11-06 09:26:17 PST
Created attachment 11398 [details]
identifier.h
Comment 2 digdog 2006-11-06 09:26:54 PST
Created attachment 11399 [details]
object.cpp
Comment 3 digdog 2006-11-06 09:27:29 PST
I try to fix it by added "pop" as Identifier's special prototype property name, and use it in *JSObject::defaultValue()
Comment 4 digdog 2006-11-06 09:31:55 PST
Should open for review.
Comment 5 digdog 2006-11-06 17:08:24 PST
Created attachment 11408 [details]
Patch
Comment 6 digdog 2006-11-06 17:24:05 PST
However, this proposal seems conflict with ECMA-262v3 specs 9.3.1 & 8.6.2.6. 

Cause Object should call ToPrimitive first then ToNumber later after, and ToPrimitive was calling valueOf and toString. However, the Array prototype does not have valueOf, so it inherits the property from Object prototype.

I am not sure if the valueOf [1,2,3] should be "1,2,3" or [object Array], and it will be "1,2,3" before this patch, and "3" after the patch applied. 
Comment 7 Maciej Stachowiak 2006-11-06 20:43:05 PST
If this change would result in violating the spec, I'd like to know:

a) Do other major browsers (IE, Opera) also have this nonstandard behavior?
b) Are there any sites that depend on the nonstandard behavior?
Comment 8 digdog 2006-11-07 05:08:39 PST
No and no. 

Mozilla SpiderMonkey (ES3) / Rinho (ES3), IE7 JScript.NET (ES4) and Opera (ES1) don't act like this, they all return NaN when array elements are greater than one. 

And since this is based on js1.2 and gone after that, I don't see any website using scripts in this way for a while.
Comment 9 Sam Weinig 2006-11-07 15:37:09 PST
If none of the other browsers do this, no websites depend on the behavior and it violates the current specification, what is the rationale for making the change?  If it's just to satisfy the test you mentioned, perhaps a better solution would be to change the test.
Comment 10 Maciej Stachowiak 2006-11-07 21:15:53 PST
Comment on attachment 11408 [details]
Patch

Since this is against the spec and no other browsers do this, we should not make this change. It would be better to change or disable the test. Mozilla probably tests this because their JS interpreter has modes to request legacy behavior of old JS versions - we don't implement that and don't plan to.
Comment 11 digdog 2006-11-08 02:21:52 PST
I see. I will try to identify the failed testcases (of previous version, e.g. js1_2) that violate current spec, and see if we should close these tests.
Comment 12 Andrew Wellington 2007-09-30 03:40:25 PDT
This was fixed by r18071 fixing bug #11545