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

RESOLVED FIXED in mozilla2.0b12

Status

()

defect
--
critical
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: jruderman, Assigned: mats)

Tracking

(Blocks 2 bugs, {crash, regression, testcase})

Trunk
mozilla2.0b12
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(Whiteboard: [softblocker], crash signature)

Attachments

(4 attachments, 1 obsolete attachment)

(Reporter)

Description

8 years ago
Posted file testcase
Might be related to bug 634291, which also involved navigated-away designMode documents.
(Reporter)

Comment 1

8 years ago
Posted file crash report
(Reporter)

Comment 2

8 years ago
Nominating for blocking because crossfuzz might hit this a lot (see bug 635539).
blocking2.0: --- → ?
(Assignee)

Comment 3

8 years ago
Posted patch fixSplinter Review
When the editor goes away it calls nsHTMLEditRules::DetachEditor()
which nulls out 'mHTMLEditor'.  So a null-check should be safe here.
Assignee: nobody → matspal
Attachment #513893 - Flags: review?(ehsan)
(Assignee)

Updated

8 years ago
OS: Mac OS X → All
Hardware: x86 → All
blocking2.0: ? → final+
Keywords: regression
Whiteboard: [softblocker]
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!
Attachment #513893 - Flags: review?(ehsan) → review+
Comment on attachment 513893 [details] [diff] [review]
fix

I'm going to write a test for this too, and will land both patches soon.
Attachment #513893 - Flags: approval2.0+
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
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Target Milestone: --- → mozilla2.0b12
Duplicate of this bug: 635539
(Assignee)

Comment 8

8 years ago
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
Posted patch Tests (obsolete) — Splinter Review
Attachment #514074 - Flags: review?(roc)

Updated

8 years ago
Depends on: 636674
Comment on attachment 514074 [details] [diff] [review]
Tests

This needs to be a chrome test and not use enablePrivilege.
Attachment #514074 - Flags: review?(roc) → review-
Posted patch TestsSplinter Review
Converted the test to a chrome test.
Attachment #514074 - Attachment is obsolete: true
Attachment #519932 - Flags: review?(roc)
Test case: http://hg.mozilla.org/mozilla-central/rev/a32d776346b5
No longer depends on: post2.0
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsHTMLEditRules::DocumentModifiedWorker]
You need to log in before you can comment on or make changes to this bug.