After a lot of discussion and various implementation efforts, we concluded that JS cannot walk anonymous nodes from and XBL document outside of the DOM inspector's treeview widget. See http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/d89ed07aea746c50# If the following two functions where implemented, I think Chromebug could support anonymous nodes. (see http://code.google.com/p/fbug/issues/detail?id=2343&q=anonymous&sort=-id) http://mxr.mozilla.org/comm-central/source/mozilla/layout/inspector/src/inDeepTreeWalker.cpp#196 http://mxr.mozilla.org/comm-central/source/mozilla/layout/inspector/src/inDeepTreeWalker.cpp#214

> It also ignores whatToShow and the filter. So the walker acts as if SHOW_ALL were set, correct? Is showAnonymousContent obeyed. (IMO it ought to default TRUE, since its why we use this, but I guess it's too late). Is showSubDocuments obeyed. (default FALSE) --- : mShowAnonymousContent(PR_FALSE), mShowSubDocuments(PR_FALSE), mWhatToShow(nsIDOMNodeFilter::SHOW_ALL) --- "It also ignores init() argument 'whatToShow', and sets the property whatToShow==SHOW_ALL, and ignores the filter." (perhaps you should just remove the whatToShow argument?)

> So the walker acts as if SHOW_ALL were set, correct? > Is showAnonymousContent obeyed. > Is showSubDocuments obeyed. Yes to all, as long as you set those booleans before calling init(). > (perhaps you should just remove the whatToShow argument?) Tempting, yes.

Comment on attachment 406847 [details] [diff] [review] With the test actually included > inDeepTreeWalker::NextNode(nsIDOMNode **_retval) > { ... >+ nsCOMPtr<nsIDOMNode> parent; >+ rv = ParentNode(getter_AddRefs(parent)); >+ if (!parent) { >+ // Nowhere else to go; we're done. Restore our state >+ while (lastChildCallsToMake--) { >+ nsCOMPtr<nsIDOMNode> dummy; >+ LastChild(getter_AddRefs(dummy)); You have tabs in here (!?!). Also, could you assert that this 'walk back' mechanism ends up at the node we started at. r=me with that

> You have tabs in here (!?!) I blame dastardly lack of modeline. Which I have now fixed locally. And added that assert.

Comment on attachment 406847 [details] [diff] [review] With the test actually included Sure would be nice if we actually had a stack class like the stls... > inDeepTreeWalker::PreviousSibling(nsIDOMNode **_retval) >+ // Pop off the current node, and push the new one So, I understand that this is what you are doing (the code is obvious), but the comment doesn't really tell me why. In general, a few more comments explaining what is going on would be very useful. I'm going to point out the few places that really aren't clear to me, but more comments would be greatly welcomed! >+ mStack.RemoveElementAt(mStack.Length()-1); nit: space around minus operator > inDeepTreeWalker::NextSibling(nsIDOMNode **_retval) >+ // Pop off the current node, and push the new one >+ mStack.RemoveElementAt(mStack.Length()-1); ditto on last two comments > inDeepTreeWalker::PreviousNode(nsIDOMNode **_retval) >+ nsCOMPtr<nsIDOMNode> node; >+ nsresult rv = PreviousSibling(getter_AddRefs(node)); >+ NS_ENSURE_SUCCESS(rv, rv); PreviousSibling only returns NS_OK, so it doesn't make much sense to check the return variable here. >+ while (node) { >+ rv = LastChild(getter_AddRefs(node)); >+ NS_ENSURE_SUCCESS(rv, rv); >+ } I'm not really sure what this loop is doing, so a comment please. Also, LastChild only ever returns NS_OK, so no need to check. > inDeepTreeWalker::NextNode(nsIDOMNode **_retval) >+ // First try our kids >+ nsresult rv = FirstChild(_retval); >+ NS_ENSURE_SUCCESS(rv, rv); ditto on on checking rv r=sdwilsh

> In general, a few more comments explaining what is going on would be very > useful. Will add. > nit: space around minus operator Will fix. > PreviousSibling only returns NS_OK, I was aiming to not make those assumptions, but I guess I can change that. No problem. > I'm not really sure what this loop is doing, so a comment please Will do.

Pushed http://hg.mozilla.org/mozilla-central/rev/a830ed8bf8dc with those changes. John, please let me know (probably in bug form?) if there are any other issues with making this work for you?

Would we want this on 1.9.2 (and maybe 1.9.1)? I should get an updated version of DOMi out to the masses with your fix in it too...

Yes, please! Having Chromebug be more useful to extension devs at large is high-value.

(By which I mean "both 1.9.1 and 1.9.2 would be ideal").

I'm not quite willing to do this on 1.9.1, since it is in fact an api-breaking change. I can see landing it on 1.9.2.

(In reply to comment #9) > John, please let me know (probably in bug form?) if there are any other issues > with making this work for you? Thanks! Firebug tracking for completing the Chromebug part is http://code.google.com/p/fbug/issues/detail?id=2343 (But it may be a week or so before I can get it).

Pushed http://hg.mozilla.org/releases/mozilla-1.9.2/rev/70cbc9ee2134

If the end of the document is reached, the walker will loop infinitely on the last node if you try to advance it with nextNode(), instead of returning null for walker.currentNode. See bug 526939.

That's the correct behavior for the walker. The problem is in the consumer, which I apparently missed somehow...