GNOME Bugzilla – Bug 676040
Rotate keyboard shortcuts interfere with the search field
Last modified: 2013-05-28 18:32:54 UTC
Usually, when the cursor is in a text field, and the user press CTRL+LEFT ARROW (or CTRL+RIGHT ARROW) it expects the cursor to jump to the beginning of the previous (respectively, end of the next) word. This fails on the text field of the search bar: 1) Open the bar by typing "CTRL+F" 2) Type two words: "foo bar" (the cursor will now be after "bar", as in "foo bar|") 3) Press CTRL+LEFT ARROW Result: * The page will be rotated conter clockwise Expected behavior: * The cursor should jump to te begin of "bar" (i.e. "foo |bar") while the page should stay unrotated I'm using evince 3.4.0 on Ubuntu 12.04.
No need to use the control key combinations. Try typing any search word that includes lower or upper case letter 'v'. Curious how this application made it this far without the developers themselves hitting this basic problem.
(In reply to comment #1) > No need to use the control key combinations. Try typing any search word that > includes lower or upper case letter 'v'. Curious how this application made it > this far without the developers themselves hitting this basic problem. This could be a different issue, perhaps a local issue (maybe a key bind added by mistake). Here I search for words with 'v' either lowercase and uppercase and it works as expected. Neither with 3.4.0 nor evince master.
A possible solution for this bug could be to disable the acceleration keys for rotate when the search entry has the focus.
Created attachment 245423 [details] [review] [PATCH] shell: forward accels to the focused widget GtkWindow catches keybindings for the menu items _before_ passing them to the focused widget. This is unfortunate and means that pressing ctrl+V in an entry on a panel ends up pasting text in the TextView. Here we override GtkWindow's handler to do the same things that it does, but in the opposite order and then we chain up to the grand parent handler, skipping gtk_window_key_press_event. See https://bugzilla.gnome.org/show_bug.cgi?id=676040 --- shell/ev-window.c | 46 ++++++++++++++++++++++++++++++++-------------- 1 file changed, 32 insertions(+), 14 deletions(-)
Created attachment 245424 [details] [review] [PATCH] shell: Remove ev_window_set_view_accels_sensitivity Now that we chain the accels to the focused widget, we do not need anymore to disable accels when the view is not focused. --- shell/ev-window.c | 44 -------------------------------------------- 1 file changed, 44 deletions(-)
Carlos, what do you think of this approach? I copy the solution in Gedit, where the key bindings are forwarded to the focused widget. That way, we don't need to disable the accels anymore since if they are defined by another widget, they will be catched up by the other widgets first. These patches depend on the patch for the bug 668446. With this, in the second patch we can just remove ev_window_set_view_accels_sensitivity
Review of attachment 245423 [details] [review]: Thanks, I have some comments though. ::: shell/ev-window.c @@ +5748,3 @@ + * GtkWindow catches keybindings for the menu items _before_ passing them to + * the focused widget. This is unfortunate and means that pressing ctrl+V + * in an entry on a panel ends up pasting text in the TextView. What TextView? @@ +5764,3 @@ + { + grand_parent_class = g_type_class_peek_parent (ev_window_parent_class); + } Please fix coding style. Do not use braces for if with a single statement. @@ +5782,3 @@ + { + handled = GTK_WIDGET_CLASS (grand_parent_class)->key_press_event (widget, event); + } Instead of checking handled 3 times, wouldn't it be better to do something like: if (gtk_window_propagate_key_event (window, event)) return TRUE; if (gtk_window_activate_key (window, event)) return TRUE; return GTK_WIDGET_CLASS (grand_parent_class)->key_press_event (widget, event);
Review of attachment 245424 [details] [review]: I love patches with only removed lines, thanks! Feel free to push once the other one get reviewed and landed.
Created attachment 245474 [details] [review] [PATCH] shell: Remove ev_window_set_view_accels_sensitivity Now that we chain the accels to the focused widget, we do not need anymore to disable accels when the view is not focused. --- shell/ev-window.c | 44 -------------------------------------------- 1 file changed, 44 deletions(-)
Created attachment 245475 [details] [review] [PATCH] shell: forward accels to the focused widget GtkWindow catches keybindings for the menu items _before_ passing them to the focused widget. This is unfortunate and means that pressing ctrl+Left arrow, Ctrl+Right arrow on the search bar ends up turning the EvView instead of moving around the text. Here we override GtkWindow's handler to do the same things that it does, but in the opposite order and then we chain up to the grand parent handler, skipping gtk_window_key_press_event. See https://bugzilla.gnome.org/show_bug.cgi?id=676040 --- shell/ev-window.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-)
Created attachment 245476 [details] [review] [PATCH] shell: remove view_actions_focus_out_cb It is now doing nothing --- shell/ev-window.c | 12 ------------ 1 file changed, 12 deletions(-)
Review of attachment 245474 [details] [review]: same patch again, sorry
So, updated patch according to review. Added another patch that further removes some lines.
Comment on attachment 245475 [details] [review] [PATCH] shell: forward accels to the focused widget Awesome, removed trailing whitespaces and fixed some minor details and pushed to git master. Thanks
Comment on attachment 245476 [details] [review] [PATCH] shell: remove view_actions_focus_out_cb Ok
This problem has been fixed in the development version. The fix will be available in the next major software release. Thank you for your bug report.