Closed Bug 572598 Opened 15 years ago Closed 15 years ago

"ASSERTION: aDocument must be current doc of aParent" with lots of execCommand

Categories

(Core :: DOM: Editor, defect)

Product:
Core
Component:
DOM: Editor
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla2.0b1

People

(Reporter: jruderman, Assigned: ehsan.akhgari)

References

Details

(Keywords: assertion, testcase)

Attachments

(8 files, 1 obsolete file)

testcase
684 bytes, text/html
Details
Part 1: Don't attempt to inject nodes for type-in state where the container can't accept child nodes
1.18 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 2: Make sure not to exceed the editor's root element when promoting ranges
1.51 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 3: Improve error checking in nsEditor::GetStartNodeAndOffset using NS_ENSURE macros
1.82 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 4: Don't attempt to check empty selections, after all, they're empty
1.13 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 5: Remove a bogus NS_ENSURE_TRUE check, and add a couple more
1.75 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 6: Don't try to split a node which can't contain a list when inserting one
1.20 KB, patch
roc
: review+
Details | Diff | Splinter Review
Part 7: reftest
1.92 KB, patch
Details | Diff | Splinter Review
Attached file testcase —
Loading and unloading the testcase trigger lots of assertions, including: ###!!! ASSERTION: transaction did not execute properly: '(NS_SUCCEEDED(result))', file /builds/slave/mozilla-central-macosx-debug/build/editor/libeditor/base/nsEditor.cpp, line 654 ###!!! ASSERTION: anonymous nodes should not be in child lists: '!aOldChild->IsRootOfAnonymousSubtree()', file /builds/slave/mozilla-central-macosx-debug/build/layout/base/nsCSSFrameConstructor.cpp, line 11517 ###!!! ASSERTION: aDocument must be current doc of aParent: '!aParent || aDocument == aParent->GetCurrentDoc()', file /builds/slave/mozilla-central-macosx-debug/build/content/base/src/nsGenericElement.cpp, line 2799 The last one isn't covered by any other bugs.
The root cause of the problem here is that when we have a type-in state (which is set by the forecolor command, among others), we don't correctly check to see if the active node in the document can accept children on the next editor transaction. Therefore, when the next command (insertunorderedlist, among others) attempts to set the type-in state, we would end up with the following content tree: input@0x204c0620 id="i" intrinsicstate=[00020040] flags=[02200383] primaryframe=0x14d8640 refcount=27< span@0x14eb74c0 _moz_dirty="" style="color: green;" intrinsicstate=[00020000] flags=[02200901] primaryframe=0x0 refcount=6< Text@0x14ee0670 intrinsicstate=[00020000] flags=[00000101] primaryframe=0x0 refcount=9<> > > which is invalid, and breaks tons of assumptions elsewhere in the code. This patch adds the check in question. With this patch, the only assertion when loading the test case is: ###!!! ASSERTION: transaction did not execute properly: '(NS_SUCCEEDED(result))', file /Users/ehsanakhgari/moz/src/editor/libeditor/base/nsEditor.cpp, line 654 More patches to follow...
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #451829 - Flags: review?(roc)
Another problem I discovered while working on this is that the HTML editor could exceed the editor's root element when promoting ranges, because it does an equality check against the root element, but the element at hand might be higher in the content hierarchy already (e.g. when the selection starts and ends on the <html> element, for example.
Attachment #452331 - Flags: review?(roc)
Attachment #452331 - Attachment description: Make sure not to exceed the editor's root element when promoting ranges → Part 2: Make sure not to exceed the editor's root element when promoting ranges
TypeInState's selection listener tries to get ranges for non-collapsed selections, but doesn't consider empty selections, which casuses it to fail.
Attachment #452353 - Flags: review?(roc)
This is the core problem causing the last assertion. The code to inject a list into the document did not change that the parent (in this case, the input element) can contain a list element (in this case, a ul element), and attempted to split this into two nodes. In addition to the assertion, this caused the input element to be split into two input elements! Therefore, I'll add a reftest which makes sure that the assertion doesn't happen, and that the input element is not split into two!
Attachment #452584 - Flags: review?(roc)
Attached patch Part 7: reftest (obsolete) — — Splinter Review
Attachment #452589 - Flags: review?(roc)
Attached patch Part 7: reftest — — Splinter Review
Actually, we need to remove the selection from the document, otherwise the rendering of 572598-1.html can't match that of 572598-ref.html.
Attachment #452589 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: