Bug 67966 – Assertion fail in WebCore::InsertNodeBeforeCommand::InsertNodeBeforeCommand

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: HTML Editing

[Annie Sullivan]

This HTML will reproduce:

<dl><div id="div" contenteditable="true"></div></dl>
<script>
div.focus();
document.execCommand("InsertUnorderedList");
</script>

This assert on line 45 of InsertNodeBeforeCommand.cpp fires:
ASSERT(m_refChild->parentNode()->rendererIsEditable() || !m_refChild->parentNode()->attached());

The problem is in InsertListCommand::doApplyForSingleParagraph().

Line 195 calls enclosingListChild() on the div, which checks to see if an the node or an ancestor has a parent which isListElement(). Since isListElement() considers <ol>, <ul>, and <dl> as lists, it returns the div.

Line 199 calls enclosingList() on the div, which checks to see if any ancestor of the node is a <ul> or <ol>. It returns null because the list is a <dl>.

So enclosingListChild() and enclosingList() disagree on whether <dl> is a list. What ends up happening is that the code crosses the editing boundary trying to fix the div being a list child outside of a list, because of the inconsistency:
1. fixOrphanedListChild is called, and tries to replace the contenteditable div with a <ul>.
2. InsertNodeBeforeCommand constructor gets called, with the div as refChild. The assertion fails because the div's parent is not editable and it is attached.

I think <dl> should not be a list because it's not really supported in the editing code or contenteditable in general; most of the code assumes a list item is <li>, not <dt> or <dd>. I'm running the layout tests now to see if this change breaks anything; if it doesn't, I'll post a patch.

[Ryosuke Niwa]

I think this is a duplicate of https://bugs.webkit.org/show_bug.cgi?id=67918.

[Annie Sullivan]

(In reply to comment #1)
> I think this is a duplicate of https://bugs.webkit.org/show_bug.cgi?id=67918.

Oops, yes it is. I think the issue of whether <dl> is a list or not is still important; will post there once my layout tests finish running.

*** This bug has been marked as a duplicate of bug 67918 ***