GNOME Bugzilla – Bug 625225
Ctrl+G (find next) shortcut doesn't work when search bar is closed
Last modified: 2013-05-18 08:30:30 UTC
The keyboard shortcuts Ctrl+G (find next) and its reverse Ctrl+Shift+G currently onyl works when the search bar is visible, otherwise it does nothing, even though the search string is retained. Shouldn't this work regardless of search bar visibility? At least that's how it works in most other applications like web browsers, text editors.
Any comments?
If the search bar is not present and there is a text to search, Ctrl+G could show the search bar again and go to the next item.
Created attachment 244304 [details] [review] Patch for making find next/prev shortcuts work when find bar is closed This simple patch makes sure we do not clear or cancel the find job when we close the find bar. And also make sure that find bar going visible do not cancel previous job.
Review of attachment 244304 [details] [review]: I like this patch, thanks! There's only one small problem. 1.- CRTl+F and search something 2.- CRTl+G until you are in the last match of the current page 3.- Close the find bar 4.- CTRL+G It jumps to the right page but the current match is not visible. This happens because the view hasn't been allocated yet after the findbar is made visible again, and ensure_rectangle_is_visible in ev-view.c is using the previous allocation. A possible solution would be to show the find bar as soon as CTRL+G is pressed, but if it was hidden do the ev_view_find_next in an idle that will happen after the allocation because it has lower priority. And of course the same for find previous.
Thanks for review and comments! And nice catch! Using an idle seem to work. I have two patches as suggestions. One checks if the find bar was hidden and then uses an idle and otherwise just calls the ev_view_find_{next|previous} the other always uses an idle and sidesteps the conditional logic. It looks nicer, I think. What do you think? Posting the two different patches as attachments.
Created attachment 244373 [details] [review] This one checks if the find bar was visible
Created attachment 244374 [details] [review] This one always use an idle
Review of attachment 244373 [details] [review]: I agree the other one is cleaner, but there's no reason to wait when the find bar visibility doesn't change. I've just pushed this with a couple of minor modifications. Thank you very much. ::: shell/ev-window.c @@ +3867,3 @@ +find_next_idle_cb (EvView *view) +{ + ev_view_find_next (EV_VIEW (view)); No need to cast here, it's a EvView * @@ +3892,3 @@ +find_previous_idle_cb (EvView *view) +{ + ev_view_find_previous (EV_VIEW (view)); Ditto.