Closed Bug 428844 Opened 12 years ago Closed 12 years ago

Crash [@ nsEditingSession::TearDownEditorOnWindow] on reload with contenteditable and xslt

Categories

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

x86
Windows XP
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla1.9

People

(Reporter: martijn.martijn, Assigned: cpearce)

References

Details

(Keywords: crash, regression, testcase)

Crash Data

Attachments

(2 files, 1 obsolete file)

Attached file testcase
See testcase, which crashes current trunk build on reload.

This seems to have regressed between 2008-02-19 and 2008-02-20:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2008-02-19+04&maxdate=2008-02-20+09&cvsroot=%2Fcvsroot
I guess a regression from bug 412920.

http://crash-stats.mozilla.com/report/index/f360bf28-09a4-11dd-af7f-001cc45a2ce4
0  	xul.dll  	nsEditingSession::TearDownEditorOnWindow  	 mozilla/editor/composer/src/nsEditingSession.cpp:576
1 	xul.dll 	nsEditingSession::StartDocumentLoad 	mozilla/editor/composer/src/nsEditingSession.cpp:1022
2 	xul.dll 	nsEditingSession::OnStateChange 	mozilla/editor/composer/src/nsEditingSession.cpp:804
3 	xul.dll 	xul.dll@0x81c1ab
Ok, so what's happening here is that we're assuming that all editable documents are html documents. We've got an nsXMLDocument here, and we're QI'ing it to an htmlDoc, and not checking that the QI succeeds before calling nsHTMLDocument::TearingDownEditor() on it, and it's crashing. We should only call TearingDownEditor() if we've got an html doc, and editing is on.
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #315425 - Flags: review?(peterv)
Attachment #315425 - Flags: superreview+
Attachment #315425 - Flags: review?(peterv)
Attachment #315425 - Flags: review+
Martijn, can you file a bug that XSLT should create an HTML document for this testcase?
Isn't that basically the same as bug 61675?
Same as patch v1, but with crashtest. r+/sr+ Peterv.
Attachment #315425 - Attachment is obsolete: true
Attachment #316134 - Flags: superreview+
Attachment #316134 - Flags: review+
Comment on attachment 316134 [details] [diff] [review]
Patch - v1 with crashtest

Low risk patch, fixes a crash, patch includes crashtest.
Attachment #316134 - Flags: approval1.9?
Comment on attachment 316134 [details] [diff] [review]
Patch - v1 with crashtest

a1.9=beltzner
Attachment #316134 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
mozilla/editor/composer/src/nsEditingSession.cpp 	1.59
mozilla/editor/composer/src/crashtests/428844-1-inner.xhtml 	1.1
mozilla/editor/composer/src/crashtests/428844-1.html 	1.1
mozilla/editor/composer/src/crashtests/crashtests.list 	1.1
mozilla/testing/crashtest/crashtests.list 	1.36
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9
Depends on: 471185
Depends on: 517275
The test added in this bug doesn't seem to really test what it wants to (in particular, the test ends before the subframe has a chance to be reloaded)....

Chris, do you want a separate bug on that?  Or is the .src set not really needed?
(In reply to comment #8)
> The test added in this bug doesn't seem to really test what it wants to (in
> particular, the test ends before the subframe has a chance to be reloaded)....
> 
> Chris, do you want a separate bug on that?  Or is the .src set not really
> needed?

May as well file a separate bug, since this one was closed almost 3 years ago. We may be able to re-enable this test while we're at it, and resolve bug 471185 too.
(In reply to comment #9)
> (In reply to comment #8)
> > The test added in this bug doesn't seem to really test what it wants to (in
> > particular, the test ends before the subframe has a chance to be reloaded)....
> > 
> > Chris, do you want a separate bug on that?  Or is the .src set not really
> > needed?
> 
> May as well file a separate bug, since this one was closed almost 3 years ago.
> We may be able to re-enable this test while we're at it, and resolve bug 471185
> too.

Boris, feel free to assign the new bug to me!  :-)
Crash Signature: [@ nsEditingSession::TearDownEditorOnWindow]
You need to log in before you can comment on or make changes to this bug.