[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