Prepare for date/time input datalist support

Prepare for date/time input datalist support

BUG=

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

[keishi, 2013-11-06 12:54:58]

Chromium side is here: https://codereview.chromium.org/23623019/

[tkent, 2013-11-07 01:43:09]

https://codereview.chromium.org/23623017/diff/56001/Source/core/html/forms/Ba...
File Source/core/html/forms/BaseChooserOnlyDateAndTimeInputType.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/core/html/forms/Ba...
Source/core/html/forms/BaseChooserOnlyDateAndTimeInputType.cpp:103:
element().setValue(String(), DispatchInputAndChangeEvent);
Are you sure this should be null string, not emptyString()?

https://codereview.chromium.org/23623017/diff/56001/Source/core/html/forms/Ba...
File Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/core/html/forms/Ba...
Source/core/html/forms/BaseMultipleFieldsDateAndTimeInputType.cpp:268:
element().setValue(String(), DispatchInputAndChangeEvent);
Ditto.

https://codereview.chromium.org/23623017/diff/56001/Source/core/rendering/Ren...
File Source/core/rendering/RenderThemeChromiumAndroid.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/core/rendering/Ren...
Source/core/rendering/RenderThemeChromiumAndroid.cpp:109: return type ==
InputTypeNames::date() || type == InputTypeNames::week() || type ==
InputTypeNames::month() || type == InputTypeNames::datetimelocal() || type ==
InputTypeNames::time();
This change should be landed after Chromium changes are landed.

https://codereview.chromium.org/23623017/diff/56001/Source/platform/DateTimeC...
File Source/platform/DateTimeChooserClient.h (right):

https://codereview.chromium.org/23623017/diff/56001/Source/platform/DateTimeC...
Source/platform/DateTimeChooserClient.h:46: virtual void didChooseValue(double)
= 0;
Do you keep didChooseValue(String), or remove it later?

https://codereview.chromium.org/23623017/diff/56001/Source/web/DateTimeChoose...
File Source/web/DateTimeChooserImpl.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/web/DateTimeChoose...
Source/web/DateTimeChooserImpl.cpp:152: for (unsigned i = 0; i <
m_parameters.suggestions.size(); i++) {
|i| should be size_t because Vector::size is size_t.
We prefer ++i.

https://codereview.chromium.org/23623017/diff/56001/Source/web/WebDateTimeSug...
File Source/web/WebDateTimeSuggestion.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/web/WebDateTimeSug...
Source/web/WebDateTimeSuggestion.cpp:31: namespace WebKit {
should be |namespace blink {|

https://codereview.chromium.org/23623017/diff/56001/Source/web/WebDateTimeSug...
Source/web/WebDateTimeSuggestion.cpp:48: } // namespace WebKit
Ditto.

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeCho...
File public/web/WebDateTimeChooserCompletion.h (right):

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeCho...
public/web/WebDateTimeChooserCompletion.h:39: virtual void
didChooseValue(double) = 0;
You can't do this because it broke Chromium compilation.
You should *add* an overload with double argument.

Please comment what NaN value means.

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeCho...
File public/web/WebDateTimeChooserParams.h (right):

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeCho...
public/web/WebDateTimeChooserParams.h:45: double currentValue;
You can't change the type now because it breaks Chromium compilation.
You should *add* |double doubleValue| or something.

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeSug...
File public/web/WebDateTimeSuggestion.h (right):

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeSug...
public/web/WebDateTimeSuggestion.h:35: namespace WebKit {
WebKit -> blink

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeSug...
public/web/WebDateTimeSuggestion.h:38: double value;
Please add a comment about what this double value is.

https://codereview.chromium.org/23623017/diff/56001/public/web/WebDateTimeSug...
public/web/WebDateTimeSuggestion.h:50: } // namespace WebKit
WebKit -> blink

[keishi, 2013-11-26 11:53:50]

Removed the doubleValue part.

https://codereview.chromium.org/23623017/diff/56001/Source/core/rendering/Ren...
File Source/core/rendering/RenderThemeChromiumAndroid.cpp (right):

https://codereview.chromium.org/23623017/diff/56001/Source/core/rendering/Ren...
Source/core/rendering/RenderThemeChromiumAndroid.cpp:109: return type ==
InputTypeNames::date() || type == InputTypeNames::week() || type ==
InputTypeNames::month() || type == InputTypeNames::datetimelocal() || type ==
InputTypeNames::time();
On 2013/11/07 01:43:10, tkent wrote:
> This change should be landed after Chromium changes are landed.

Done.

[tkent, 2013-11-26 13:34:35]

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

https://codereview.chromium.org/23623017/diff/196001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:1849: suggestion.value =
m_inputType->parseToNumber(option->value(), -1).toDouble();
Why -1?
If option->value() is invalid, shouldn't we append the DateTimeSuggestion?

[keishi, 2013-11-27 07:30:57]

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

https://codereview.chromium.org/23623017/diff/196001/Source/core/html/HTMLInp...
Source/core/html/HTMLInputElement.cpp:1849: suggestion.value =
m_inputType->parseToNumber(option->value(), -1).toDouble();
On 2013/11/26 13:34:35, tkent wrote:
> Why -1?
> If option->value() is invalid, shouldn't we append the DateTimeSuggestion?

Done.

[tkent, 2013-11-27 07:38:29]

lgtm

[commit-bot: I haz the power, 2013-11-27 07:40:27]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/23623017/216001

[commit-bot: I haz the power, 2013-11-27 09:04:47]

Retried try job too often on linux_blink for step(s) webkit_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_blin...

[commit-bot: I haz the power, 2013-11-27 09:27:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/keishi@chromium.org/23623017/236001

[commit-bot: I haz the power, 2013-11-27 11:05:54]

Change committed as 162754