Bug 80502 - Editing caret should skip elements with shadow dom.
Summary: Editing caret should skip elements with shadow dom.
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Hajime Morrita
URL:
Keywords:
Depends on:
Blocks: 82697
  Show dependency treegraph
 
Reported: 2012-03-07 01:02 PST by Hajime Morrita
Modified: 2012-07-17 20:23 PDT (History)
3 users (show)

See Also:


Attachments
Patch (8.22 KB, patch)
2012-03-07 02:28 PST, Hajime Morrita
no flags Details | Formatted Diff | Diff
Patch (10.96 KB, patch)
2012-03-12 04:13 PDT, Hajime Morrita
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hajime Morrita 2012-03-07 01:02:48 PST
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.
Comment 1 Hajime Morrita 2012-03-07 02:28:07 PST
Created attachment 130576 [details]
Patch
Comment 2 Hajime Morrita 2012-03-07 02:30:06 PST
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.
Comment 3 WebKit Review Bot 2012-03-07 04:31:19 PST
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
Comment 4 Ryosuke Niwa 2012-03-07 08:31:07 PST
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.
Comment 5 Ryosuke Niwa 2012-03-07 08:35:03 PST
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.
Comment 6 Dimitri Glazkov (Google) 2012-03-07 08:54:29 PST
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?
Comment 7 Hajime Morrita 2012-03-07 18:50:30 PST
OK, maybe we could have more robust way to do this...
Comment 8 Hajime Morrita 2012-03-12 04:13:42 PDT
Created attachment 131313 [details]
Patch
Comment 9 Hajime Morrita 2012-03-12 04:16:05 PDT
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.
Comment 10 WebKit Review Bot 2012-03-12 05:24:23 PDT
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
Comment 11 WebKit Review Bot 2012-03-12 06:26:33 PDT
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