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)

x86
Linux
enhancement
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: caillon, Assigned: jst)

Details

Attachments

(1 file, 1 obsolete file)

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.
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"?
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.
That's why I filed this in extensions. :-)
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.
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...
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.
I seem to recall an existing bug on this issue...
Its effects on the DOM inspector _would_ be nice.
Attached patch Make array items enumerable (obsolete) (deleted) — Splinter Review
Attached patch Make array items enumerable (deleted) — Splinter Review
Attachment #127772 - Attachment is obsolete: true
Attachment #127773 - Flags: review?(caillon)
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+
Attachment #127773 - Flags: superreview?(peterv)
Attachment #127773 - Flags: superreview?(peterv) → superreview+
Btw, ignore my previous comment. I see that you are via the condition in the for-loop.
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) ?
The latter would work with this patch, but it really seems like you want if (document.getElementsByTagName("editor").length > 0) ;
Assignee: caillon → jst
.length is slow (nodelists are lazy, remember?) if you only care about item 0.
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.
Is this bug's patch ready for checkin?
Fix checked in on jst's behalf.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
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
Component: DOM: Mozilla Extensions → DOM
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: