82625 - we should support DOM treeWalker

Status: VERIFIED FIXED

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

Description

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It would be nice to support the DOM treeWalker interface. Full implementation 
coming up...

Comment 1

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: zip with diff and new files

Comment 2

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: testpage i've used for testing

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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.

Comment 4

[Fabian Guisset]

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 :)

Comment 5

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

I just remembered that I shouldn't use 0 to return null. Changed to nsnull 
everywhere in my tree

Comment 6

[Johnny Stenback (:jst)]

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.

Comment 7

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: updated diff with jsts idl changes

Comment 8

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: updated nsTreeWalker.cpp

Comment 9

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: updated nsTreeWalker.h

Comment 10

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

another batch up for review. Basically I've made some idl changes jst wanted 
and fixed a bunsh of stylistic stuff in nsTreeWalker.cpp

Comment 11

[Johnny Stenback (:jst)]

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.

Comment 12

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Have added the xpinstaller stuff in my tree... Though I couldn't find and 
basebrowser-mac for embedding though?

Comment 13

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

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?

Comment 14

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

this finally works in my tree. I'll just make it blazingly fast then i'll 
attach a patch...

Comment 15

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: this one is lightning fast :-)

Comment 16

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: addresses comments from jst

Comment 17

[Johnny Stenback (:jst)]

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 18

[Johnny Stenback (:jst)]

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

sr=jst

Comment 19

[Fabian Guisset]

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)

Comment 20

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Attachment: xpinstall and embedding stuff

Comment 21

[Fabian Guisset]

Comment on attachment 48508 [details] [diff] [review]
xpinstall and embedding stuff

grepped the source and couldn't find any other places.. kewl
r=fabian

Comment 22

[Johnny Stenback (:jst)]

Comment on attachment 48508 [details] [diff] [review]
xpinstall and embedding stuff

sr=jst

Comment 23

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

ugh, forgot that i need a macbuddy.. hopefully I can convince peterv tomorrow

Comment 24

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

patches landed...

Thanks everybody that helped to test/review/land/comment on this!

Comment 25

[Fabian Guisset]

VERIFIED fixed, thanks to Jonas once again :-)