Bug 88502 – [Shadow][Editing] applying document.execCommand('bold') twice to elements having shadow insertion points causes a crash.

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

[Shinya Kawanaka]

HTML:
<div contenteditable>
    BEFORE HOST
    <div id="host">    <span contenteditable="false">not editable<span></div>
    AFTER HOST
</div>

Shadow DOM for host:
<div id="shadow-host"><span contenteditable>SHADOW (BEFORE)</span><shadow></shadow>SHADOW (AFTER)</div>

Shadow DOM for shadow-host
<div contenteditable>NESTED BEFORE<shadow></shadow><input></input>NESTED AFTER</div>


Select from NESTED BEFORE to NESTED AFTER, then document.execCommand('bold') twice. It causes a crash.

ASSERTION FAILED: !m_insertedChild->parentNode()
InsertNodeBeforeCommand.cpp(41) : InsertNodeBeforeCommand

[Shinya Kawanaka]

Hmm...
Currently "not editable" becomes bold after the first document.execCommand('bold'). It's strange.

[Shinya Kawanaka]

Anyway, I'll investigate this more.

[Shinya Kawanaka]

Maybe this is related to Bug 88483...

[Shinya Kawanaka]

Created attachment 147009 [details]
Repro

Place it in LayoutTest/editing/shadow/

[Shinya Kawanaka]

This will be happend when <shadow> is removed from a tree. It will invalidate Shadow DOM distribution, so renderer will be gone. However, we still need the renderer. We have to recalculate layout again.

Bug 88968 will resolve this.

[Shinya Kawanaka]

Actually this is very related to Bug 89380. To fix this, some subset of Node::rendererIsEditable should be converted to Node::isContentEditable().

[Shinya Kawanaka]

Created attachment 148181 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 148181 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=148181&action=review

> Source/WebCore/ChangeLog:9
> +        When inserting or removing insertion points (<shadow> or <content>) into or from Shdaow DOM, shadow content
> +        distribution is invalidated. It will remove all renderers in the same Shadow DOM. When it happends in editing,

What do you mean by "shadow content distribution" is invalidated?

> Source/WebCore/ChangeLog:10
> +        a content-editable elements may be considered as non-content-editable element. This triggers ASSERT failure, since

Why? We need to clarify what's happening. e.g. style isn't updated properly.

> Source/WebCore/ChangeLog:13
> +        To prevent it, content-editablility should be recalculated if necessary. This will be achieved by using

To prevent what?

> LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt:3
> +before host
> + not editable
> +after host

Can we hide this?

> LayoutTests/editing/shadow/bold-twice-in-shadow-expected.txt:7
> +PASS document.execCommand("Bold") did not cause a crash.
> +PASS successfullyParsed is true
> +
> +TEST COMPLETE

This output isn't helpful because we can't tell what we're tesing.

> LayoutTests/editing/shadow/bold-twice-in-shadow.html:34
> +debug('PASS document.execCommand("Bold") did not cause a crash.');
> +
> +var successfullyParsed = true;

I don't think we need to make this a js test. All we need is document.write.

[Shinya Kawanaka]

Created attachment 148365 [details]
Patch

[WebKit Review Bot]

Comment on attachment 148365 [details]
Patch

Clearing flags on attachment: 148365

Committed r120749: <http://trac.webkit.org/changeset/120749>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.