Dispatch change and blur event for input type=text when window focus is changed

Dispatch change and blur event for input type=text when window focus is changed

FocusController::dispatchEventsOnWindowAndFocusedNode now dispatches setFocus(false) when window loses focus and setFocus(true) when window focus is back. 

R=tkent, keishi1
BUG=102830
TEST=Window focus is lost and text is updated by user, dispatch both blur and change event

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

[Habib Virji, 2014-03-20 11:28:28]

Dispatch change and blur event, if value is changed of the text and window focus
is lost.

It does not duplicate changeEvent, in other scenarios as 
m_wasChangedSinceLastFormControlChangeEvent gets set if change event has already
been dispatched. 

Please note if value if updated via JS, this CL will not dispatch change event.
It only does if it is user input.

[tkent, 2014-03-25 23:38:43]

Can we call Element::setFocus(false) in dispatchEventsOnWindowAndFocusedNode of
FocusController.cpp? How other browsers work?

[Habib Virji, 2014-03-26 11:53:36]

On 2014/03/25 23:38:43, tkent wrote:
> Can we call Element::setFocus(false) in dispatchEventsOnWindowAndFocusedNode
of
> FocusController.cpp? How other browsers work?

- Firefox generates both change and blur event. 
- IE behaves like chrome, only blur event is dispatched and no change event.
- The only thing from not changing dispatchEventsOnWindowAndFocusedNode, was
value details have to be known before dispatching. 

Will look further if wasChangedSinceLastFormControlChangeEvent can be accessed,
else unwanted change event could be dispatched.

[Habib Virji, 2014-03-26 16:26:39]

Updated to dispatch change event from FocusController. Did not use setFocus
because had to use formControl to check if value is changed.

[tkent, 2014-03-27 00:44:54]

On 2014/03/26 11:53:36, Habib Virji wrote:
> On 2014/03/25 23:38:43, tkent wrote:
> > Can we call Element::setFocus(false) in dispatchEventsOnWindowAndFocusedNode
> of
> > FocusController.cpp? How other browsers work?
> 
> - Firefox generates both change and blur event. 
> - IE behaves like chrome, only blur event is dispatched and no change event.
> - The only thing from not changing dispatchEventsOnWindowAndFocusedNode, was
> value details have to be known before dispatching. 

Thank you for the investigation.  How document.querySelector(':focus') works
after dispatching blur event?
If it returns nothing, we may call setFocus(false).

[tkent, 2014-03-27 00:46:03]

https://codereview.chromium.org/205033004/diff/60001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/205033004/diff/60001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:49: #include "core/html/HTMLInputElement.h"
I don't think adding dependency to a specific element to FormController is a
right approach.

[Habib Virji, 2014-03-27 17:10:38]

Uploaded new patch.

Yes that's true setFocus does work, it take care of checking if value is changed
and does not require adding any dependency.

[tkent, 2014-03-31 07:35:37]

I checked behavior of Firefox and IE11.

Suppose an element E has focus, then a browser window gets inactive:

* Firefox: document.activeElement is still E.  document.querySelector(':focus')
doesn't match to E.

* IE: document.activeElement is still E.  document.querySelector(':focus')
matches to E.

The current Blink behavior matches to IE, and I think this CL makes Blink
matches to Firefox.  This behavior change looks reasonable.

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:148:
toHTMLElement(focusedElement)->setFocus(false);
We don't need to check isHTMLElement, and don't need to apply toHTMLElement. 
Element has setFocus.

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:150: if (focusedElement ==
document->focusedElement())
We don't need to check it.  setFocus(false) don't change
Document::focusedElement value.

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:163: focusedElement->dispatchFocusEvent(0,
FocusTypePage);
Because we called setFocus(false) on page blur, we should call setFocus(true) on
page focus.

[tkent, 2014-03-31 07:40:22]

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
File LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html
(right):

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:6: <input
type='text' id='text'/>
The quotation marks should be " for consistency with other attributes.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:18:
text.addEventListener('blur',   function() {
should remove extra space after ,

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:20:
shouldEvaluateTo('blurEventCounter',   1);
extra space after ,

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:25:
document.execCommand("InsertText", false, "hello world");
The quotation marks should be ' for consistency with other JavaScript code.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:26:
shouldEvaluateTo('blurEventCounter',   0);
extra space after ,

[Habib Virji, 2014-03-31 09:30:15]

Thanks. Updated as per review comments. 

Did test setFocus is do working correctly.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
File LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html
(right):

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:6: <input
type='text' id='text'/>
On 2014/03/31 07:40:22, tkent wrote:
> The quotation marks should be " for consistency with other attributes.

Done.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:18:
text.addEventListener('blur',   function() {
On 2014/03/31 07:40:22, tkent wrote:
> should remove extra space after ,

Done.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:20:
shouldEvaluateTo('blurEventCounter',   1);
On 2014/03/31 07:40:22, tkent wrote:
> extra space after ,

Done.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:25:
document.execCommand("InsertText", false, "hello world");
On 2014/03/31 07:40:22, tkent wrote:
> The quotation marks should be ' for consistency with other JavaScript code.

Done.

https://codereview.chromium.org/205033004/diff/80001/LayoutTests/fast/forms/t...
LayoutTests/fast/forms/text/text-window-lost-focus-change-event.html:26:
shouldEvaluateTo('blurEventCounter',   0);
On 2014/03/31 07:40:22, tkent wrote:
> extra space after ,

Done.

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
File Source/core/page/FocusController.cpp (right):

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:148:
toHTMLElement(focusedElement)->setFocus(false);
On 2014/03/31 07:35:37, tkent wrote:
> We don't need to check isHTMLElement, and don't need to apply toHTMLElement. 
> Element has setFocus.

Done.

https://codereview.chromium.org/205033004/diff/80001/Source/core/page/FocusCo...
Source/core/page/FocusController.cpp:150: if (focusedElement ==
document->focusedElement())
On 2014/03/31 07:35:37, tkent wrote:
> We don't need to check it.  setFocus(false) don't change
> Document::focusedElement value.

Done.

[tkent, 2014-03-31 09:34:55]

please use try bots whenever you upload a new patch.

[tkent, 2014-04-01 02:08:30]

lgtm

[commit-bot: I haz the power, 2014-04-01 02:08:31]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/habib.virji@samsung.com/205033004/100001

[commit-bot: I haz the power, 2014-04-01 02:51:58]

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

[commit-bot: I haz the power, 2014-04-01 02:52:01]

Try jobs failed on following builders:
  tryserver.blink on win_blink_rel

[tkent, 2014-04-01 02:59:38]

The CQ bit was checked by tkent@chromium.org

[commit-bot: I haz the power, 2014-04-01 02:59:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/habib.virji@samsung.com/205033004/100001

[commit-bot: I haz the power, 2014-04-01 03:58:24]

Change committed as 170511