173350 - nsIDOMTreeWalker crashes with XUL documents

Status: RESOLVED FIXED

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

Description

[Aaron Leventhal]

I believe it is crashing because it is getting an incorrect result from
IndexOF(). The IndexOf() function calls nsXULDocument::IndexOF() which is not
implemented, and has the NS_NOTREACHED assertion.

Eventually this causes nsTreeWalker::TestNode() to crash because it has a null
pointer for the node.

Comment 1

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

nice!

will be an easy fix, hrm.. why is there no 1.2 final target?

Comment 2

[Aaron Leventhal]

This is blocking my fix for bug 161410.

There are a lot of NS_NOT_IMPLEMENTED methods in nsXULDocument. Shouldn't we fix
all of them at once?

At least, if there is a pure out param, we should zero it out.

Comment 3

[Aaron Leventhal]

Hmm, I have fixes cloggin up my tree waiting for this. Sicking, do you want me
to take it?

Comment 4

[Aaron Leventhal]

I think we'll get this fixed for free when bug 124412 is fixed, because we'll
inherit an IndexOf() impl from nsDocument.

Comment 5

[Aaron Leventhal]

Have patch.

Comment 6

[Aaron Leventhal]

Attachment: Implement IndexOf(), ChildAt() and GetChildCount() for nsXULDocument -- the only child is mRootContnent

Seeking r=sicking, sr=jst or bz

Comment 7

[Boris Zbarsky [:bzbarsky]]

+    NS_IF_ADDREF(aResult = mRootContent);
+    return NS_OK;

Should this throw if mRootContent is null?

+    aIndex = (aPossibleChild == mRootContent)? 0 : -1;

What if aPossibleChild == mRootContent == nsnull?

+    aCount = 1;

What if mRootContent is null?

If mRootContent is never null, why not just use NS_ADDREF?

Comment 8

[Aaron Leventhal]

Attachment: New patch, taking care of bz's concerns

Seeking r=

Comment 9

[Boris Zbarsky [:bzbarsky]]

That addressed concerns 1 and 4 out of 4... returning "1" for the count when we
have no kids seems wrong. IndexOf(nsnull) returning "0" seems wrong.

Comment 10

[Aaron Leventhal]

Attachment: Sorry bz, here you go.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 104447 [details] [diff] [review]
Sorry bz, here you go.

much better.  ;)

Comment 12

[Johnny Stenback (:jst)]

Comment on attachment 104447 [details] [diff] [review]
Sorry bz, here you go.

r/sr=jst

Comment 13

[Aaron Leventhal]

checked in

Comment 14

[Asa Dotzler [:asa]]

Comment on attachment 104447 [details] [diff] [review]
Sorry bz, here you go.

a=asa for checkin to the 1.2 branch (on behalf of drivers)

Comment 15

[Aaron Leventhal]

Checked into 1.2 branch.