Replace native mOuter aggregate cruft from nsXULElement with XBL magic.

hewitt could do this when he starts working on <listbox>. Want to reassign it to him?

I'd love to!

this is almost done but I'll wait until the opening of 0.9.7 to land it

I renamed clearItemSelection() to clearSelection, and converted multiple="true" to seltype="multiple", so this patch includes updates all xul/js uses in the tree.

Comment on attachment 58814 [details] [diff] [review] patch to content, dom, and tree.xml Sweet justice. Just a few questions: conditional sr=waterson if you've got answers that make sense. ;-) >+ nsCOMPtr<nsIDOMDocumentEvent> doc(do_QueryInterface(mDocument)); >+ nsCOMPtr<nsIDOMEvent> event; >+ doc->CreateEvent(NS_LITERAL_STRING("Events"), getter_AddRefs(event)); Did you want this to be initial-caps? ^^^^^^ (Most event names I've seen are all lower-case.) >@@ -3340,7 +3251,7 @@ > parent = mParent; > } > >- if (retarget || (parent != mParent)) { >+ if (!retarget || (parent != mParent)) { Was this a latent bug? >@@ -4743,6 +4654,9 @@ > nsXULPrototypeElement* proto = mPrototype; > mPrototype = nsnull; > >+ if (proto->mNumAttributes == 0) >+ return NS_OK; >+ Won't this cause us to leak the prototype when it has no attributes? >+ <constructor> >+ <![CDATA[ >+ var els = this.getElementsByAttribute("selected", "true"); >+ this.selectedItems = []; >+ for (var i = 0; i < els.length; ++i) >+ this.selectedItems.push(els[i]); >+ ]]> This could cause some pain for larger RDF-based trees since it will cause a content model crawl. Any other way to do this? hyatt should review the other changes to the tree stuff in nsXULElement.cpp, I don't know that code at all.

Case doesn't matter but convention is generally initial cap. http://lxr.mozilla.org/seamonkey/source/content/events/src/nsEventListenerManage r.cpp#2335

The DOM spec uses "Events" with an initial cap. For these sorts of "event groups", the DOM spec uses initial cap notation. This is also the convention it is using with new events as well, e.g., DOMAttrModified, etc. I think the content model crawl is just fine actually, since this is going to become nothing more than a listbox by the time we're done with it. We may take an initial penalty now, but once we morph all trees into outliners, doing a crawl only for listboxes will be fine.

IMHO this change is completely wrong: - if (retarget || (parent != mParent)) { + if (!retarget || (parent != mParent)) { I'd be curious if this is an accident or if you were trying to fix something. The original code, though, is correct AFAICT.

I have no idea how the !retarget and proto->mNumAttributes stuff got into that patch, as I don't remember typing them at all. Ignore them, they will be removed. As for the selectedItems crawl, would it satisfy you if I did that lazily the first time selectedItems is accessed?

r= or sr=waterson, then.

r/sr=hyatt

fixed