Leaving the testcase (e.g. loading another document in the same tab, or closing the tab, or quitting) triggers: ###!!! ASSERTION: EnsureEditorData() called when detached. : '!HasDetachedEditor()', file /Users/jruderman/trunk/mozilla/docshell/base/nsDocShell.cpp, line 9010 [sic] This assertion was added in bug 386782.

Reloading triggers a whole bunch of it.

* InternalLoad calls DetachEditorFromWindow which stores editor data on the shentry * InternalLoad sets the editor data on the shentry to null, because aLoadType & LOAD_CMD_HISTORY is 0 * the pagehide handler turns off editing, which needs the editing session so we create new editor data * FirePageHideNotification calls DetachEditorFromWindow which stores editor data on the shentry We should probably just delay DetachEditorFromWindow/clearing editor data on the shentry until FirePageHideNotification.

This seems to fix it, don't know if it is enough and whether it breaks anything, so it'll need a bunch of testing (designmode, contenteditable, compose mail, composer). Chris, could you take this one from here?

Breaks when you load a testcase, navigate to a DM page, nav back to testcase, reload testcase, then nav forward to DM page. Investigating...

Yeah, turns out for a reload InteralLoad is called before FirePageHide... Been working on a better fix, but not there yet :-/.

This one seems to behave better.

(In reply to comment #6) > Created an attachment (id=318017) [details] > v1.1 > > This one seems to behave better. > Still triggers assertions for me when you reload the page. Fixes a couple of other ones, so we're getting closer at least...

(In reply to comment #6) > Created an attachment (id=318017) [details] > v1.1 > > This one seems to behave better. > Peter, why did you remove StartPageLoad() etc from nsEditingSession in this patch? Would that affect Smaug's patch for bug 430858?

StartPageLoad is unneeded, even with the patch in bug 430858. I don't remember the exact issue with StartDocumentLoad. There was a problem with the ordering, we'd postpone removing the editing session as a progress listener till PageHide and so StartDocumentLoad was called, which messed things up. I've tried your new patch in bug 386782 without the changes to StartDocumentLoad and I can't reproduce the problem, but that could also be because I don't remember the exact steps to reproduce :-/.

Comment on attachment 318017 [details] [diff] [review] v1.1 This is handled by bug 430624's patch.

The assertion failure is fixed by 430624. There's still a warning firing when you reload the page though: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004002: file c:/work/src/browser/mozilla/content/html/document/src/nsHTMLDocument.cpp, line 3943 This is happening because we're detaching the editor in nsEditingSession::StartDocumentLoad(), and then the pagehide for the testcase is removing the last CE node, which toggles CE off, calling nsHTMLDocument::TurnEditingOff(), which can't get the editing session from its docshell, and fails an ENSURE_SUCCESS.

cpearce, please file a new bug if you think the warning is a problem.

Crashtest: http://hg.mozilla.org/mozilla-central/rev/f4622261cce8