RESOLVED FIXED 18363
[GTK] Combo boxes cannot be opened pressing space
https://bugs.webkit.org/show_bug.cgi?id=18363
Summary [GTK] Combo boxes cannot be opened pressing space
Marco Barisione
Reported 2008-04-08 06:57:53 PDT
In GTK combo boxes can be opened by pressing space when they are focused. I tried to implement this behaviour in WebKit but I got lost while debugging keyboard events handling :(
Attachments
Take away space out of ARROW_KEYS_POP_MENU and into SPACE_POP_MENU for platforms that display the popup when spacebar is pressed (1.92 KB, patch)
2009-07-10 15:52 PDT, Jan Alonzo
no flags
addressed eric's feedback and added a manual test (2.55 KB, patch)
2009-07-13 01:28 PDT, Jan Alonzo
no flags
updated: return key should popup the menu list too for gtk (3.13 KB, patch)
2009-07-17 15:52 PDT, Jan Alonzo
no flags
Make it if-elif-else-endif as suggested by Holger (3.28 KB, patch)
2009-07-17 19:25 PDT, Jan Alonzo
zecke: review+
Jan Alonzo
Comment 1 2009-06-30 04:10:06 PDT
*** Bug 26818 has been marked as a duplicate of this bug. ***
Jan Alonzo
Comment 2 2009-07-10 15:52:39 PDT
Created attachment 32586 [details] Take away space out of ARROW_KEYS_POP_MENU and into SPACE_POP_MENU for platforms that display the popup when spacebar is pressed The patch takes away space (' ') out of ARROW_KEYS_POP_MENU and into SPACE_POP_MENU for platforms that want to display a popup menu when spacebar is pressed. No change in behaviour except for Gtk.
Eric Seidel (no email)
Comment 3 2009-07-10 23:08:01 PDT
Comment on attachment 32586 [details] Take away space out of ARROW_KEYS_POP_MENU and into SPACE_POP_MENU for platforms that display the popup when spacebar is pressed This looks like it will break mac. You seem to want #if and #endif then another #if, not #elif This also needs a manual test.
Jan Alonzo
Comment 4 2009-07-13 01:28:27 PDT
Created attachment 32652 [details] addressed eric's feedback and added a manual test
Gustavo Noronha (kov)
Comment 5 2009-07-14 14:34:28 PDT
Comment on attachment 32652 [details] addressed eric's feedback and added a manual test Looks good to me, after the changes. While you're at it, you could also have enter also open the combo box, to match what GTK+ does. Should be a matter of adding || keyCode == 'r' to the if inside SPACE_POP_MENU. Make it SPACE_AND_RETURN_POP_MENU =P.
Adam Barth
Comment 6 2009-07-17 00:22:42 PDT
Will land.
Adam Barth
Comment 7 2009-07-17 00:28:47 PDT
On second thought, I probably should go to sleep...
Jan Alonzo
Comment 8 2009-07-17 15:52:16 PDT
Created attachment 32984 [details] updated: return key should popup the menu list too for gtk
Holger Freyther
Comment 9 2009-07-17 18:17:06 PDT
(In reply to comment #8) > Created an attachment (id=32984) [details] > updated: return key should popup the menu list too for gtk Now a ' ' will show the popup twice?
Holger Freyther
Comment 10 2009-07-17 18:33:13 PDT
(In reply to comment #9) > (In reply to comment #8) > > Created an attachment (id=32984) [details] [details] > > updated: return key should popup the menu list too for gtk > > Now a ' ' will show the popup twice? To be more precise this can happen when both ARROW_KEYS_POP_MENU and SPACE_OR_RETURN_POP_MENU are set. It should be mutual exclusive given that the current code is #if #else #endif and we turn it into #if #endif, #if #else #endif.
Jan Alonzo
Comment 11 2009-07-17 19:25:32 PDT
Created attachment 33001 [details] Make it if-elif-else-endif as suggested by Holger
Holger Freyther
Comment 12 2009-07-17 19:41:55 PDT
Comment on attachment 33001 [details] Make it if-elif-else-endif as suggested by Holger OKay, the next thing boils down to taste. To reduce code duplication one could do if ( #if SPACE_OR_.. keyCode == ' ' || keyCode == '\r' #elif keyCode == ' ' #endif ) { common Code }
Jan Alonzo
Comment 13 2009-07-17 20:29:34 PDT
(In reply to comment #12) > (From update of attachment 33001 [details]) Landed as http://trac.webkit.org/changeset/46081 > OKay, the next thing boils down to taste. To reduce code duplication one could > do > > if ( > #if SPACE_OR_.. > keyCode == ' ' || keyCode == '\r' > #elif > keyCode == ' ' > #endif > ) { > common Code > } I'll do this in a separate bug.
Note You need to log in before you can comment on or make changes to this bug.