Update of change event for datetime input type

Update of change event for datetime input type

BaseMultipleFieldDateAndTime includes changes for sending change event when popup is closed, keyboard event and mouse event.

BaseChoosedOnlyDateAndTime includes changes for sending change event when popup is closed. Special case for input type datetime-local.

R=tkent
BUG=155747
TEST=Fixes currently broken test in bug331189

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

[tkent, 2014-01-14 01:04:50]

Please upload the CL again.  The Patch Set 1 doesn't show side-by-side diffs.

Please add tests.

I don't think the patch is correct. The problem is that 'change' events are too
much, but the patch adds 'change' event dispatching.

[Habib Virji, 2014-01-15 15:10:54]

On 2014/01/14 01:04:50, tkent wrote:
> Please upload the CL again.  The Patch Set 1 doesn't show side-by-side diffs.
> 
> Please add tests.
> 
> I don't think the patch is correct. The problem is that 'change' events are
too
> much, but the patch adds 'change' event dispatching.

Added tests to check input and change event for all spinner inherited types.

CL only alters event dispatchFormControlChangeEvent previously triggered via
editControlValueChange, to more respective event types (picker and keyboard).
This CL relies on 
128153002, for spin button mouse event handling.

[Habib Virji, 2014-01-16 17:07:25]

On 2014/01/15 15:10:54, habib.virji wrote:
> On 2014/01/14 01:04:50, tkent wrote:
> > Please upload the CL again.  The Patch Set 1 doesn't show side-by-side
diffs.
> > 
> > Please add tests.
> > 
> > I don't think the patch is correct. The problem is that 'change' events are
> too
> > much, but the patch adds 'change' event dispatching.
> 
> Added tests to check input and change event for all spinner inherited types.
> 
> CL only alters event dispatchFormControlChangeEvent previously triggered via
> editControlValueChange, to more respective event types (picker and keyboard).
> This CL relies on 
> 128153002, for spin button mouse event handling.

Added a new patch with only change in forms.

[Habib Virji, 2014-01-18 08:24:20]

@tkent can you please review the changes

[tkent, 2014-01-20 02:05:21]

lgtm

https://codereview.chromium.org/128153002/diff/130001/Source/core/html/forms/...
File Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp (right):

https://codereview.chromium.org/128153002/diff/130001/Source/core/html/forms/...
Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp:433:
element().dispatchFormControlChangeEvent();
We shouldn't put dispatchFormControlChangeEvent here because it looks unclear
why we need it. We should move it elsewhere.

[commit-bot: I haz the power, 2014-01-20 02:05:27]

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

[tkent, 2014-01-20 02:05:31]

oops, not lgtm

[Habib Virji, 2014-01-20 10:14:58]

Uploaded new patch. 

Keyboard event for date time are handled in forwardEvent when of type
DateTimeEditElement.

All keyboard events for DataTime are handled in DateTimeFieldElement, since we
do not want to include HTMLInputElement in shadow class, have handled in forward
event.

[tkent, 2014-01-20 13:36:23]

On 2014/01/20 10:14:58, habib.virji wrote:
> Keyboard event for date time are handled in forwardEvent when of type
> DateTimeEditElement.
> 
> All keyboard events for DataTime are handled in DateTimeFieldElement, since we
> do not want to include HTMLInputElement in shadow class, have handled in
forward
> event.

I know it, and I know your current patch will work well.  However the code is
not good.

BaseMultipleFieldsDateAndTimeInputType forwards events to the
DateTimeEditElement and its sub-fields.  So,
BaseMultipleFieldsDateAndTimeInputType should not know what they do for the
events, and should not call dispatchFormControlChangeEvent().
dispatchFormControlChangeEvent() should be called by who changes the input
value, and you may add new callback to DateTimeEditElement::EditControlOwner or
DateTimeFieldElement::FieldOwner, or something.  It's like what you did for
SpinButtonElement::SpinButtonOwner.

[Habib Virji, 2014-01-20 15:22:12]

Thanks for review comment. Uploaded new patch with the suggested changes.

[tkent, 2014-01-21 01:00:53]

https://codereview.chromium.org/128153002/diff/300001/Source/core/html/shadow...
File Source/core/html/shadow/DateTimeFieldElement.cpp (right):

https://codereview.chromium.org/128153002/diff/300001/Source/core/html/shadow...
Source/core/html/shadow/DateTimeFieldElement.cpp:66:
m_fieldOwner->fieldOwnerDidReleaseMouseCapture();
The function name is wrong.  This code is not related to mouse capture. The
function name should be "fieldDidChangeValueByKeyboard" or something.

Also, we don't change the input value yet at this line. We should call the
callback after handleKeyboardEvent and after defaultKeyboardEventHandler.

[Habib Virji, 2014-01-21 09:16:32]

Uploaded new patch as per review comments.

[tkent, 2014-01-21 09:19:01]

https://codereview.chromium.org/128153002/diff/340001/Source/core/html/shadow...
File Source/core/html/shadow/DateTimeFieldElement.cpp (right):

https://codereview.chromium.org/128153002/diff/340001/Source/core/html/shadow...
Source/core/html/shadow/DateTimeFieldElement.cpp:68: return;
m_fieldOwner->fieldDidChangeValueByKeyboard() should be called before the
"return" too.

[Habib Virji, 2014-01-21 09:26:57]

Updated new patch as per review comments.

https://codereview.chromium.org/128153002/diff/340001/Source/core/html/shadow...
File Source/core/html/shadow/DateTimeFieldElement.cpp (right):

https://codereview.chromium.org/128153002/diff/340001/Source/core/html/shadow...
Source/core/html/shadow/DateTimeFieldElement.cpp:68: return;
On 2014/01/21 09:19:01, tkent wrote:
> m_fieldOwner->fieldDidChangeValueByKeyboard() should be called before the
> "return" too.
> 

Done.

[tkent, 2014-01-21 10:37:40]

lgtm

[commit-bot: I haz the power, 2014-01-21 10:38:02]

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

[commit-bot: I haz the power, 2014-01-21 11:43:13]

Change committed as 165455

[tkent, 2014-02-12 07:35:29]

A revert of this CL has been created in
https://codereview.chromium.org/159893007/ by tkent@chromium.org.

The reason for reverting is: INPUT element dispatches extra events.
.