After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756183 - window-slot: fix search state when going back/forward
window-slot: fix search state when going back/forward
Status: RESOLVED FIXED
Product: nautilus
Classification: Core
Component: File Search Interface
unspecified
Other All
: Normal normal
: ---
Assigned To: Nautilus Maintainers
Nautilus Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-07 12:44 UTC by Georges Basile Stavracas Neto
Modified: 2016-01-18 12:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
window-slot: fix search state when going back/forward (3.76 KB, patch)
2015-10-07 12:44 UTC, Georges Basile Stavracas Neto
none Details | Review
window-slot: fix search state when going back/forward (4.51 KB, patch)
2015-10-07 13:20 UTC, Georges Basile Stavracas Neto
none Details | Review
window-slot: fix search state when going back/forward (3.86 KB, patch)
2015-10-07 19:28 UTC, Georges Basile Stavracas Neto
none Details | Review
search-engine-model: fix infinity waiting (1.98 KB, patch)
2015-12-17 14:53 UTC, Carlos Soriano
none Details | Review
window-slot: sync search visibility when using history (5.17 KB, patch)
2015-12-17 14:53 UTC, Carlos Soriano
none Details | Review
search-engine-model: fix infinity waiting (1.98 KB, patch)
2015-12-17 14:54 UTC, Carlos Soriano
committed Details | Review
window-slot: sync search visibility when using history (5.21 KB, patch)
2015-12-17 14:54 UTC, Carlos Soriano
committed Details | Review

Description Georges Basile Stavracas Neto 2015-10-07 12:44:09 UTC
When a search is performed, Nautilus adds it to the history
stack, and we can return to it through the back or forward
buttons.

While we can indeed go back to the search, the UI components
such as the search bar doesn't update accordingly, leaving us
with a search directory without a search bar.

Fix that by properly checking the search-visible action state.
Comment 1 Georges Basile Stavracas Neto 2015-10-07 12:44:14 UTC
Created attachment 312819 [details] [review]
window-slot: fix search state when going back/forward
Comment 2 Georges Basile Stavracas Neto 2015-10-07 13:20:27 UTC
Created attachment 312822 [details] [review]
window-slot: fix search state when going back/forward

When a search is performed, Nautilus adds it to the history
stack, and we can return to it through the back or forward
buttons.

While we can indeed go back to the search, the UI components
such as the search bar doesn't update accordingly, leaving us
with a search directory without a search bar.

Fix that by properly checking the search-visible action state.
Comment 3 Carlos Soriano 2015-10-07 14:41:30 UTC
Review of attachment 312822 [details] [review]:

::: src/nautilus-window-slot.c
@@ +283,3 @@
+                        nautilus_window_slot_set_search_visible (slot, TRUE);
+                }
+

nautilus_window_slot_set_search_visible already sets the query and performs the required actions. Also, since this only sets the action, everything is well managed. So this code can be as simple as:
if (nautilus_view_is_searching (view)) {
    /* make sure the slot shows the search if the view
     * we switched is searching. This can happens when using
     * the toolbar back and forward buttons. */
    nautilus_window_slot_set_search_visible (slot, TRUE);
    return;
}
Comment 4 Georges Basile Stavracas Neto 2015-10-07 19:28:16 UTC
Created attachment 312850 [details] [review]
window-slot: fix search state when going back/forward

When a search is performed, Nautilus adds it to the history
stack, and we can return to it through the back or forward
buttons.

While we can indeed go back to the search, the UI components
such as the search bar doesn't update accordingly, leaving us
with a search directory without a search bar.

Fix that by properly checking the search-visible action state.
Comment 5 Carlos Soriano 2015-12-16 15:03:22 UTC
Review on IRC, to not lose it again...

csoriano_	feaneron: I think I remember, I didn't like you are not using the already there nautilus_files_view_set_search_query 
and instead modifying the private data directly
feaneron	ooh, yes
csoriano_: and I gave up working on that because it was causing undefined behavior
csoriano_	feaneron: yes
and I remember one of the reasons is that all the signaling is done there
we have the is-searching signal...
in this case it's going to be no-op, not sure if that's correct or not
with the current code it doesn't matter
but, we should be consistent
feaneron	technically it's a property
csoriano_	and have only one code path for modifying properties
yep
so basically, we have to figure out a way to make it work goign tru it
and be consistent with signaling
(in this case it should signal, the view indeed starts searching and therefore it's needed for showing progress or whatever)
we lost all of that if we just set the internal data
instead of going tru it
Comment 6 Carlos Soriano 2015-12-17 14:53:22 UTC
Created attachment 317569 [details] [review]
search-engine-model: fix infinity waiting

How search works:
The main engine starts.
It starts in order all the search providers which start one
or more threads.
Then the owner of the engine can decide to stop, and therefore
requesting the providers to stop.
Then the providers take their time in the different threads to cancel
and to report to the engine, which is the main thread, that they
finished.
At that point the engine signals that the engine is stopped and
finished.

However, if one of the search providers fail to report it's finalization
the engine is hanging forever, making everything stopping to work.

This was the issue when the engine requests the model provider to stop,
and then start again before it got time to request the directory info,
since we add the iddle but never rested it's id, so never signal a finish.

This was working most of the times because, this idle is only requested
when we stop the model provider and it's still running, but usually the
work the model has to do is so little that always gets finished before
stopping it.

So to fix this issue, reset the idle id when finished.
Comment 7 Carlos Soriano 2015-12-17 14:53:28 UTC
Created attachment 317570 [details] [review]
window-slot: sync search visibility when using history

We can return to an old location which happens to be a search
location.
The issue is, we were not updating all the associated properties
of the search when that happened so the window slot and the
files view was in a inconsistent state.

For that, we need the files view to also accept to change to search
locations, in which case it needs to set it's query appropiately
to the query the search location was using.
Comment 8 Carlos Soriano 2015-12-17 14:54:00 UTC
Created attachment 317573 [details] [review]
search-engine-model: fix infinity waiting

How search works:
The main engine starts.
It starts in order all the search providers which start one
or more threads.
Then the owner of the engine can decide to stop, and therefore
requesting the providers to stop.
Then the providers take their time in the different threads to cancel
and to report to the engine, which is the main thread, that they
finished.
At that point the engine signals that the engine is stopped and
finished.

However, if one of the search providers fail to report it's finalization
the engine is hanging forever, making everything stopping to work.

This was the issue when the engine requests the model provider to stop,
and then start again before it got time to request the directory info,
since we add the iddle but never rested it's id, so never signal a finish.

This was working most of the times because, this idle is only requested
when we stop the model provider and it's still running, but usually the
work the model has to do is so little that always gets finished before
stopping it.

So to fix this issue, reset the idle id when finished.
Comment 9 Carlos Soriano 2015-12-17 14:54:05 UTC
Created attachment 317574 [details] [review]
window-slot: sync search visibility when using history

We can return to an old location which happens to be a search
location.
The issue is, we were not updating all the associated properties
of the search when that happened so the window slot and the
files view was in a inconsistent state.

For that, we need the files view to also accept to change to search
locations, in which case it needs to set it's query appropiately
to the query the search location was using.

Thanks to Georges for the initial work on this.
Comment 10 Carlos Soriano 2015-12-17 15:00:37 UTC
So it needs two fixes, but it ended to call the same code path :)
Not too different from what you did, thanks for your work!

Attachment 317573 [details] pushed as 95cadcc - search-engine-model: fix infinity waiting
Attachment 317574 [details] pushed as f24fd69 - window-slot: sync search visibility when using history
Comment 11 Carlos Soriano 2016-01-18 12:25:44 UTC
To 3.18 and master