206353 - DOM Lists (NodeList, HTMLCollection, etc) should be enumerable

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[Christopher Aillon (sabbatical, not receiving bugmail)]

var nodes = document.getElementsByTagName("p");
for (var p in nodes) {
  // foo
}

Although unspecified, the above should work since we treat those things like JS
objects/arrays.

Comment 1

[Hixie (not reading bugmail)]

I would love it if that worked, but how legitimate is it? If I use it in a
script, will IE-evangelists come to me and say "what you are doing is non-standard"?

Comment 2

[Alex Vincent [:WeirdAl]]

Um, Chris:  they do work for the DOM-defined properties.  I get back
"namedItem", "item" and "length".

However, the numbered properties (such as the first item of document.childNodes)
are not enumerable.  If you're proposing to make them enumerable, as our
discussion implies, then you've got my vote.

Comment 3

[Christopher Aillon (sabbatical, not receiving bugmail)]

That's why I filed this in extensions.  :-)

Comment 4

[Hixie (not reading bugmail)]

I guess my question is just "what's the point of this if authors can't use it",
then. Seems like the only uses for this would be in chrome JS. I love the idea,
I'm just worried we'd be leading authors into doing the wrong thing.

Comment 5

[Alex Vincent [:WeirdAl]]

I can answer that one, Ian.  DOM Inspector currently doesn't show the numbered
properties of such lists; all we get in the JSObject panel is the enumerated
properties.

Try navigating via JavaScript Object panel from the document node to a paragraph
node in HTML.  You just can't do it.  You can do it from the left-hand panel in
DOM Node...

Comment 6

[Christopher Aillon (sabbatical, not receiving bugmail)]

Is this the sort of thing that the DOM WG can specify?  Currently, it's not
specified.  If so, I think that it should definitely be brought up to the WG. 
It would be useful for chrome for sure.  And with IE's market share, I can't see
it being a problem that someone uses this and breaks IE/other browsers if they
need it to be interoperable.

Comment 7

[Boris Zbarsky [:bzbarsky]]

I seem to recall an existing bug on this issue...

Comment 8

[Hixie (not reading bugmail)]

Its effects on the DOM inspector _would_ be nice.

Comment 9

[Johnny Stenback (:jst)]

Attachment: Make array items enumerable

Comment 10

[Johnny Stenback (:jst)]

Attachment: Make array items enumerable

Comment 11

[Christopher Aillon (sabbatical, not receiving bugmail)]

Comment on attachment 127773 [details] [diff] [review]
Make array items enumerable


>+  if (ok && JSVAL_IS_INT(len_val)) {
>+    PRInt32 length = JSVAL_TO_INT(len_val);
>+    char buf[11];
>+
>+    for (PRInt32 i = 0; ok && i < length; ++i) {
>+      PR_snprintf(buf, sizeof(buf), "%d", i);
>+
>+      ok = ::JS_DefineProperty(cx, obj, buf, JSVAL_VOID, nsnull, nsnull,
>+                               JSPROP_ENUMERATE);

Do you maybe want to break out of this loop if (!ok) ?	Looks fine otherwise,
r=caillon

>+    }
>+  }
>+
>+  sCurrentlyEnumerating = PR_FALSE;
>+
>+  return ok ? NS_OK : NS_ERROR_UNEXPECTED;
>+}

Comment 12

[Christopher Aillon (sabbatical, not receiving bugmail)]

Btw, ignore my previous comment.  I see that you are via the condition in the
for-loop.

Comment 13

[[e-mail]]

Hey, is this in anyway related to the hack that bz had to do for editor, i.e.
if (document.getElementsByTagName("editor").item(0))
instead of
if (0 in document.getElementsByTagName("editor"))
(well, paraphrased) ?

Comment 14

[Christopher Aillon (sabbatical, not receiving bugmail)]

The latter would work with this patch, but it really seems like you want

if (document.getElementsByTagName("editor").length > 0) ;

Comment 15

[Boris Zbarsky [:bzbarsky]]

.length is slow (nodelists are lazy, remember?) if you only care about item 0.

Comment 16

[Christopher Aillon (sabbatical, not receiving bugmail)]

Well if perf is an issue here (bz assures me it is), then you probably don't
want to use this since it will end up iterating twice over the length.

Comment 17

[Alex Vincent [:WeirdAl]]

Is this bug's patch ready for checkin?

Comment 18

[[e-mail]]

Fix checked in on jst's behalf.

Comment 19

[Brendan Eich [:brendan]]

This indirectly caused bug 310351 -- just FYI, if you add an enumerate hook to
reflect eagerly all the lazy properties, you need a resolve (JSCLASS_NEW_RESOLVE
style, preferably) hook to reflect individual lazy properties on demand.

One thing that could be more efficient in the patch: use JS_DefineElement if
INT_FITS_IN_JSVAL(i), avoiding sprintf to a buffer, atomization, etc.

/be