Crash [@ nsHTMLEditRules::WillOutdent] involving designMode and removeformat

RESOLVED FIXED

Status

()

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

People

(Reporter: Jesse Ruderman, Assigned: smaug)

Tracking

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

Trunk
crash, testcase
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9 +
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

12 years ago
This is similar to bug 335995, but seems to involve different code and doesn't involve the <select> element.
(Reporter)

Comment 1

12 years ago
Created attachment 220377 [details]
testcase
(Reporter)

Comment 2

12 years ago
Before crashing, a debug build spews:

###!!! ASSERTION: not an element node: 'element', file /Users/admin/trunk/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp, line 8579

###!!! ASSERTION: null node passed to nsEditor::Tag(): 'aNode', file /Users/admin/trunk/mozilla/editor/libeditor/base/nsEditor.cpp, line 3977

WARNING: NS_ENSURE_TRUE(aNode) failed: file /Users/admin/trunk/mozilla/editor/libeditor/html/nsHTMLCSSUtils.cpp, line 1403

###!!! ASSERTION: null node passed to nsHTMLEditor::IsTableElement: 'node', file /Users/admin/trunk/mozilla/editor/libeditor/html/nsHTMLEditUtils.cpp, line 189
OS: Mac OS X 10.3 → Mac OS X 10.4
OS: Mac OS X 10.4 → All
Hardware: Macintosh → All
(Reporter)

Updated

12 years ago
Blocks: 336383
(Reporter)

Comment 3

12 years ago
The testcase as-is doesn't work because it hits WRONG_DOCUMENT_ERR when trying to move nodes into the iframe, because of the change in bug 47903.  Once I fix that, it runs into NS_ERROR_DOM_RANGE_BAD_BOUNDARYPOINTS_ERR when trying to create the second range, thanks to the change in bug 336381.

--> Fixed or dup of bug 336381.
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
(Reporter)

Comment 4

12 years ago
Created attachment 256263 [details]
testcase with importNode (wrong bug)
Attachment #220377 - Attachment is obsolete: true
(Reporter)

Comment 5

12 years ago
I even tried changing it to create r2 while its endpoints are still in the document.  Still no crash.
(Reporter)

Comment 6

12 years ago
Oops, I mixed up this bug and bug 335995.  The last three comments (including the testcase) are all for bug 335995.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Reporter)

Updated

12 years ago
Attachment #256263 - Attachment description: testcase with importNode → testcase with importNode (wrong bug)
Attachment #256263 - Attachment is obsolete: true
(Reporter)

Comment 7

12 years ago
Created attachment 256265 [details]
testcase with importNode

Trunk Firefox still crashes.
(Reporter)

Updated

12 years ago
Assignee: mozeditor → nobody
Status: REOPENED → NEW
(Reporter)

Comment 8

12 years ago
This bug prevents me from testing execCommand well :(
Flags: blocking1.9?
(Reporter)

Comment 9

12 years ago
Smaug/Martijn, can you fix this, or do you know who might be able to fix it?
I bet Smaug can fix this ;)
To fix the crash, it is enough to add simple |n &&| to the while(...)
where it is crashing, but that doesn't fix the assertions.
(Reporter)

Comment 12

12 years ago
Thanks, that was enough to let me test execCommand a lot more than before.  Well, enough to find bug 372094, anyway :)
Flags: blocking1.9? → blocking1.9+
Created attachment 257969 [details] [diff] [review]
to fix the crash

Could we take this simple null check for now?
Attachment #257969 - Flags: superreview?(neil)
Attachment #257969 - Flags: review?(neil)

Comment 14

12 years ago
Comment on attachment 257969 [details] [diff] [review]
to fix the crash

Well, although this hides the crash, I think this is papering over the bug in GetNodeLocation for a node that has no parent (or indeed, document).
Attachment #257969 - Flags: superreview?(neil)
Attachment #257969 - Flags: review?(neil)
Attachment #257969 - Flags: review+
QA Contact: editor
smaug, can you get SR and land this?  and file a followup on Neil's comment?
(Assignee)

Updated

11 years ago
Attachment #257969 - Flags: superreview?(peterv)
Comment on attachment 257969 [details] [diff] [review]
to fix the crash

Couldn't we just break (or continue?) immediately after GetNodeLocation? And we need a follow-up bug for the real fix, I wonder if we shouldn't make GetNodesFromSelection remove nodes that are not in a document.
Attachment #257969 - Flags: superreview?(peterv) → superreview+
(Assignee)

Updated

11 years ago
Depends on: 385895
I'll check in the null check fix so that testing execCommand is easier.
Bug 385895 filed.
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 12 years ago11 years ago
Resolution: --- → FIXED
(Assignee)

Updated

11 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Updated

11 years ago
Assignee: nobody → Olli.Pettay
Status: REOPENED → NEW
(Assignee)

Updated

11 years ago
Status: NEW → RESOLVED
Last Resolved: 11 years ago11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite?
(Reporter)

Comment 18

11 years ago
Crashtest checked in.
Flags: in-testsuite? → in-testsuite+
Crash Signature: [@ nsHTMLEditRules::WillOutdent]
You need to log in before you can comment on or make changes to this bug.