Bug 11529 – under js1.2, Array object should pop the last element as Number object's argument

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

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

Created attachment 11398 [details]
identifier.h

Created attachment 11399 [details]
object.cpp

I try to fix it by added "pop" as Identifier's special prototype property name, and use it in *JSObject::defaultValue()

Should open for review.

Created attachment 11408 [details]
Patch

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. 

[Maciej Stachowiak]

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?

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.

[Sam Weinig]

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.

[Maciej Stachowiak]

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.

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.

[Andrew Wellington]

This was fixed by r18071 fixing bug #11545