635636 - Crash [@ nsHTMLEditRules::DocumentModifiedWorker] with navigated-away designMode document

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Might be related to bug 634291, which also involved navigated-away designMode documents.

Comment 1

[Jesse Ruderman]

Attachment: crash report

Comment 2

[Jesse Ruderman]

Nominating for blocking because crossfuzz might hit this a lot (see bug 635539).

Comment 3

[Mats Palmgren (inactive)]

Attachment: fix

When the editor goes away it calls nsHTMLEditRules::DetachEditor()
which nulls out 'mHTMLEditor'.  So a null-check should be safe here.

Comment 4

[(no longer active)]

Comment on attachment 513893 [details] [diff] [review]
fix

(In reply to comment #3)
> Created attachment 513893 [details] [diff] [review]
> fix
> 
> When the editor goes away it calls nsHTMLEditRules::DetachEditor()
> which nulls out 'mHTMLEditor'.  So a null-check should be safe here.

Indeed!

Comment 5

[(no longer active)]

Comment on attachment 513893 [details] [diff] [review]
fix

I'm going to write a test for this too, and will land both patches soon.

Comment 6

[(no longer active)]

The test is taking longer than I expected (trying to get rid of the timeouts and still crash in a non-patched build), so I landed the patch itself for now.

http://hg.mozilla.org/mozilla-central/rev/b63a30d564e0

Comment 8

[Mats Palmgren (inactive)]

Apparently the method signature was changed, so I pushed the following bustage fix:
 void
 nsHTMLEditRules::DocumentModifiedWorker()
 {
   if (!mHTMLEditor) {
-    return NS_OK;
+    return;

http://hg.mozilla.org/mozilla-central/rev/8fa0a465d15b

Comment 9

[(no longer active)]

Attachment: Tests

Comment 10

[(no longer active)]

roc: ping?

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 514074 [details] [diff] [review]
Tests

This needs to be a chrome test and not use enablePrivilege.

Comment 12

[(no longer active)]

Attachment: Tests

Converted the test to a chrome test.

Comment 13

[(no longer active)]

Test case: http://hg.mozilla.org/mozilla-central/rev/a32d776346b5