Closed
Bug 206353
Opened 21 years ago
Closed 21 years ago
DOM Lists (NodeList, HTMLCollection, etc) should be enumerable
Categories
(Core :: DOM: Core & HTML, enhancement)
Tracking
()
RESOLVED
FIXED
People
(Reporter: caillon, Assigned: jst)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
caillon
:
review+
peterv
:
superreview+
|
Details | Diff | Splinter Review |
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•21 years ago
|
||
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•21 years ago
|
||
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.
Reporter | ||
Comment 3•21 years ago
|
||
That's why I filed this in extensions. :-)
Comment 4•21 years ago
|
||
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•21 years ago
|
||
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...
Reporter | ||
Comment 6•21 years ago
|
||
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•21 years ago
|
||
I seem to recall an existing bug on this issue...
Comment 8•21 years ago
|
||
Its effects on the DOM inspector _would_ be nice.
Assignee | ||
Comment 9•21 years ago
|
||
Assignee | ||
Comment 10•21 years ago
|
||
Attachment #127772 -
Attachment is obsolete: true
Updated•21 years ago
|
Attachment #127773 -
Flags: review?(caillon)
Reporter | ||
Comment 11•21 years ago
|
||
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;
>+}
Attachment #127773 -
Flags: review?(caillon) → review+
Updated•21 years ago
|
Attachment #127773 -
Flags: superreview?(peterv)
Updated•21 years ago
|
Attachment #127773 -
Flags: superreview?(peterv) → superreview+
Reporter | ||
Comment 12•21 years ago
|
||
Btw, ignore my previous comment. I see that you are via the condition in the
for-loop.
Comment 13•21 years ago
|
||
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) ?
Reporter | ||
Comment 14•21 years ago
|
||
The latter would work with this patch, but it really seems like you want
if (document.getElementsByTagName("editor").length > 0) ;
Assignee: caillon → jst
Comment 15•21 years ago
|
||
.length is slow (nodelists are lazy, remember?) if you only care about item 0.
Reporter | ||
Comment 16•21 years ago
|
||
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•21 years ago
|
||
Is this bug's patch ready for checkin?
Comment 18•21 years ago
|
||
Fix checked in on jst's behalf.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Comment 19•19 years ago
|
||
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
Updated•12 years ago
|
Component: DOM: Mozilla Extensions → DOM
Updated•6 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•