[Webkit-unassigned] [Bug 101889] Enable calendar picker for input types datetime/datetime-local

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=101889





--- Comment #7 from Kunihiko Sakamoto <ksakamoto at chromium.org>  2012-11-13 21:03:44 PST ---
(From update of attachment 173568)
View in context: https://bugs.webkit.org/attachment.cgi?id=173568&action=review

>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:487
>> +void DateTimeEditElement::setValueFromCalendarPicker(const DateComponents& date)
> 
> nit: It's not a good name because it doesn't represent what the function does.  My proposal is setOnlyYearMonthDay.

Renamed it to setOnlyYearMonthDay.

>>>> Source/WebCore/html/shadow/DateTimeEditElement.cpp:498
>>>> +    setValueAsDateTimeFieldsState(dateTimeFieldsState);
>>> 
>>> Can you avoid to change the setValueAsDateTimeFieldsState signature?
>>> I think passing a DateComponents for the old value as dateForReadOnlyField argument works well though I'm not 100% sure.
>> 
>> Hmm, It won't work if there is a read-only field and fields has empty values.
> 
> The logic same as BaseMultipleFieldsDateAndTimeInputType::updateInnerTextValue should work.
> i.e. Use the previous value for dateForReadOnlyField if it's not empty, otherwise use StepRange::minimum().

Done.
Thanks to your refactoring, now setValueAsDateTimeFieldsState does not require dateForReadOnlyField.

>> Source/WebCore/rendering/RenderThemeChromiumCommon.cpp:56
>> +        || type == InputTypeNames::datetimelocal();
> 
> nit: please sort in alphabetical order

Done.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.