Closed Bug 82625 Opened 21 years ago Closed 21 years ago

we should support DOM treeWalker

Categories

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

defect
Not set
normal

Tracking

()

VERIFIED FIXED

People

(Reporter: sicking, Assigned: sicking)

Details

Attachments

(8 files)

It would be nice to support the DOM treeWalker interface. Full implementation 
coming up...
The diff+new files in the diff implements the nsIDOMDocumentTraversal and 
nsIDOMTreeWalker interfaces. All that's needed for full DOM Traversal support 
is NodeIterator.

I wasn't sure if the nsIDOMDocumentTraversal interface should be added in 
domstubs.idl, nsIDOMClassInfo.h and nsDOMClassInfo.cpp files, but the other 
nsIDOMDocument* interfaces didn't seem to be so I left it out.
Status: NEW → ASSIGNED
Keywords: review
FWIW, I tested this on linux. The patch applies cleanly, it builds fine, and it
works fine (from the little testing I did). Even nodeFilters work :)
Keywords: patch
I just remembered that I shouldn't use 0 to return null. Changed to nsnull 
everywhere in my tree
Jonas, this rocks!

I just checked in your interface files into mozilla/dom/public/idl/traversal, I
have some comments (stylistic, mostly) about the other files and changes, let's
chat about this on #mozilla.
another batch up for review. Basically I've made some idl changes jst wanted 
and fixed a bunsh of stylistic stuff in nsTreeWalker.cpp
One additional change that's needed to get this to work in nightly builds is to
add dom_traversal.xpt to the package files that the installer uses.
Have added the xpinstaller stuff in my tree... Though I couldn't find and 
basebrowser-mac for embedding though?
jst: I've (almost) redone the implementation using nsIContent rather then 
nsIDOMNode. However I've ran into a pretty big problem: document nodes don't 
implement nsIContent :(.

So my plan is to use nsIDOMNodes/GetChildNodes but QI to nsIContent/nsIDocument 
when I need to get IndexOf. Sounds like a plan?
this finally works in my tree. I'll just make it blazingly fast then i'll 
attach a patch...
In addition to the attached patch you also need to add dom_traversal.xpt to the
packaging files:

embedding/config/basebrowser-unix
embedding/config/basebrowser-win
xpinstall/packager/packages-win
xpinstall/packager/packages-mac
xpinstall/packager/packages-unix
xpinstall/packager/packages-static-unix

Comment on attachment 48389 [details] [diff] [review]
addresses comments from jst

sr=jst
Attachment #48389 - Flags: superreview+
Comment on attachment 48389 [details] [diff] [review]
addresses comments from jst

r=fabian (i'll get you on the next one for the style nits)
Attachment #48389 - Flags: review+
Comment on attachment 48508 [details] [diff] [review]
xpinstall and embedding stuff

grepped the source and couldn't find any other places.. kewl
r=fabian
Attachment #48508 - Flags: review+
Comment on attachment 48508 [details] [diff] [review]
xpinstall and embedding stuff

sr=jst
Attachment #48508 - Flags: superreview+
ugh, forgot that i need a macbuddy.. hopefully I can convince peterv tomorrow
patches landed...

Thanks everybody that helped to test/review/land/comment on this!
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
VERIFIED fixed, thanks to Jonas once again :-)
Status: RESOLVED → VERIFIED
Component: DOM: Traversal-Range → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.