Content in disabled input field should not be selectable.

Content in disabled input field should not be selectable.

Spec: https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr-input-readonly

BUG=626581
Committed: https://crrev.com/21d670d3e740b26dcdf63631d163c45a57d9531c
Cr-Commit-Position: refs/heads/master@{#405717}

[ramya.v, 2016-07-14 08:51:58]

Description was changed from

==========
Content in disabled input field should not be selectable.

Spec: https://html.spec.whatwg.org/multipage/forms.html

BUG=626581
==========

to

==========
Content in disabled input field should not be selectable.

Spec:
https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr...

BUG=626581
==========

[ramya.v, 2016-07-14 09:00:28]

ramya.v@samsung.com changed reviewers:
+ tkent@chromium.org

[ramya.v, 2016-07-14 09:00:28]

PTAL!

Thanks

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
(right):

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html:45:
<textarea id="readOnlyDisabledTextArea" cols="31" readonly
disabled>ReadonlyDisabledTextarea</textarea>
For Disabled and ReadOnlyDisabled textarea still selection is happening on
longpress, because innerNode is coming as #text node in case of textarea.
However in case of <input> innerNode is coming as div hence working.

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/editing/S...

Shall I raise a separate bug for long-press scenario on disabled textarea?

[tkent, 2016-07-14 09:07:27]

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
(right):

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html:45:
<textarea id="readOnlyDisabledTextArea" cols="31" readonly
disabled>ReadonlyDisabledTextarea</textarea>
> Shall I raise a separate bug for long-press scenario on disabled textarea?

No. We should fix it in this CL.

Maybe we should check shadowHost()->isDisabledFormControl() in
canStartSelection()?

[ramya.v, 2016-07-14 09:15:41]

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
(right):

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html:45:
<textarea id="readOnlyDisabledTextArea" cols="31" readonly
disabled>ReadonlyDisabledTextarea</textarea>
On 2016/07/14 09:07:27, tkent wrote:
> > Shall I raise a separate bug for long-press scenario on disabled textarea?
> 
> No. We should fix it in this CL.
> 
> Maybe we should check shadowHost()->isDisabledFormControl() in
> canStartSelection()?

In this case because of innerNode->isTextNode check it is proceeding for
selection. (canStartSelection and contentEditable checks are false only)
 bool innerNodeIsSelectable = innerNode && (innerNode->isContentEditable() ||
innerNode->isTextNode() || innerNode->canStartSelection());

[ramya.v, 2016-07-14 12:02:41]

Patchset #3 (id:40001) has been deleted

[tkent, 2016-07-14 17:00:09]

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
(right):

https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html:45:
<textarea id="readOnlyDisabledTextArea" cols="31" readonly
disabled>ReadonlyDisabledTextarea</textarea>
On 2016/07/14 at 09:15:41, ramya.v wrote:
> On 2016/07/14 09:07:27, tkent wrote:
> > > Shall I raise a separate bug for long-press scenario on disabled textarea?
> > 
> > No. We should fix it in this CL.
> > 
> > Maybe we should check shadowHost()->isDisabledFormControl() in
> > canStartSelection()?
> 
> In this case because of innerNode->isTextNode check it is proceeding for
selection. (canStartSelection and contentEditable checks are false only)
>  bool innerNodeIsSelectable = innerNode && (innerNode->isContentEditable() ||
innerNode->isTextNode() || innerNode->canStartSelection());

Oh, I see.
Probably the code here should be just
    bool innerNodeIsSelectable = innerNode && innerNode->isContentEditable();

[tkent, 2016-07-14 17:00:49]

On 2016/07/14 at 17:00:09, tkent wrote:
>
https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
> File
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html
(right):
> 
>
https://codereview.chromium.org/2145323002/diff/20001/third_party/WebKit/Layo...
>
third_party/WebKit/LayoutTests/editing/selection/readonly-disabled-text-selection.html:45:
<textarea id="readOnlyDisabledTextArea" cols="31" readonly
disabled>ReadonlyDisabledTextarea</textarea>
> On 2016/07/14 at 09:15:41, ramya.v wrote:
> > On 2016/07/14 09:07:27, tkent wrote:
> > > > Shall I raise a separate bug for long-press scenario on disabled
textarea?
> > > 
> > > No. We should fix it in this CL.
> > > 
> > > Maybe we should check shadowHost()->isDisabledFormControl() in
> > > canStartSelection()?
> > 
> > In this case because of innerNode->isTextNode check it is proceeding for
selection. (canStartSelection and contentEditable checks are false only)
> >  bool innerNodeIsSelectable = innerNode && (innerNode->isContentEditable()
|| innerNode->isTextNode() || innerNode->canStartSelection());
> 
> Oh, I see.
> Probably the code here should be just
>     bool innerNodeIsSelectable = innerNode && innerNode->isContentEditable();

oops, should be
   bool innerNodeIsSelectable = innerNode && innerNode->canStartSelection();

[ramya.v, 2016-07-15 05:31:20]

PTAL!

Thanks

[tkent, 2016-07-15 06:24:08]

The CQ bit was checked by tkent@chromium.org

[tkent, 2016-07-15 06:24:09]

lgtm

[commit-bot: I haz the power, 2016-07-15 06:24:19]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-15 07:37:17]

Description was changed from

==========
Content in disabled input field should not be selectable.

Spec:
https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr...

BUG=626581
==========

to

==========
Content in disabled input field should not be selectable.

Spec:
https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr...

BUG=626581
==========

[commit-bot: I haz the power, 2016-07-15 07:37:18]

Committed patchset #4 (id:80001)

[commit-bot: I haz the power, 2016-07-15 07:37:20]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-15 07:39:15]

Description was changed from

==========
Content in disabled input field should not be selectable.

Spec:
https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr...

BUG=626581
==========

to

==========
Content in disabled input field should not be selectable.

Spec:
https://html.spec.whatwg.org/multipage/forms.html#the-readonly-attribute:attr...

BUG=626581

Committed: https://crrev.com/21d670d3e740b26dcdf63631d163c45a57d9531c
Cr-Commit-Position: refs/heads/master@{#405717}
==========

[commit-bot: I haz the power, 2016-07-15 07:39:16]

Patchset 4 (id:??) landed as
https://crrev.com/21d670d3e740b26dcdf63631d163c45a57d9531c
Cr-Commit-Position: refs/heads/master@{#405717}

[yoichio, 2016-10-27 02:14:43]

A revert of this CL (patchset #4 id:80001) has been created in
https://codereview.chromium.org/2455853002/ by yoichio@chromium.org.

The reason for reverting is: It caused regression:
https://bugs.chromium.org/p/chromium/issues/detail?id=656736.