306900 - [FIX]nsDocument::StartDocumentLoad must not leave aDocListener untouched

Status: RESOLVED FIXED

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

Description

[timeless]

/* interface foo : nsISupports { nsISupports bar(); void baz(out nsISupports
bip); } */
class Foo : public foo {
nsresult Bar(nsISupports **ign){ return 0;}
nsresult Baz(nsISupports **ign){ return 0;}
}

<shaver> both are bogus
<shaver> for not guaranteeing the state of *ign

Comment 1

[Christian :Biesinger (don't email me, ping me on IRC)]

this is a pure virtual function, subclasses are supposed to fill in aDocListener...

Comment 2

[Boris Zbarsky [:bzbarsky]]

Yeah, we never actually have an nsDocument instance around, so this is just a
non-issue, I think.

Comment 3

[Boris Zbarsky [:bzbarsky]]

I'm going to mark this invalid.  This is never called by anyone but subclasses, and they're the ones responsible for creating a listener.  I suppose we could rename this DoStartDocumentLoad and remove that param altogether, but that doesn't seem worth it...

Comment 4

[Christian :Biesinger (don't email me, ping me on IRC)]

we could make it a pure virtual function (with an implementation), to ensure that all subclasses have to override it.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Hmm... Is it possible to have a pure virtual function that still has an impl?  If so, great -- this becomes something like a 3-character fix, right?

Comment 6

[Christian :Biesinger (don't email me, ping me on IRC)]

yes, it is.

Comment 7

[Boris Zbarsky [:bzbarsky]]

Attachment: Do the virtual thing

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 209212 [details] [diff] [review]
Do the virtual thing

r+sr=jst

Comment 9

[Boris Zbarsky [:bzbarsky]]

Fixed.