[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


Kent Tamura <tkent at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #173568|review?                     |review-
               Flag|                            |




--- Comment #2 from Kent Tamura <tkent at chromium.org>  2012-11-12 00:24:54 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.

> 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.

> Source/WebCore/html/shadow/PickerIndicatorElement.h:53
> +    class PickerIndicatorOwner {
> +    public:
> +        virtual ~PickerIndicatorOwner() { }
> +        virtual void pickerIndicatorChooseValue(const String&) = 0;
> +    };

Introducing PickerIndicatorOwner interface is nice.  However it's not great that it has only one function.
If you'd like to introduce PickerIndicatorOwner, you should remove HTMLInputElement (hostInput()) dependency from PickerIndicatorElement, and PickerIndicatorOwner should replace the role of HTMLInputElement in PickerIndicatorElement.  Such change should be done *before* this bug.

> Source/WebCore/rendering/RenderThemeChromiumCommon.cpp:56
>      return type == InputTypeNames::date()
>          || type == InputTypeNames::month()
> -        || type == InputTypeNames::week();
> +        || type == InputTypeNames::week()
> +        || type == InputTypeNames::datetime()
> +        || type == InputTypeNames::datetimelocal();

nit: please sort in alphabetical order

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