Bug 80502 – Editing caret should skip elements with shadow dom.

• Status: NEW
• Component: HTML Editing

[Hajime Morrita]

Currently caret isn't aware the shadow and its movement is broken over the shadow-backed elements.
It should behave as if it is a replaced element.

[Hajime Morrita]

Created attachment 130576 [details]
Patch

[Hajime Morrita]

Hi Dimitri, could you take a look?
Maybe we could be better by take <content> in account somehow butI have no clear idea yet. 
And current behavior is simple broken. I thinks It's worth fixing.

[WebKit Review Bot]

Comment on attachment 130576 [details]
Patch

Attachment 130576 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11835964

New failing tests:
svg/text/text-path-01-b.svg
svg/custom/use-clipped-hit.svg

[Ryosuke Niwa]

Comment on attachment 130576 [details]
Patch

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

> Source/WebCore/html/HTMLDetailsElement.cpp:163
> +    // The UA Shadow guarantees all content is distributed, but it's not true for author shadows.
> +    return 1 == shadowTree()->size();

I don't understand. Why it's okay for selection to be inside the light dom when there are more than 1 shadow trees?

> LayoutTests/editing/selection/move-over-shadow.html:6
> +<div id="container" contentEditable>A<span id="host1"></span><span id="host2"></span>C</div>

This test is kind of bogus. Because spans are empty, WebKit will never put selection endpoints inside them even without this change.

> LayoutTests/editing/selection/move-over-shadow.html:8
> +<script><!--

<!--?

> LayoutTests/editing/selection/move-over-shadow.html:18
> +var sel = window.getSelection();

Please do't use abbreviations like sel.

[Ryosuke Niwa]

Comment on attachment 130576 [details]
Patch

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

>> LayoutTests/editing/selection/move-over-shadow.html:6
>> +<div id="container" contentEditable>A<span id="host1"></span><span id="host2"></span>C</div>
> 
> This test is kind of bogus. Because spans are empty, WebKit will never put selection endpoints inside them even without this change.

I guess without this patch, selection code would have moved from ("A", 0) to ("C", 0) in one step. But we should still test the case when spans have contents.

[Dimitri Glazkov (Google)]

Comment on attachment 130576 [details]
Patch

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

>> Source/WebCore/html/HTMLDetailsElement.cpp:163
>> +    return 1 == shadowTree()->size();
> 
> I don't understand. Why it's okay for selection to be inside the light dom when there are more than 1 shadow trees?

I don't understand either :) IMG, VIDEO, and AUDIO all have the content not distributed?

[Hajime Morrita]

OK, maybe we could have more robust way to do this...

[Hajime Morrita]

Created attachment 131313 [details]
Patch

[Hajime Morrita]

Generalized the last version.
The most naive approach would be actually traverse whole content and find possible caret position.
But I don't want it because it doubles the traversal cost.

[WebKit Review Bot]

Comment on attachment 131313 [details]
Patch

Attachment 131313 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11939835

New failing tests:
svg/text/text-path-01-b.svg
svg/custom/use-clipped-hit.svg

[WebKit Review Bot]

Comment on attachment 131313 [details]
Patch

Attachment 131313 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11940856

New failing tests:
svg/text/text-path-01-b.svg
svg/custom/use-clipped-hit.svg