Created attachment 298123 [details] [review]
proposed fix

Fix patch mimetype

Review of attachment 298123 [details] [review]:

If you take a look at the code in nautilus_window_slot_set_search_visible you will see that it already takes into account if you call it being hidden and marking hidden again.
Actually, window-slot itself use it as this in nautilus_window_slot_sync_search_widgets.

It makes sense to not rely in if it's shown or not, you just want it to be hidden. A similar behavior happens with gtk_widget_hide ().

So not need to add new function to query the state, just call the hide function.

But I would actually investigate why is this not working: https://git.gnome.org/browse/nautilus/tree/src/nautilus-query-editor.c#n241

Created attachment 298141 [details] [review]
proposed patch v2

This patch tries to to solve it the "right" way. Actually, the GtkBindingSet code was not working at all. The commit message was updated accordingly.

Review of attachment 298141 [details] [review]:

The thing is that we use it in the location editor and it works there. So I would like to investigate further why is not working here.

The code of this solution would be good though, but better know why the binding is not working.

If I may make a recommendation here, GtkSearchEntry in 3.15 gained a stop-search signal that is bound to Escape (as well as previous-search and next-search signals). Might be nice to use those.

Created attachment 298191 [details] [review]
use GtkSearchBar to handle input

This patch takes a completely different approach and uses GtkSearchBar to handle search input events.

Review of attachment 298191 [details] [review]:

This seems to be intended in two patches.
Is not GtkSearchBar which fixes the problem, is using the already there GtkEntry, connecting to its stop-searching signal.

The other patch would be use GtkSearchBar instead of what we have now.
This patch with your implementation currently has some problems:
- The width of the query is not filling the box. Either modify the child properties of the GtkSearchBar so the boxes inside it doesn't try to expand, or don't use a GtkSearchBar at all.
- Esc is not actually working. It's showing again the query. HINT: GDK_EVENT_STOP == TRUE, nautilus_query_editor_handle_event maybe was doing the opposite before =). Yeah I know, kudos to the inconsistency here... =)
- Code desing wise, it's strange that we use some signals from the nautilus-query and then some signals from the entry inside that nautilus-query. Either use one or the other. For this specific case...take a look at the setup_widgets function in nautilus-query, is actually connecting to the signals of the entry to emit it's own signals.

Review of attachment 298191 [details] [review]:

Forgot to say:

::: src/nautilus-window-slot.c
@@ +428,3 @@
 	if (!NAUTILUS_IS_DESKTOP_WINDOW (window)) {
+		retval = gtk_search_bar_handle_event (GTK_SEARCH_BAR (slot->details->query_editor_search_bar),
+				(GdkEvent*) event);

this makes nautilus_query_editor_handle_event unused.
Also keep an eye in the code aligment, nautilus uses tabs with 8 size.

Created attachment 298283 [details] [review]
use GtkSearchEntry signal

This implements Matthias' idea about reusing the GtkSearchEntry::'stop-search' signal.

Created attachment 298286 [details] [review]
query-editor: cancel search with entry's signal

By listening to the GtkSearchEntry::'stop-search' signal
and sending NautilusQueryEditor::'cancel' right after,
the Esc problem is solved.

Review of attachment 298283 [details] [review]:

Explain what was the problem. Esc wasn't working.

Then explain your suspicious: the signal was not triggered and then nautilus-window-slot was not catching the signal to hide the search bar.

Then the solution: Given that GtkSearchEntry now have a "cancel-search" signal we can use that to trigger the nautilus-query "cancel" signal, which
fixes the problem

::: src/nautilus-query-editor.c
@@ +266,3 @@
 
+static void
+stop_search_cb (GtkWidget *entry, NautilusQueryEditor *editor)

nautilus_query_editor_on_stop_search ? I know we have a mix here, but I think is better to always put the prefix (nautilus_query_editor_) and then use on instead of callback (some people used cb for shorteness, other people prefer callback, etc. So "on" is as short as "cb" but it doesn't have a longer version that people can use)

@@ +940,3 @@
 			  G_CALLBACK (entry_changed_cb), editor);
+	g_signal_connect (editor->details->entry, "stop-search",
+			  G_CALLBACK (stop_search_cb), editor);

can you use spaces for align?

Created attachment 298297 [details] [review]
query-editor: cancel search with entry's signal

After the introduction of GtkSearchEntry by commit
5521f24f, the Esc key stopped working, specifically
after the introduction of the GtkSearchEntry::'stop-search'
signal.

The old approach of adding a GtkBindingSet directly to the
NautilusQueryEditor class then stopped working, as the Esc
key won't be propagated by the search entry. Without the key
catch, the NautlisQueryEditor::'cancel' signal couldn't be
fired, which causes the issue described in this bug.

By listening to the GtkSearchEntry::'stop-search' signal
and sending NautilusQueryEditor::'cancel' right after,
the Esc problem is solved.

Created attachment 298300 [details] [review]
query-editor: cancel search with entry's signal

After the introduction of GtkSearchEntry by commit
5521f24f, the Esc key stopped working, specifically
after the introduction of the GtkSearchEntry::'stop-search'
signal.

The old approach of adding a GtkBindingSet directly to the
NautilusQueryEditor class then stopped working, as the Esc
key won't be propagated by the search entry. Without the key
catch, the NautlisQueryEditor::'cancel' signal couldn't be
fired, which causes the Esc issue.

By listening to the GtkSearchEntry::'stop-search' signal
and sending NautilusQueryEditor::'cancel' right after,
the Esc key is correctly detected and the problem is
solved.

Review of attachment 298300 [details] [review]:

Yep! Thanks for the work!

Attachment 298300 [details] pushed as dc2dffd - query-editor: cancel search with entry's signal