[webkit-reviews] review denied: [Bug 96085] Datalist UI for input type=date for Chromium port : [Attachment 164709] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Sep 19 07:26:45 PDT 2012
Kent Tamura <tkent at chromium.org> has denied Keishi Hattori
<keishi at webkit.org>'s request for review:
Bug 96085: Datalist UI for input type=date for Chromium port
https://bugs.webkit.org/show_bug.cgi?id=96085
Attachment 164709: Patch
https://bugs.webkit.org/attachment.cgi?id=164709&action=review
------- Additional Comments from Kent Tamura <tkent at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=164709&action=review
I agree with the basic structure of this patch. However it's too large to
reivew. Let's split into multiple pieces.
For example,
1. Preparation in calendarPicker.{css.js}
Adding .calendar-picker, rename validateArguments to
CalendarPicker.validateConfig, adding CalendarPicker.cleanup(), etc.
2. Adding DateTimeChooserParameters::localizedSuggestionValues, and <datalist>
support in CalendarPickerElement::openPopup().
3. Update DateTimeChooserImpl.cpp and WebLocalizedString.h
4. suggestionPicker.{css.js} and remaining JavaScript changes
5. Enable <datalist> support for date type in RenderThemeChromiumCommon.cpp
without hilightFirstSuggestionEntry feature , and adding tests.
6. Adding hilightFirstSuggestionEntry feature.
> Source/WebCore/html/DateInputType.cpp:-147
> - if (m_pickerElement)
> - m_pickerElement->closePopup();
> -
Don't change the existing behavior for a test crash.
You should fix the testing framework, or probably you can avoid the crash by
opening the picker without focus. See
fast/forms/date/calendar-picker-appearance.html.
More information about the webkit-reviews
mailing list