Crash [@ nsEditorUtils::IsDescendantOf]

RESOLVED FIXED in mozilla2.0b8

Status

()

Core
Editor
--
critical
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Jesse Ruderman, Assigned: Ehsan)

Tracking

(Blocks: 1 bug, {assertion, crash, testcase})

Trunk
mozilla2.0b8
x86
Mac OS X
assertion, crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(blocking2.0 final+)

Details

(crash signature)

Attachments

(3 attachments)

(Reporter)

Description

8 years ago
Created attachment 493901 [details]
testcase (crashes Firefox when loaded)
(Reporter)

Comment 1

8 years ago
Created attachment 493902 [details]
stack trace
(Assignee)

Comment 2

8 years ago
I can't reproduce the crash, but I debugged this and I think I have a handle on what happens here, and in the other editor related test cases which trigger the mismatched update view count assertions (this one does that for me).

What's happening is that some operations in the editor insert and remove nodes from the document (for example, for splitting a node).  One such operation is outdenting.

When this happens, if that node is the last contenteditable node in the document, the editor will reinitialize on the document, which replaces the edit rules object, among other symptoms.

In this test case, for example, when the outdent command is in progress, we split the div element, which removes and reinserts the contenteditable span under it, which triggers a reinitialization of the editor on the document.

When this happens, the new rules object has no notion of the previous update view count and action nesting, so it will happily corrupt those values, leading into update view count mismatches (and possibly crashes).  Basically, with something like that happening, all bets are off, and anything could happen!

I have a patch which tackles one approach of preventing that from happening, I'm testing it right now...

I think this needs to block 2.0.
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Keywords: assertion
(Assignee)

Updated

8 years ago
blocking2.0: --- → ?
(Assignee)

Comment 3

8 years ago
To elaborate, this _does_ crash nightlies, it doesn't crash my local build (which has the patch to bug 612128, which I think is responsible for making the behavior more benign here).  Nevertheless, even with the fix to bug 612128, this is a bug very much worth fixing in its own right.
(Assignee)

Updated

8 years ago
Blocks: 615015
(Assignee)

Comment 4

8 years ago
Created attachment 494298 [details] [diff] [review]
Patch (v1)
Attachment #494298 - Flags: review?(roc)
If we destroy an nsHTMLEditRules while its mRestoreContentEditableCount is true, doesn't that a call to ChangeContentEditableCount(-1) is lost?
(Assignee)

Comment 6

8 years ago
(In reply to comment #5)
> If we destroy an nsHTMLEditRules while its mRestoreContentEditableCount is
> true, doesn't that a call to ChangeContentEditableCount(-1) is lost?

It would, but that shouldn't happen, since the only way for nsHTMLEditRules to be destroyed is for the editor to be reinitialized, and that is triggered by contenteditable count reaching zero.
(Assignee)

Updated

8 years ago
Attachment #494298 - Flags: approval2.0?
(Assignee)

Updated

8 years ago
Whiteboard: [needs landing]
(Assignee)

Comment 7

8 years ago
http://hg.mozilla.org/mozilla-central/rev/92d0320fb62e
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla2.0b8
Crash Signature: [@ nsEditorUtils::IsDescendantOf]
You need to log in before you can comment on or make changes to this bug.