INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is changed.

INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() to change
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102
Review-Url: https://codereview.chromium.org/2735633006
Cr-Commit-Position: refs/heads/master@{#455380}
Committed: https://chromium.googlesource.com/chromium/src/+/158aa81d2e11bd3cce433df4801cbffc9922d86d

[tkent, 2017-03-07 01:16:13]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 01:17:02]

Dry run: 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, 2017-03-07 02:41:38]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 02:41:39]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[tkent, 2017-03-07 03:47:06]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 03:48:04]

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

[tkent, 2017-03-07 04:54:27]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectedRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* 

BUG=696102
==========

[tkent, 2017-03-07 04:57:10]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectedRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* 

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* 

BUG=696102
==========

[tkent, 2017-03-07 05:02:32]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* 

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

BUG=696102
==========

[tkent, 2017-03-07 05:09:50]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* 

BUG=696102
==========

[tkent, 2017-03-07 05:10:21]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* 

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense any longer.

BUG=696102
==========

[tkent, 2017-03-07 05:10:35]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense any longer.

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102
==========

[tkent, 2017-03-07 05:21:14]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() changes
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() to
change
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102
==========

[tkent, 2017-03-07 05:25:45]

Patchset #1 (id:1) has been deleted

[tkent, 2017-03-07 05:32:42]

tkent@chromium.org changed reviewers:
+ yoichio@chromium.org

[tkent, 2017-03-07 05:32:43]

yoichio@, would you review this please?

I think intent-to-ship isn't necessary. This just reduces the number of event
dispatches.

[commit-bot: I haz the power, 2017-03-07 06:59:09]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 06:59:11]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[yoichio, 2017-03-07 09:00:56]

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html
(left):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html:15:
input.selectionEnd = 3;
This test confirms throttle event queueing.
Could you change this to 
 input.value = "foo"
 input.value = "bar" ?
Even that situation, no doubling?

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/TextControlElement.h (right):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/TextControlElement.h:51: kSetSelectionToEnd,
Why do you use another coding style to above enums?

[tkent, 2017-03-07 09:27:34]

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html
(left):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html:15:
input.selectionEnd = 3;
On 2017/03/07 at 09:00:56, yoichio wrote:
> This test confirms throttle event queueing.
> Could you change this to 
>  input.value = "foo"
>  input.value = "bar" ?
> Even that situation, no doubling?

The throttling itself is unnecessary.  I'll update the code.

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/core/html/TextControlElement.h (right):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/core/html/TextControlElement.h:51: kSetSelectionToEnd,
On 2017/03/07 at 09:00:56, yoichio wrote:
> Why do you use another coding style to above enums?

kFooBar is the standard naming convention.  FooBar style will be disappear soon.

[tkent, 2017-03-07 09:31:22]

The CQ bit was checked by tkent@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-07 09:32:28]

Dry run: 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, 2017-03-07 11:24:01]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-07 11:24:02]

Dry run: Try jobs failed on following builders:
  win_chromium_x64_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[tkent, 2017-03-07 11:30:08]

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html
(left):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html:15:
input.selectionEnd = 3;
On 2017/03/07 at 09:27:34, tkent wrote:
> On 2017/03/07 at 09:00:56, yoichio wrote:
> > This test confirms throttle event queueing.
> > Could you change this to 
> >  input.value = "foo"
> >  input.value = "bar" ?
> > Even that situation, no doubling?
> 
> The throttling itself is unnecessary.  I'll update the code.

Done.  Uploaded new patch set.

[yoichio, 2017-03-08 02:00:36]

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html
(left):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html:15:
input.selectionEnd = 3;
Do you think this kind of flood is friends of infinite
 loop and web authors should check it themselves?
https://bugs.chromium.org/p/chromium/issues/detail?id=449470

[tkent, 2017-03-08 02:05:23]

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
File
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html
(left):

https://codereview.chromium.org/2735633006/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/events/select-event-on-input-recursive.html:15:
input.selectionEnd = 3;
On 2017/03/08 at 02:00:36, yoichio wrote:
> Do you think this kind of flood is friends of infinite
>  loop and web authors should check it themselves?
> https://bugs.chromium.org/p/chromium/issues/detail?id=449470

This kind of flood won't happen after this CL because setting same values to
selectionStart/End won't dispatch events.
It's the purpose of the spec change.

[yoichio, 2017-03-08 03:46:13]

lgtm

[tkent, 2017-03-08 04:32:49]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2017-03-08 04:33:07]

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, 2017-03-08 04:39:49]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1488947569853770,
"parent_rev": "5c4056a61c3d24b5df4ebdba406467b96ff68866", "commit_rev":
"158aa81d2e11bd3cce433df4801cbffc9922d86d"}

[commit-bot: I haz the power, 2017-03-08 04:41:16]

Description was changed from

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() to
change
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102
==========

to

==========
INPUT/TEXTAREA elements: Dispatch 'select' event only if text selection is
changed.

* TextControlElement::cacheSelection() and setSelectionRange()
  This is the main change of this CL. We dispatch 'select' event only if new
values
 are not identical to old values.

* TextControlElement::setRangeText()
  Add a new flag to TextControlElement::setValue() in order to control text
  selection change in setValue(). setRangeText() doesn't want setValue() to
change
  text selection.

* web-platform-tests html/semantics/forms/textfieldselection/select-event.html
 - Mark this slow.  This test make sure that |select| events are NOT dispatched.
  So it takes long time.
 - Correct "right" to "backward" to avoid the unknown keyword is recognized as
  the default keyword.
 - Shorten setTimeout value.

* LayoutTests/fast/events/select-event-on-input-and-textarea.html
 Removed because this is covered by the above test.

* LayoutTests/fast/events/select-event-on-input-recursive.html
 Removed. This test doesn't make sense since this CL.

BUG=696102

Review-Url: https://codereview.chromium.org/2735633006
Cr-Commit-Position: refs/heads/master@{#455380}
Committed:
https://chromium.googlesource.com/chromium/src/+/158aa81d2e11bd3cce433df4801c...
==========

[commit-bot: I haz the power, 2017-03-08 04:41:17]

Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/158aa81d2e11bd3cce433df4801c...

[jeffcarp, 2017-03-08 19:31:54]

On 2017/03/08 at 04:41:17, commit-bot wrote:
> Committed patchset #2 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/158aa81d2e11bd3cce433df4801c...

This CL failed the WPT stability check[1]. I think it might be due to the
decreased setTimeout value. I'm going to increase it in the current patch[2] so
that it kicks off another Travis CI run.

[1] https://travis-ci.org/w3c/web-platform-tests/jobs/208848753
[2] https://github.com/w3c/web-platform-tests/pull/5068

[tkent, 2017-03-09 00:28:04]

On 2017/03/08 at 19:31:54, jeffcarp wrote:
> This CL failed the WPT stability check[1]. I think it might be due to the
decreased setTimeout value. I'm going to increase it in the current patch[2] so
that it kicks off another Travis CI run.
> 
> [1] https://travis-ci.org/w3c/web-platform-tests/jobs/208848753
> [2] https://github.com/w3c/web-platform-tests/pull/5068

Thank you for the fix!  Can wpt-exporter send a pull-request status to a patch
author?  It's hard for a Chromium patch author to know export failures.

[jeffcarp, 2017-03-09 00:53:06]

On 2017/03/09 at 00:28:04, tkent wrote:
> On 2017/03/08 at 19:31:54, jeffcarp wrote:
> > This CL failed the WPT stability check[1]. I think it might be due to the
decreased setTimeout value. I'm going to increase it in the current patch[2] so
that it kicks off another Travis CI run.
> > 
> > [1] https://travis-ci.org/w3c/web-platform-tests/jobs/208848753
> > [2] https://github.com/w3c/web-platform-tests/pull/5068
> 
> Thank you for the fix!  Can wpt-exporter send a pull-request status to a patch
author?  It's hard for a Chromium patch author to know export failures.

I'm currently looking into moving the exporter to Gerrit - part of the plan is
to comment status updates on the CL (e.g. 'travis failed' or 'export succeeded')