Bug 86345 – Deleting a content-editable element having a ShadowRoot causes a crash

• Status: RESOLVED
• Resolution: FIXED
• Component: DOM

[Shinya Kawanaka]

Repro:

<div id="container" contenteditable></div>
<script>
var shadowRoot = new WebKitShadowRoot(container);
shadowRoot.innerHTML = "<span contenteditable>HOGE</span>";
</script>


Select somewhere from after the container to the container. (Don't select HOGE but container.)
And press DELETE.

[Shinya Kawanaka]

When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code...

[Ryosuke Niwa]

(In reply to comment #1)
> When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code...

That's probably because we check the existence of the renderer :(

[Shinya Kawanaka]

(In reply to comment #2)
> (In reply to comment #1)
> > When an element does not have a shadow root, we cannot select contenteditable element in this example... So I think it is odd that contenteditable can be selected by this code...
> 
> That's probably because we check the existence of the renderer :(

Yeah... probably so. I'll investigate this more today.

[Shinya Kawanaka]

Created attachment 145724 [details]
Patch

[Ryosuke Niwa]

Comment on attachment 145724 [details]
Patch

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

> Source/WebCore/editing/VisibleSelection.cpp:457
> +        // boundaries again. See http://wkb.ug/87463

Can we use the regular webkit bug URL instead? I don't see a benefit in using a short URL in a comment like this.

> LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:8
> +<p>This test confirms that selecting an element having Shadow DOM doesn't cross editing boundaries errornously.</p>

Should we say something along the line of this test passes if WebKit didn't crash?

> LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:34
> +// Checks crash won't happen.
> +document.execCommand('delete');

You should use debug() to print out something to indicate we passed the test.

> LayoutTests/editing/shadow/select-contenteditable-shadowhost.html:46
> +// Check crash won't happen.
> +document.execCommand('delete');

Ditto.

[Shinya Kawanaka]

Created attachment 146209 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 146209 [details]
Patch for landing

Clearing flags on attachment: 146209

Committed r119711: <http://trac.webkit.org/changeset/119711>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.