WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
Bug 93069
Add keyboard support for color suggestion popup
https://bugs.webkit.org/show_bug.cgi?id=93069
Summary
Add keyboard support for color suggestion popup
Keishi Hattori
Reported
2012-08-02 23:22:37 PDT
Add keyboard support for color suggestion popup
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
Show Obsolete
(7)
View All
Add attachment
proposed patch, testcase, etc.
Keishi Hattori
Comment 1
2012-08-03 00:01:37 PDT
Created
attachment 156270
[details]
Patch
Kent Tamura
Comment 2
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.
Keishi Hattori
Comment 3
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.
Keishi Hattori
Comment 4
2012-08-03 02:28:24 PDT
I was able to make it work.
Keishi Hattori
Comment 5
2012-08-03 02:36:36 PDT
Created
attachment 156298
[details]
Patch
Keishi Hattori
Comment 6
2012-08-03 02:38:26 PDT
Created
attachment 156299
[details]
Patch
Keishi Hattori
Comment 7
2012-08-03 02:39:12 PDT
Created
attachment 156300
[details]
Patch
Kent Tamura
Comment 8
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?
Kent Tamura
Comment 9
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.
Keishi Hattori
Comment 10
2012-08-03 02:52:58 PDT
Created
attachment 156303
[details]
Patch
Keishi Hattori
Comment 11
2012-08-03 03:17:33 PDT
Created
attachment 156304
[details]
Patch
Keishi Hattori
Comment 12
2012-08-03 03:50:06 PDT
Created
attachment 156314
[details]
Patch
Kent Tamura
Comment 13
2012-08-03 04:03:15 PDT
Comment on
attachment 156314
[details]
Patch Looks ok, but purple EWS.
Keishi Hattori
Comment 14
2012-08-03 04:09:25 PDT
Created
attachment 156320
[details]
Patch
WebKit Review Bot
Comment 15
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
>
WebKit Review Bot
Comment 16
2012-08-03 06:53:58 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug