Set selection to INPUT/TEXTAREA before dispatching 'focus' event.

Set selection to INPUT/TEXTAREA before dispatching 'focus' event.

This CL moves updateFocusAppearance() call from Element::focus() to
Document::setFocusedElement() so that 'focus' event handler can refer to correct
selection information.

The last updateLayoutIgnorePendingStylesheets() call in Element::focus is
unnecessary now.  Because this CL removes it,
textfield-to-password-on-focus.html needs a delay to pour key inputs.

BUG=526516
Committed: https://crrev.com/d76505b8d68f4fffd18620d9ab1b4d19c467ec4c
Cr-Commit-Position: refs/heads/master@{#360545}

[tkent, 2015-11-11 09:57:40]

Patchset #1 (id:1) has been deleted

[tkent, 2015-11-11 16:08:44]

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

[tkent, 2015-11-11 16:08:45]

kochi@, yoichio@, would you review this please?

[kochi, 2015-11-11 23:23:21]

Who can set selectionBehavior to SelectionBehaviorOnFocus::None
now?

[kochi, 2015-11-11 23:24:03]

On 2015/11/11 23:23:21, Takayoshi Kochi wrote:
> Who can set selectionBehavior to SelectionBehaviorOnFocus::None
> now?

s/Who can set/Who sets/

[kochi, 2015-11-11 23:32:36]

On 2015/11/11 23:24:03, Takayoshi Kochi wrote:
> On 2015/11/11 23:23:21, Takayoshi Kochi wrote:
> > Who can set selectionBehavior to SelectionBehaviorOnFocus::None
> > now?
> 
> s/Who can set/Who sets/

Ah, ok, I understand.  There are many users of SelectionBehaviorOnFocus::None
but now that the updateFocusAppearance() moved, HTMLInputElement and
HTMLTextAreaElement is affected now.

lgtm

[yoichio, 2015-11-17 05:56:40]

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/forms/selection-functions.html (right):

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/selection-functions.html:98:
getSelection().removeAllRanges();
Could you have |elt.selectionStart = elt.selectionEnd = 0 | 
 to assure Element::focus sets selectionStart/End ?

[tkent, 2015-11-17 06:02:45]

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/forms/selection-functions.html (right):

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/selection-functions.html:98:
getSelection().removeAllRanges();
On 2015/11/17 at 05:56:40, yoichio wrote:
> Could you have |elt.selectionStart = elt.selectionEnd = 0 | 
>  to assure Element::focus sets selectionStart/End ?

Setting 0 to them makes unable to check the behavior correctness because we
wrongly returns 0,0 in focus event handler. See crbug.com/526516 .
Adding |selectionStart = selectionEnd = 7| explicitly would make sense.

[tkent, 2015-11-17 06:24:54]

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
File third_party/WebKit/LayoutTests/fast/forms/selection-functions.html (right):

https://codereview.chromium.org/1437813002/diff/20001/third_party/WebKit/Layo...
third_party/WebKit/LayoutTests/fast/forms/selection-functions.html:98:
getSelection().removeAllRanges();
On 2015/11/17 at 06:02:45, tkent wrote:
> On 2015/11/17 at 05:56:40, yoichio wrote:
> > Could you have |elt.selectionStart = elt.selectionEnd = 0 | 
> >  to assure Element::focus sets selectionStart/End ?
> 
> Setting 0 to them makes unable to check the behavior correctness because we
wrongly returns 0,0 in focus event handler. See crbug.com/526516 .
> Adding |selectionStart = selectionEnd = 7| explicitly would make sense.

I added it to new patch set.

[yoichio, 2015-11-18 06:22:40]

> 
> Setting 0 to them makes unable to check the behavior correctness because we
wrongly returns 0,0 in focus event handler. See crbug.com/526516 .
> Adding |selectionStart = selectionEnd = 7| explicitly would make sense.

I'm confused. What do you fix in this patch and enable developpers?

[tkent, 2015-11-18 06:31:57]

On 2015/11/18 at 06:22:40, yoichio wrote:
> > 
> > Setting 0 to them makes unable to check the behavior correctness because we
wrongly returns 0,0 in focus event handler. See crbug.com/526516 .
> > Adding |selectionStart = selectionEnd = 7| explicitly would make sense.
> 
> I'm confused. What do you fix in this patch and enable developpers?

The CL description says:
> so that 'focus' event handler can refer to correct selection information.

Did you read crbug.com/526516 ?

[yoichio, 2015-11-19 07:56:14]

If you confirm that selectionStart and selectionEnd in focus handler should be
end of text,
 |elt.selectionStart = elt.selectionEnd = 6 (any number except 0 or 7);
elt.focus(); ,,,| makes more sense?

[tkent, 2015-11-19 08:04:06]

On 2015/11/19 at 07:56:14, yoichio wrote:
> If you confirm that selectionStart and selectionEnd in focus handler should be
end of text,
>  |elt.selectionStart = elt.selectionEnd = 6 (any number except 0 or 7);
elt.focus(); ,,,| makes more sense?

The element has 48 characters.  Any valid values other than 0,0 are ok.

[tkent, 2015-11-19 08:05:00]

> The element has 48 characters.  Any valid values other than 0,0 are ok.

Correction: other than 0,0 or 48,48

[yoichio, 2015-11-19 08:07:54]

On 2015/11/19 at 08:04:06, tkent wrote:
> On 2015/11/19 at 07:56:14, yoichio wrote:
> > If you confirm that selectionStart and selectionEnd in focus handler should
be end of text,
> >  |elt.selectionStart = elt.selectionEnd = 6 (any number except 0 or 7);
elt.focus(); ,,,| makes more sense?
> 
> The element has 48 characters.  Any valid values other than 0,0 are ok.

Both removeAllRanges() and text.focus() don't move text.selectionStart/End. I
see.

lgtm.

[tkent, 2015-11-19 08:08:56]

The CQ bit was checked by tkent@chromium.org

[tkent, 2015-11-19 08:08:56]

The patchset sent to the CQ was uploaded after l-g-t-m from kochi@chromium.org
Link to the patchset: https://codereview.chromium.org/1437813002/#ps40001
(title: "improve selection-functions.html")

[commit-bot: I haz the power, 2015-11-19 08:09:36]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1437813002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1437813002/40001

[commit-bot: I haz the power, 2015-11-19 10:35:00]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2015-11-19 10:35:50]

Patchset 2 (id:??) landed as
https://crrev.com/d76505b8d68f4fffd18620d9ab1b4d19c467ec4c
Cr-Commit-Position: refs/heads/master@{#360545}