Bug 98754 – F4 key should open the picker popup on Windows

• Status: RESOLVED
• Resolution: FIXED
• Component: Forms

[Keishi Hattori]

F4 key should open the picker popup on Windows

[Keishi Hattori]

Created attachment 167751 [details]
Patch

[WebKit Review Bot]

Comment on attachment 167751 [details]
Patch

Attachment 167751 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/14214770

New failing tests:
fast/forms/date/calendar-picker-key-operations.html
platform/chromium/fast/forms/date/date-suggestion-picker-key-operations.html
platform/chromium/fast/forms/time/time-suggestion-picker-key-operations.html

[Keishi Hattori]

Created attachment 167894 [details]
removed extra {

[Kent Tamura]

Comment on attachment 167894 [details]
removed extra {

View in context: https://bugs.webkit.org/attachment.cgi?id=167894&action=review

> Source/WebCore/html/BaseMultipleFieldsDateAndTimeInputType.cpp:197
> +    } else if (m_pickerIndicatorIsVisible && event->keyIdentifier() == "Down" && event->getModifierState("Alt")) {
>          m_pickerIndicatorElement->openPopup();

Should we check nullness of m_pickerIndicatorElement here too?

> Source/WebCore/rendering/RenderThemeChromiumWin.cpp:814
> +    return true;

We apply the same key bindings to Chromium-Windows and Chromium-Linux.  Please add this function to RenderThemeChromiumLinux too.

[Keishi Hattori]

Created attachment 167922 [details]
Patch

[Kent Tamura]

Comment on attachment 167922 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=167922&action=review

> Source/WebCore/ChangeLog:27
> +        * html/BaseMultipleFieldsDateAndTimeInputType.cpp:
> +        (WebCore::BaseMultipleFieldsDateAndTimeInputType::handleKeydownEvent):
> +        * rendering/RenderTheme.cpp:
> +        (WebCore::RenderTheme::shouldOpenPickerWithF4Key):
> +        (WebCore):
> +        * rendering/RenderTheme.h:
> +        (RenderTheme):
> +        * rendering/RenderThemeChromiumLinux.cpp:
> +        (WebCore::RenderThemeChromiumLinux::shouldOpenPickerWithF4Key): Returns true if we want to enable the F4 key binding on this platform.
> +        (WebCore):
> +        * rendering/RenderThemeChromiumLinux.h:
> +        * rendering/RenderThemeChromiumWin.cpp:
> +        (WebCore):
> +        (WebCore::RenderThemeChromiumWin::shouldOpenPickerWithF4Key):
> +        * rendering/RenderThemeChromiumWin.h:
> +        (RenderThemeChromiumWin):

Please write per-file / per-function comments as possible

[Keishi Hattori]

Created attachment 168203 [details]
Patch

[WebKit Review Bot]

Comment on attachment 168203 [details]
Patch

Clearing flags on attachment: 168203

Committed r131054: <http://trac.webkit.org/changeset/131054>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.