Bug 106526 – When a selected node in nested ShadowDOM is deleted, selection have wrong range.

• Component: DOM
• Status: RESOLVED FIXED

[Shinya Kawanaka]

selection will contain deleted host element.
If node in non-nested ShadowDOM is deleted, selection contains BODY, though.

[Shinya Kawanaka]

Created attachment 182064 [details]
Patch

[Dimitri Glazkov (Google)]

Ryosuke, can you take a look?

[Ryosuke Niwa]

Comment on attachment 182064 [details]
Patch

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

> Source/WebCore/editing/FrameSelection.cpp:341
> -    return element->contains(position.anchorNode()) || element->contains(position.anchorNode()->shadowAncestorNode());
> +    return element->containsIncludingShadowDOM(position.anchorNode());

This is strange since selection & editing operations are limited to one level of shadow DOM. Why should removing a node that contains shadow DOM update the selection of that removed shadow DOM?

[Shinya Kawanaka]

I think the original code did not consider nested Shadow DOM case at all.

Let's consider the following example.

div1 -- SR
        |-- div2 -- SR
                    |- div3
                         |-- #text

When text in div3 is selected, and div1 is removed from the tree.
removingNodeRemovesPosition() is called with node = div1, and position = somewhere in #text.
In this case, 
  element->contains(position.anchorNode()) || element->contains(position.anchorNode()->shadowAncestorNode());
returns false. However, this is strange. when div1 is removed, position in #text also should be removed, I think.

[Ryosuke Niwa]

(In reply to comment #4)
> I think the original code did not consider nested Shadow DOM case at all.
> 
> Let's consider the following example.
> 
> div1 -- SR
>         |-- div2 -- SR
>                     |- div3
>                          |-- #text
> 
> When text in div3 is selected, and div1 is removed from the tree.
> removingNodeRemovesPosition() is called with node = div1, and position = somewhere in #text.
> In this case, 
>   element->contains(position.anchorNode()) || element->contains(position.anchorNode()->shadowAncestorNode());
> returns false. However, this is strange. when div1 is removed, position in #text also should be removed, I think.

But whatever position we adjust to inside SR for div2 is meaningless at that point, right? I mean why should detaching an ancestor of the shadow host should affect how selection works inside a shadow DOM? It seems like positions and selections inside div2's shadow root should be completely independent of what happens outside of the shadow root, no?

[Shinya Kawanaka]

> But whatever position we adjust to inside SR for div2 is meaningless at that point, right? I mean why should detaching an ancestor of the shadow host should affect how selection works inside a shadow DOM? It seems like positions and selections inside div2's shadow root should be completely independent of what happens outside of the shadow root, no?

However, since the selected node is removed from the Document, we have to adjust FrameSelection, right?
I think we have to move FrameSelection to around div1 (or remove Selection). To achieve it, we have to check selection is in Shadow DOM.

Currently we don't move Selection if it's in nested ShadowDOM but we move Selection if it's in non-nested Shadow DOM. This is apparently wrong.

[Ryosuke Niwa]

Sorry, I misunderstood your change. I was thinking about shadowRoot.selection but your change affects FrameSelection, the one and the only selection per frame.

[Ryosuke Niwa]

Comment on attachment 182064 [details]
Patch

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

> LayoutTests/fast/dom/shadow/selection-in-nested-shadow-expected.txt:3
> +PASS anchorNode1 is container
> +PASS anchorNode2 is container
> +PASS successfullyParsed is true

This output isn't so helpful. Please give more descriptive variable names.

[Shinya Kawanaka]

Created attachment 182251 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 182251 [details]
Patch for landing

Clearing flags on attachment: 182251

Committed r139401: <http://trac.webkit.org/changeset/139401>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.