55750 - Crash opening "new composer window" to bogus location, after "this type of page can't be edited" dialog

Status: VERIFIED FIXED

(Core :: DOM: Editor, defect, P3)

Description

[Jesse Ruderman]

Steps to reproduce crash:
1. Ctrl-L
2. Type in http://www.askljdflskajflksjflaksdjflk.asd
3. Select "new composer window"
4. Click "open"

At this point you'll see the (incorrect) error message, "This type of page 
can't be edited".

5. Click ok to error message

Crash in docshell.dll.

Win98, 2000100708.

Comment 1

[Jesse Ruderman]

Using the same steps with a valid ftp site will crash in jsdom.dll before 
bringing up the error dialog.  With a bogus ftp site, the error dialog comes up 
and makes the editor window go away without crashing.

Comment 2

[Simon Fraser [no longer active]]

Easy null-check fix for RTM.
This is crashing in nsWebShell.cpp, line 1005, at
         NS_ENSURE_SUCCESS(mPrefs->GetBoolPref("keyword.enabled", &
keywordsEnabled),
            NS_ERROR_FAILURE);

because mPrefs is NULL.

-> anthonyd

Comment 3

[Simon Fraser [no longer active]]

All platforms.

Comment 4

[anthonyd]

setting to m19 to show up on radar.

anthonyd

Comment 5

[rpotts (gone)]

shouldn't mPrefs have been initialized?

Comment 6

[anthonyd]

Attachment: patch for the bug

Comment 7

[anthonyd]

need review, approval, and super review

Comment 8

[Simon Fraser [no longer active]]

r=sfraser on the patch.

Comment 9

[Phil Peterson]

PDT marking [rtm++]

Comment 10

[anthonyd]

ok, just need the a= and the sr=
please, anyone?
anthonyd

Comment 11

[anthonyd]

hoping kin can give me an sr= for this relatively minor fix for a crasher.

anthonyd

Comment 12

[kinmoz]

sr=[e-mail]

Comment 13

[rpotts (gone)]

I still don't understand why mPrefs is null at this point...  mPrefs *must* be 
valid or the docShell will crash in lots of different places - since it *never* 
validates mPrefs.

Has nsIBaseWindow::Create() been called on the docshell?  This is the method 
that initializes mPrefs.

Right now I think that this patch is masking a symptom of something else that is 
wrong.

-- rick

Comment 14

[Simon Fraser [no longer active]]

rpotts: what's happening here is that nsWebShell::OnEndDocumentLoad() is getting 
called on a webShell whose Destroy method has been called already. The Destroy 
method (which is actually called multiple times) nulls out mPrefs.

I'd be happy with this patch for now, since this is an edge case (window 
destruction after url load failure), and fixing the real problem later.

Comment 15

[rpotts (gone)]

oh, sweet...  yeah, I agree, lets go with this patch for now.  (sr=rpotts)

I'll keep in mind that notifications to the docshell can happen *after* destroy 
is called...  Maybe I can bulletproof the code some more if/when I dive into 
docshell cleanup again :-)

Comment 16

[anthonyd]

rpotts,
quick question, if you give me the sr=, who do i goto for the a=?

thanks,
anthonyd

Comment 17

[anthonyd]

fix checked in (cvs comment to follow):
fix for bug # 55750
Crash opening "new composer window" to bogus location, after "this type of page 
can't be edited" dialog
r=sfraser
sr=kin
sr=rpotts

Comment 18

[sujay]

verified in 10/13 build.