New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 27 users

Issue metadata

Status: Duplicate
Merged: issue 401699
Owner: ----
Closed: Apr 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Bug

Blocked on:
issue 235008



Sign in to add a comment

Add support for [ArrayClass] and use that on NodeList

Project Member Reported by arv@chromium.org, Apr 9 2013

Issue description

From https://bugs.webkit.org/show_bug.cgi?id=81573

http://dom.spec.whatwg.org/#interface-nodelist

DOM4 specs NodeList as having Array.prototype in its prototype chain.


Some more background for this one. [ArrayClass] allows us to do things like document.querySelectorAll('.foo').forEach etc. The patch at bugs.webkit.org has a runtime flag because it is unclear if this will still be possible to achieve.

High gain but also high risk.
 

Comment 1 by arv@chromium.org, May 6 2013

Blockedon: chromium:235008

Comment 2 Deleted

Comment 3 by arv@chromium.org, Aug 2 2014

NodeList.prototype.length is read only so using Array.prototype.push and other functions that needs to mutate the object would not correctly work. They would set indexed properties but length cannot be updated.

Comment 4 Deleted

Why does blink-in-JS need to mutate NodeLists? We never mutate them in C++,
we shouldn't do that in JS either.

Comment 6 Deleted

This bug is unrelated to Blink-in-JS.

> the alternative is invoking C++ routines which do the work instead

Another alternative is to not use Blink-in-JS for whatever feature needs this ability.
> This bug is unrelated to Blink-in-JS.

I think we all agree on that. What I'm asking is a question: given that the Array is an exotic object, child classes of the Array should likely inherit those special behaviours (such as certain operations affecting the `length` property), would this have the side-effect of making these collections possible to be populated in a script context?

If the answer to that question is "no, because these collection prototypes all have a readonly `length` property in their IDL", then the follow up question becomes "why?".

Both of these are, I think, valid questions. Blink-in-JS comes to mind because certain features are completely unimplementable in JS given these limitations (obviously the set of features unimplementable in JS is not limited to these), but this is really a valid question which deserves an answer anyways.

I'm somewhat reminded of the script-coord thread regarding exposing constructors for *ReadOnly interfaces to client scripts, because I think it would be useful for a client to be able to populate such a collection. Useful for implementing engine features in JS (though yes, I agree that this doesn't apply to all features), useful for testing, useful for correctly polyfilling future DOM apis, et cetera.

But, it is just a question regarding this bug, not a bug, not a feature request, not a suggestion that this is a blocker for other work.

Comment 9 by bruan...@gmail.com, Aug 4 2014

> certain features are completely unimplementable in JS given these limitations

There are implementable in ES6 with Proxies. If you notice something that is not, please post to public-script-coord.
Hey arv, are you still planning on doing this? Should I just add Symbol.iterator support (legacyiterable<>) through IDL and call it a day? Or should we try (iterable<>) so forEach() and keys() and such are also exposed but with more risk?

Comment 11 by arv@chromium.org, Oct 3 2014

I have no intent on working on this any time soon.

iterable<> sounds safer and better for the future.
Okay, NodeList no longer has [ArrayClass], but it has iterable<> now.

Comment 13 by arv@chromium.org, Jun 29 2015

Owner: ----
So the WHATWG's DOM spec got rid of the [ArrayClass] on NodeList a while ago, in favour of iterable declarations https://github.com/whatwg/dom/commit/72a2f99e968571989790d58f5b35e8465c52a462

Maybe it's worth supporting the [ArrayClass] attribute regardless, just in case it's ever needed (it's not a whole lot of dead code), and work on supporting iterable declarations for NodeList?

Comment 15 by d@domenic.me, Jul 1 2015

I think there's interest in [ArrayClass] if we can get away with it, but last time we tried it failed due to concat-related things. It depends on implementers being adventurous and trying things (like [ArrayClass] + isConcatSpreadable hacks, or even just trying [ArrayClass] by itself since maybe those sites have fixed themselves). Less-adventurous implementers can just do iterable NodeList for a quick payoff.

Comment 16 by ojan@chromium.org, Jul 1 2015

FYI, the concat-related issue was in the Closure library, which was fixed a long time ago. So, it might be worth trying again for [ArrayClass] if someone has the time to give it a go.
See https://crrev.com/1214693007 --- haraken isn't keen on it at the moment, but I guess there are a few other interfaces spec'd to use it (MediaList for one, DOMRectList for another which is not yet implemented in Blink), so I feel like someone might get some use out of it.
Just to clarify: I'm fine with it if there is a use case. I'm just not really happy about introducing dead code to the code base :)


Cc: -dpranke@chromium.org
Labels: hotlist-trend-0701
FWIW, it's no longer spec'd as such. [1] Although, the preferred future replacement, Elements, is spec'd to be an actual Array subclass. [2]

[1]: https://dom.spec.whatwg.org/#interface-nodelist
[2]: https://dom.spec.whatwg.org/#element-collections

Comment 22 by Deleted ...@, Sep 1 2015

Should this be closed, or is someone interested in implementing [LegacyArrayClass] (as it is now called) and using it in other contexts?
I think we should try turning this on again. :)
Project Member

Comment 24 by bugdroid1@chromium.org, Oct 29 2015

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b

commit 9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b
Author: caitpotter88 <caitpotter88@gmail.com>
Date: Thu Oct 29 02:50:01 2015

[dom] support iterable<> NodeList

Adds missing iterable<Node> support to NodeList. This results in 4
additional enumerable properties on the interface:

- values()
- entries()
- keys()
- forEach()

The WebIDL spec wants each of these methods to return an ArrayIterator,
but for the time being these are still returning IDL Iterator types.

Ported from https://codereview.chromium.org/1220883007/

BUG= 229398 
LOG=N
R=jsbell@chromium.org, philipj@opera.com

Review URL: https://codereview.chromium.org/1367523002

Cr-Commit-Position: refs/heads/master@{#356744}

[add] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/LayoutTests/fast/dom/NodeList/nodelist-iterable.html
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/LayoutTests/fast/dom/domListEnumeration-expected.txt
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/LayoutTests/fast/dom/script-tests/domListEnumeration.js
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/LayoutTests/webexposed/global-interface-listing-expected.txt
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/Source/core/core.gypi
[add] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/Source/core/dom/NodeList.cpp
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/Source/core/dom/NodeList.h
[modify] http://crrev.com/9004adaa987fa68b8b4da6abfdc4669d6c8a3b9b/third_party/WebKit/Source/core/dom/NodeList.idl

Comment 25 by tkent@chromium.org, Apr 18 2016

Mergedinto: 401699
Status: Duplicate (was: Available)

Sign in to add a comment