Bug 93069 - Add keyboard support for color suggestion popup
Summary: Add keyboard support for color suggestion popup
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Forms (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Keishi Hattori
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-08-02 23:22 PDT by Keishi Hattori
Modified: 2012-08-03 06:53 PDT (History)
2 users (show)

See Also:


Attachments
Patch (5.73 KB, patch)
2012-08-03 00:01 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (16.47 KB, patch)
2012-08-03 02:36 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (16.45 KB, patch)
2012-08-03 02:38 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (16.42 KB, patch)
2012-08-03 02:39 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (16.91 KB, patch)
2012-08-03 02:52 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (17.07 KB, patch)
2012-08-03 03:17 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (17.11 KB, patch)
2012-08-03 03:50 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff
Patch (17.24 KB, patch)
2012-08-03 04:09 PDT, Keishi Hattori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Keishi Hattori 2012-08-02 23:22:37 PDT
Add keyboard support for color suggestion popup
Comment 1 Keishi Hattori 2012-08-03 00:01:37 PDT
Created attachment 156270 [details]
Patch
Comment 2 Kent Tamura 2012-08-03 00:22:59 PDT
Comment on attachment 156270 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        No new tests. Can't send key events to page popup.

Really?

> Source/WebCore/Resources/colorSuggestionPicker.js:80
> +//function enclosingNodeOrSelfWithClass(node, className)
> +//{
> +//    for (var node = this; node && node !== this.ownerDocument; node = node.parentNode) {
> +//        if (node.nodeType === Node.ELEMENT_NODE && node.classList.contains(className))
> +//            return node;
> +//    }
> +//    return null;
> +//}

Do not leave unused code.

> Source/WebCore/Resources/colorSuggestionPicker.js:223
> +        switch (key) {
> +            case "Left":

Please use the indentation same as our C++ style.
Comment 3 Keishi Hattori 2012-08-03 00:57:39 PDT
(In reply to comment #2)
> (From update of attachment 156270 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156270&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        No new tests. Can't send key events to page popup.
> 
> Really?
I've tried window.eventSender.keyDown and pickerWindow.eventSender.keyDown and I've tried focusing but it didn't work.
Comment 4 Keishi Hattori 2012-08-03 02:28:24 PDT
I was able to make it work.
Comment 5 Keishi Hattori 2012-08-03 02:36:36 PDT
Created attachment 156298 [details]
Patch
Comment 6 Keishi Hattori 2012-08-03 02:38:26 PDT
Created attachment 156299 [details]
Patch
Comment 7 Keishi Hattori 2012-08-03 02:39:12 PDT
Created attachment 156300 [details]
Patch
Comment 8 Kent Tamura 2012-08-03 02:43:47 PDT
Comment on attachment 156300 [details]
Patch

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

> Source/WebCore/Resources/colorSuggestionPicker.js:204
> +    if (key != "Left" && key != "Up" && key != "Right" && key != "Down")
> +        return;

How to choose the focused color with the keyboard?
Comment 9 Kent Tamura 2012-08-03 02:47:33 PDT
(In reply to comment #8)
> (From update of attachment 156300 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=156300&action=review
> 
> > Source/WebCore/Resources/colorSuggestionPicker.js:204
> > +    if (key != "Left" && key != "Up" && key != "Right" && key != "Down")
> > +        return;
> 
> How to choose the focused color with the keyboard?

Alos, we should provide a key bind to close the popup.
Comment 10 Keishi Hattori 2012-08-03 02:52:58 PDT
Created attachment 156303 [details]
Patch
Comment 11 Keishi Hattori 2012-08-03 03:17:33 PDT
Created attachment 156304 [details]
Patch
Comment 12 Keishi Hattori 2012-08-03 03:50:06 PDT
Created attachment 156314 [details]
Patch
Comment 13 Kent Tamura 2012-08-03 04:03:15 PDT
Comment on attachment 156314 [details]
Patch

Looks ok, but purple EWS.
Comment 14 Keishi Hattori 2012-08-03 04:09:25 PDT
Created attachment 156320 [details]
Patch
Comment 15 WebKit Review Bot 2012-08-03 06:53:54 PDT
Comment on attachment 156320 [details]
Patch

Clearing flags on attachment: 156320

Committed r124605: <http://trac.webkit.org/changeset/124605>
Comment 16 WebKit Review Bot 2012-08-03 06:53:58 PDT
All reviewed patches have been landed.  Closing bug.