Generate focusin for input type=date/time when selected by tab

Generate focusin for input type=date/time when selected by tab

setFocusedElement fails to generate focusin event as the element in focus changes. 

This patch creates new handler for focusin event, which gets generated from handleFocusInEvent. 

* Source/core/dom/Document.cpp
* Source/core/dom/Element.h
* Source/core/dom/Element.cpp
* Souce/core/page/FocusController.cpp
  dispatchFocusInEvent passes extra parameter FocusType, as handleFocusInEvent needs this.

* Source/core/html/HTMLInputElement.h
* Source/core/html/HTMLInputElement.cpp
  dispatchFocusInEvent, calls handleFocusInEvent handler when event is DOMFocusIn. Name of the function is handlerFocusInEvent, just in case if DOMFocusIn is deprecated only event used before dispatching handleFocusInEvent needs to be changed.

* Source/core/html/forms/InputTypeView.h
* Source/core/html/forms/InputTypeView.cpp
  handleFocusInEvent call pass to the form.

* Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.h
* Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp
  Removed handleFocusEvent and moved its code to handleFocusInEvent handler. 

BUG=408107
R=tkent, keishi
TEST=Test covering date and time field when moved into by pressing tab, generates focusin event.

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182023

[Habib Virji, 2014-09-08 08:35:07]

Please have a look.

[tkent, 2014-09-08 08:45:42]

This is not a correct fix.  We need to take care of "DOMFocusIn" event too.

Now we move the focus to a shadow element in "focus' event handler of the shadow
host element. Probably we should move it to "DOMFocusIn" event handler, which is
the last focus event.  See Document::setFocusedElement.

[Habib Virji, 2014-09-08 09:22:49]

Thanks for quick review.

"Now we move the focus to a shadow element in "focus' event handler of the
shadow host element"
-> Yes that's true, focus is handled in shadow element.
DateTimeEditElement::focusedOnNextField gets called from
Document:setFocusedElement, dispatchFocusEvent.

"Probably we should move it to "DOMFocusIn" event handler, which is the last
focus event."
-> Commit description is based on behavior in Document:setFocusedEleemnt,
focusin and DOMFocusIn are not getting generated. Reason being
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...,
dispatchFocusEvent handler results in call to
DateTimeEditElement::focusOnNextFocusableField, which moves focus to next
element. This is resulting in "m_focusedElement != newFocusedElement", and it
does not dispatch focusin events.

I am not sure what you meant by move to DOMFocusIn, as both events(focusin and
DOMFocusIn) are generated together. Since one is not happening, neither does
other happen.

[tkent, 2014-09-09 01:12:50]

On 2014/09/08 09:22:49, Habib Virji wrote:
> Thanks for quick review.
> 
> "Now we move the focus to a shadow element in "focus' event handler of the
> shadow host element"
> -> Yes that's true, focus is handled in shadow element.
> DateTimeEditElement::focusedOnNextField gets called from
> Document:setFocusedElement, dispatchFocusEvent.
> 
> "Probably we should move it to "DOMFocusIn" event handler, which is the last
> focus event."
> -> Commit description is based on behavior in Document:setFocusedEleemnt,
> focusin and DOMFocusIn are not getting generated.

Right.  It's the root cause of the bug.  So, we should shift the focus after
dispatching "DOMFocusIn" event targeting <input>, not after dispatching "focus"
event targeting <input>.

We should not dispatch "focusin" and "DOMFocusIn" in places other than
Document::setFocusedElement() for maintenability.

[Habib Virji, 2014-09-09 16:21:14]

Your suggestion makes sense, but sorry did not fathom comment#2 correctly. 

* Source/core/dom/Document.cpp
* Source/core/dom/Element.h
* Source/core/dom/Element.cpp
* Souce/core/page/FocusController.cpp
  dispatchFocusInEvent passes extra parameter FocusType, as handleFocusInEvent
needs this.

* Source/core/html/HTMLInputElement.h
* Source/core/html/HTMLInputElement.cpp
* Source/core/html/forms/InputTypeView.h
* Source/core/html/forms/InputTypeView.cpp
  handleFocusInEvent call pass to the form.

* Source/core/html/HTMLTextFormControlElement.h
* Source/core/html/HTMLTextFormControlElement.cpp
  dispatchFocusInEvent, calls handleFocusInEvent handler when event is
DOMFocusIn. Name of the function is handlerFocusInEvent, just in case if
DOMFocusIn is deprecated only event used before dispatching handleFocusInEvent
needs to be changed.

* Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.h
* Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp
  Removed handleFocusEvent and moved its code to handleFocusInEvent handler.

[tkent, 2014-09-09 23:00:08]

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
File Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:98: void
HTMLTextFormControlElement::dispatchFocusInEvent(const AtomicString& eventType,
Element* oldFocusedElement, FocusType type)
This function should be moved to HTMLInputElement.

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:101:
handleFocusInEvent(oldFocusedElement, type);
Move this function to HTMLInputElmenet, and this line should be
m_inputTypeView->handleFocusInEvent(...)

[Habib Virji, 2014-09-10 10:00:48]

Thanks, code updated.

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
File Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:98: void
HTMLTextFormControlElement::dispatchFocusInEvent(const AtomicString& eventType,
Element* oldFocusedElement, FocusType type)
On 2014/09/09 23:00:08, tkent (overloaded) wrote:
> This function should be moved to HTMLInputElement.

Done.

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:101:
handleFocusInEvent(oldFocusedElement, type);
On 2014/09/09 23:00:07, tkent (overloaded) wrote:
> Move this function to HTMLInputElmenet, and this line should be
> m_inputTypeView->handleFocusInEvent(...)
> 

Was already part of patch#2.

[tkent, 2014-09-10 23:47:11]

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
File Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:101:
handleFocusInEvent(oldFocusedElement, type);
On 2014/09/10 10:00:48, Habib Virji wrote:
> On 2014/09/09 23:00:07, tkent (overloaded) wrote:
> > Move this function to HTMLInputElmenet, and this line should be
> > m_inputTypeView->handleFocusInEvent(...)
> > 
> 
> Was already part of patch#2.

You misunderstood my comment.

https://codereview.chromium.org/541993003/diff/60001/LayoutTests/fast/forms/d...
File
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html
(right):

https://codereview.chromium.org/541993003/diff/60001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:5:
<input type="date" id="date" onfocusin="onfocusin()">
Please listen to DOMFocusIn event too.

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:394: handleFocusInEvent(oldFocusedElement,
type);
This line should be replaced with m_inputTypeView->handleFocusInEvent(...).

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:398: void
HTMLInputElement::handleFocusInEvent(Element* oldFocusedElement, FocusType type)
This function is unnecessary.
The function body is just 1 line, and no callers other than
dispatchFocusInEvent().

[Habib Virji, 2014-09-11 09:12:22]

Thanks and apologise for being short sighted, handleFocusInEvent was not needed
did not see it.  Code updated, please have a look.

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
File Source/core/html/HTMLTextFormControlElement.cpp (right):

https://codereview.chromium.org/541993003/diff/20001/Source/core/html/HTMLTex...
Source/core/html/HTMLTextFormControlElement.cpp:101:
handleFocusInEvent(oldFocusedElement, type);
On 2014/09/10 23:47:10, tkent (overloaded) wrote:
> On 2014/09/10 10:00:48, Habib Virji wrote:
> > On 2014/09/09 23:00:07, tkent (overloaded) wrote:
> > > Move this function to HTMLInputElmenet, and this line should be
> > > m_inputTypeView->handleFocusInEvent(...)
> > > 
> > 
> > Was already part of patch#2.
> 
> You misunderstood my comment.

Done.

https://codereview.chromium.org/541993003/diff/60001/LayoutTests/fast/forms/d...
File
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html
(right):

https://codereview.chromium.org/541993003/diff/60001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:5:
<input type="date" id="date" onfocusin="onfocusin()">
On 2014/09/10 23:47:10, tkent (overloaded) wrote:
> Please listen to DOMFocusIn event too.

Done.

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:394: handleFocusInEvent(oldFocusedElement,
type);
On 2014/09/10 23:47:11, tkent (overloaded) wrote:
> This line should be replaced with m_inputTypeView->handleFocusInEvent(...).

Done.

https://codereview.chromium.org/541993003/diff/60001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:398: void
HTMLInputElement::handleFocusInEvent(Element* oldFocusedElement, FocusType type)
On 2014/09/10 23:47:11, tkent (overloaded) wrote:
> This function is unnecessary.
> The function body is just 1 line, and no callers other than
> dispatchFocusInEvent().

Done.

[tkent, 2014-09-11 23:52:55]

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
File
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html
(right):

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:11:
var input = document.querySelector('input');
This is not used.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:34:
eventSender.keyDown('\t'); //move to date input element
Add space after //.
Add period at the end of the comment.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:38:
eventSender.keyDown('\t'); //move to date field
Ditto.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:39:
eventSender.keyDown('\t'); //move to  year field
Ditto.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:42:
eventSender.keyDown('\t'); //move to time field
Ditto.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:47:
eventSender.keyDown('\t'); //move to hour field
Ditto.

If you don't check resultant dispatchedFocusInEvent or dispatchedDOMFocusInEvent
value, these two TAB presses are unnecessary.

https://codereview.chromium.org/541993003/diff/80001/Source/core/html/HTMLInp...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/541993003/diff/80001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:393: if (eventType == 
EventTypeNames::DOMFocusIn)
Two spaces after == should be one.

[Habib Virji, 2014-09-12 08:18:26]

Thanks, updated as suggested.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
File
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html
(right):

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:11:
var input = document.querySelector('input');
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> This is not used.

Done.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:34:
eventSender.keyDown('\t'); //move to date input element
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> Add space after //.
> Add period at the end of the comment.

Done.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:38:
eventSender.keyDown('\t'); //move to date field
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> Ditto.

Done.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:39:
eventSender.keyDown('\t'); //move to  year field
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> Ditto.

Done.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:42:
eventSender.keyDown('\t'); //move to time field
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> Ditto.

Done.

https://codereview.chromium.org/541993003/diff/80001/LayoutTests/fast/forms/d...
LayoutTests/fast/forms/date-multiple-fields/date-multiple-fields-focusin-event.html:47:
eventSender.keyDown('\t'); //move to hour field
On 2014/09/11 23:52:54, tkent (overloaded) wrote:
> Ditto.
> 
> If you don't check resultant dispatchedFocusInEvent or
dispatchedDOMFocusInEvent
> value, these two TAB presses are unnecessary.

Done.

https://codereview.chromium.org/541993003/diff/80001/Source/core/html/HTMLInp...
File Source/core/html/HTMLInputElement.cpp (right):

https://codereview.chromium.org/541993003/diff/80001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:393: if (eventType == 
EventTypeNames::DOMFocusIn)
On 2014/09/11 23:52:55, tkent (overloaded) wrote:
> Two spaces after == should be one.

Done.

[Habib Virji, 2014-09-12 10:05:54]

Patchset #7 (id:120001) has been deleted

[Habib Virji, 2014-09-12 10:06:04]

Patchset #6 (id:100001) has been deleted

[tkent, 2014-09-15 23:33:18]

The CQ bit was checked by tkent@chromium.org

[tkent, 2014-09-15 23:33:19]

lgtm

[commit-bot: I haz the power, 2014-09-15 23:33:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patchset/541993003/140001

[commit-bot: I haz the power, 2014-09-16 00:21:57]

Committed patchset #6 (id:140001) as 182023