Bug 144367 – Array.from should work with document.all

• Component: WebCore Misc.
• Status: NEW

[Joseph Pecoraro]

* SUMMARY
Array.from should work with document.all.

* STEPS TO REPRODUCE
1. Inspect about:blank
2. js> Array.from(document.all)
  => TypeError: Array.from requires an array-like object - not null or undefined.

* NOTES
In perhaps the darkest corner of JavaScript, `document.all == null` is true and `typeof document.all === 'undefined'` is true. We can at least fix Array.from for this case, and test it with other collection types.

[Joseph Pecoraro]

Created attachment 251906 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Created attachment 251907 [details]
[PATCH] For Bots (requires other pending patches to pass this test)

[Geoffrey Garen]

Comment on attachment 251906 [details]
[PATCH] Proposed Fix

r=me

[Geoffrey Garen]

Actually, is this important? Do we think that authors writing new code will still use document.all? I don't think we should add any further accommodations for document.all unless compatibility absolutely forces us to. It would be nice if Array.from encouraged a developer to stop using document.all, if this ever came to pass.

[Ryosuke Niwa]

It seems like Firefox DOES support this.

[Build Bot]

Comment on attachment 251906 [details]
[PATCH] Proposed Fix

Attachment 251906 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5911993080872960

New failing tests:
fast/dom/collection-to-array.html

[Build Bot]

Created attachment 251908 [details]
Archive of layout-test-results from ews103 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Joseph Pecoraro]

(In reply to comment #4)
> Actually, is this important? Do we think that authors writing new code will
> still use document.all? I don't think we should add any further
> accommodations for document.all unless compatibility absolutely forces us
> to. It would be nice if Array.from encouraged a developer to stop using
> document.all, if this ever came to pass.

I don't think it is important.

Reasons why I considered it:

  - easy fix, doesn't change the semantics of the code at all
  - Firefox supports Array.from and seems to handle this just fine
  - if not fixed, this is an Exception with an unexpected error message

However, I'm more than happy not making the change and instead pushing to removing document.all. I know inspector code has all kinds of code to special case that collection and its unusual typing.

[Build Bot]

Comment on attachment 251906 [details]
[PATCH] Proposed Fix

Attachment 251906 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/5660502344597504

New failing tests:
fast/dom/collection-to-array.html

[Build Bot]

Created attachment 251911 [details]
Archive of layout-test-results from ews105 for mac-mavericks-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews105  Port: mac-mavericks-wk2  Platform: Mac OS X 10.9.5

[Ryosuke Niwa]

Unfortunately, there's no way we can remove document.all. The entire Web depends on it.

[Geoffrey Garen]

I'm not quite suggesting that we remove document.all; rather, I'm suggesting that we not add new features or workarounds for it, since our existing features and workarounds exist only for backwards compatibility. 

That being said, our other Array functions use explicit independent checks for null and undefined, so I guess it's a good idea to do that in .from as well, all other things being equal.