Bug 83305
Summary: | toNativeArray should not require a real array | ||
---|---|---|---|
Product: | WebKit | Reporter: | Erik Arvidsson <arv> |
Component: | DOM | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED CONFIGURATION CHANGED | ||
Severity: | Normal | CC: | adamk, ahmad.saleem792, ap, bfulgham, code.vineet, heycam, rniwa |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
URL: | http://www.w3.org/TR/WebIDL/#es-sequence |
Erik Arvidsson
Our toNativeArray checks if the value is a real array. This is not correct.
Cameron: The WebIDL spec here seems like it could use some improvements to better behave like the generic Array methods in JS.
1. Initialize i to be 0.
2. Let A be the result of calling ToObject(V).
3. Let length be the result of calling [[Get]] on A with property name “length”.
4. Let n be the result of calling ToUint32(length).
5. Initialize S0..n−1 to be an IDL sequence with elements of type T, where each element is uninitialized.
6. While i < n:
1. Let P be the result of calling ToString(i).
2. Let E be the result of calling [[Get]] on A with property name P.
3. Set Si to the result of converting E to an IDL value of type T.
4. Set i to i + 1.
7. Return S.
Note that ToObject throws a TypeError if V is null or undefined according to 9.9 of ES5.1 spec
We could add a step between 2 and 3 to check if the object has a "length" property. It is not what ES does but it might lead to less confusion.
Attachments | ||
---|---|---|
Add attachment proposed patch, testcase, etc. |
Cameron McCormack (:heycam)
Erik, sorry I don't follow what your suggestion for making the #es-sequence conversion behave more like generic Array methods. Do you mean accepting any object, and not just an Array or platform object with indexed properties or platform array object? The spec used to say this some time ago, but not since I wanted to allow overloads between sequences and dictionaries, such as:
void f(SomeDict x);
void f(sequence<T> x);
You're right that I could check for a "length" property, but so far overload resolution does not look at the properties on an object at all before deciding which overload to call. It would also make it confusing what to do when the dictionary type has a "length" member too. Well I guess we could then disallow the overloading in that case, but this seems to be complicating things.
Erik Arvidsson
(In reply to comment #1)
> Do you mean accepting any object, and not just an Array or platform object with indexed properties or platform array object?
Yes. The following object should work as an argument to a sequence<T>:
{length: 1, 0: someT}
That is how the Array methods works in ES. Look at the spec for Array.prototype.slice for example.
Cameron McCormack (:heycam)
OK. Well, as I say it was a conscious decision, the current behaviour. With the more simplified overload resolution we've got now, though, it might not be so terrible to try and get this working again. Would you mind starting a thread on public-script-coord about it? Thanks.
Ahmad Saleem
I looked into Chrome bug related to this:
https://bugs.chromium.org/p/chromium/issues/detail?id=263758
and from this commit - https://src.chromium.org/viewvc/blink?view=revision&revision=154940
Took the test case and changed it into this JSFiddle:
Link - https://jsfiddle.net/me512rh4/show
Safari 15.6 is passing same test as Chrome Canary 106 and Firefox Nightly. I think nothing further is required here. Hence, I am going to mark this as "RESOLVED CONFIGURATION CHANGED" but if someone further is required or I am wrong, please reopen and mark / tag this bug accordingly. Thanks!