Bug 144367 - Array.from should work with document.all
Summary: Array.from should work with document.all
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-28 18:00 PDT by Joseph Pecoraro
Modified: 2015-04-29 11:13 PDT (History)
5 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (6.35 KB, patch)
2015-04-28 18:03 PDT, Joseph Pecoraro
ggaren: review+
buildbot: commit-queue-
Details | Formatted Diff | Diff
[PATCH] For Bots (requires other pending patches to pass this test) (34.79 KB, patch)
2015-04-28 18:04 PDT, Joseph Pecoraro
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews103 for mac-mavericks (605.22 KB, application/zip)
2015-04-28 18:17 PDT, Build Bot
no flags Details
Archive of layout-test-results from ews105 for mac-mavericks-wk2 (639.88 KB, application/zip)
2015-04-28 18:32 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2015-04-28 18:00:56 PDT
* 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.
Comment 1 Joseph Pecoraro 2015-04-28 18:03:53 PDT
Created attachment 251906 [details]
[PATCH] Proposed Fix
Comment 2 Joseph Pecoraro 2015-04-28 18:04:37 PDT
Created attachment 251907 [details]
[PATCH] For Bots (requires other pending patches to pass this test)
Comment 3 Geoffrey Garen 2015-04-28 18:05:11 PDT
Comment on attachment 251906 [details]
[PATCH] Proposed Fix

r=me
Comment 4 Geoffrey Garen 2015-04-28 18:06:16 PDT
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.
Comment 5 Ryosuke Niwa 2015-04-28 18:16:35 PDT
It seems like Firefox DOES support this.
Comment 6 Build Bot 2015-04-28 18:17:23 PDT
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
Comment 7 Build Bot 2015-04-28 18:17:28 PDT
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
Comment 8 Joseph Pecoraro 2015-04-28 18:22:06 PDT
(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.
Comment 9 Build Bot 2015-04-28 18:32:45 PDT
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
Comment 10 Build Bot 2015-04-28 18:32:48 PDT
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
Comment 11 Ryosuke Niwa 2015-04-28 19:04:37 PDT
Unfortunately, there's no way we can remove document.all. The entire Web depends on it.
Comment 12 Geoffrey Garen 2015-04-29 11:13:15 PDT
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.