GNOME Bugzilla – Bug 771165
Pressing ESC key doesn’t close search box
Last modified: 2017-12-17 21:01:58 UTC
Using GNOME-Terminal 3.21.90 from Debian Sid/unstable, opening the search box with Ctrl + f, and pressing the escape key (ESC) it doesn’t close. This is the expected behavior for a user.
Duplicate of bug 771165 ? (This one is better, since it's focused on a single issue)
Created attachment 365650 [details] [review] Fix Something like this? Interestingly, on Unity 7, upon pressing ESC, the window's title looks weird for a fraction of a second. The two buttons become gray over white/transparent? background, and the title "Search" is shown in outlined. Something I haven't seen before, doesn't happen when I close by clicking on X, Alt+F4, close the terminal by Ctrl+D or Ctrl+Shift+W etc. Probably absolutely harmless and nothing we could do about, and Unity is dying anyways. On a side note, at some point (maybe with the GMenu port?) the Search menu got renamed to Find, yet this popup still has the title Search, should it be renamed too?
+ gtk_widget_hide (popover); I'd rather gtk_widget_destroy() it. + g_signal_connect (popover, "key_press_event", G_CALLBACK (key_press_cb), NULL); Nit: dashes, not underscores, in the signal name is the canonical name. With these fixed, ok to commit. Thanks! (In reply to Egmont Koblinger from comment #2) > On a side note, at some point (maybe with the GMenu port?) the Search menu > got renamed to Find, yet this popup still has the title Search, should it be > renamed too? It got renamed because of bug 757545 comment 2, so probably yes, it should also be renamed in the popover.
(In reply to Christian Persch from comment #3) > + gtk_widget_hide (popover); > > I'd rather gtk_widget_destroy() it. I copied from close_clicked_cb() just above this method. Change both or neither? :) > Nit: dashes, not underscores, in the signal name is the canonical name. This is what you get when you're coding from stackoverflow rather than the docs :-D
(In reply to Egmont Koblinger from comment #4) > (In reply to Christian Persch from comment #3) > > + gtk_widget_hide (popover); > > > > I'd rather gtk_widget_destroy() it. > > I copied from close_clicked_cb() just above this method. Change both or > neither? :) Hmm, true. I'd prefer destroy for both, if that works.
> Hmm, true. I'd prefer destroy for both, if that works. > Interestingly, on Unity 7, upon pressing ESC, the window's title looks weird > for a fraction of a second. The two buttons become gray [...] These two are actually related, the weird look only happens on hide, not destroy. On the other hand, a hide+open preserves the text you're searching for and the state of the checkboxes, while a destroy+open resets them. This is pretty bad usability. g-t would explicitly need to save and re-initialize these fields then. How about the following: I go for hide now, and let you do these if/whenever you feel like?
Created attachment 365662 [details] [review] Rename "Search" to "Find"
(In reply to Egmont Koblinger from comment #6) > How about the following: I go for hide now, and let you do these if/whenever > you feel like? OK.
Continuing in bug 791712, closing this one.