Bug 106287 – [Chromium] Cannot copy text when selecting readonly (or disabled) input elements

• Component: HTML Editing
• Status: RESOLVED FIXED

[Shinya Kawanaka]

This is related to Bug 85244.
When selecting text on disabled input element, we cannot paste the selected string by mid-click on X Window.

Since the input element is disabled, the inner element of the input element is not editable.
So in WebViewImpl::caretOrSelectionRange, rootEditableElementOrDocumentElement() returns document element.
However the range is in different tree scope.

We should return ShadowRoot instead of document here.

[Shinya Kawanaka]

Created attachment 181620 [details]
Patch

[Build Bot]

Comment on attachment 181620 [details]
Patch

Attachment 181620 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/15736950

[Build Bot]

Comment on attachment 181620 [details]
Patch

Attachment 181620 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/15738967

[Peter Beverloo (cr-android ews)]

Comment on attachment 181620 [details]
Patch

Attachment 181620 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15734946

[Shinya Kawanaka]

Created attachment 181829 [details]
Build Test

[Peter Beverloo (cr-android ews)]

Comment on attachment 181829 [details]
Build Test

Attachment 181829 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/15757376

[Shinya Kawanaka]

Created attachment 181842 [details]
Build Test

[Shinya Kawanaka]

Created attachment 181852 [details]
Patch

[Hajime Morrita]

Comment on attachment 181852 [details]
Patch

Can we make the tests LayoutTest?

[Dominic Cooney]

Comment on attachment 181852 [details]
Patch

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

I am not a reviewer, but I have two comments and a question inline.

> Source/WebCore/ChangeLog:3
> +        [Chromium] Cannot copy text when selecting readonly (or disable) input elements.

disable => disabled

> Source/WebCore/ChangeLog:13
> +        We should use ShadowRoot instead of document so that we can stay in the same tree scope.

Nice explanation of the issue.

> Source/WebCore/editing/FrameSelection.h:138
>      Element* rootEditableElementOrDocumentElement() const;

What is your opinion about this method? It has many callers. Are they all likely to need updating? (Not in this patch, of course.)

[Shinya Kawanaka]

Comment on attachment 181852 [details]
Patch

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

>> Source/WebCore/editing/FrameSelection.h:138
>>      Element* rootEditableElementOrDocumentElement() const;
> 
> What is your opinion about this method? It has many callers. Are they all likely to need updating? (Not in this patch, of course.)

Basically they should be updated.

[Shinya Kawanaka]

Created attachment 185129 [details]
Patch

[Hajime Morrita]

Comment on attachment 185129 [details]
Patch

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

> Source/WebKit/chromium/tests/WebViewTest.cpp:694
> +    EXPECT_EQ(length, 29UL);

Let's do some strlen()-like thing for explaining the magic number.

[Shinya Kawanaka]

Created attachment 185131 [details]
Patch

[Build Bot]

Comment on attachment 185131 [details]
Patch

Attachment 185131 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/16163910

[Shinya Kawanaka]

Created attachment 185146 [details]
Patch for landing

[Shinya Kawanaka]

Waiting for bots green

[WebKit Review Bot]

Comment on attachment 185146 [details]
Patch for landing

Clearing flags on attachment: 185146

Committed r141196: <http://trac.webkit.org/changeset/141196>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.