Merge lp://qastaging/~junrrein/pantheon-files/fix-1085662 into lp://qastaging/~elementary-apps/pantheon-files/trunk

Proposed by Julián Unrrein
Status: Merged
Merged at revision: 1113
Proposed branch: lp://qastaging/~junrrein/pantheon-files/fix-1085662
Merge into: lp://qastaging/~elementary-apps/pantheon-files/trunk
Diff against target: 60 lines (+28/-2)
2 files modified
libcore/gof-directory-async.vala (+19/-2)
libcore/gof-file.c (+9/-0)
To merge this branch: bzr merge lp://qastaging/~junrrein/pantheon-files/fix-1085662
Reviewer Review Type Date Requested Status
Mario Guerriero (community) Approve
Cody Garver (community) Abstain
Julián Unrrein (community) Needs Fixing
Review via email: mp+148315@code.qastaging.launchpad.net

Description of the change

When deleting a folder, remove it from the gof-directory-async cache to fix bug #1085662.

When loading a directory from gof-directory-async cache, make sure it has proper info.

To post a comment you must log in.
Revision history for this message
Julián Unrrein (junrrein) wrote :

I sort of forgot how to trigger the bug in views other than column view, sorry.

Revision history for this message
Julián Unrrein (junrrein) wrote :

The second commit is to solve this situation:

1) Go to a non-existent directory through the location bar.
2) Go to the parent folder and create that directory.

If you try to see the new directory's properties, it will show nothing, because in step 1 it was cached with a null fileinfo.

I hope you don't condemn me for not filing a bug report about this :P

Revision history for this message
Julián Unrrein (junrrein) wrote :

If you try to trigger the bug using Simon's screen cast as a guide, but you go further and after recreating the directory you also recreate its sub-folder, the files that were under those sub-folders will still show up.

So, when deleting a folder, Files should make sure to also delete its sub-folders from the gof-directory-async cache.

review: Needs Fixing
Revision history for this message
Julián Unrrein (junrrein) wrote :

Now it should do.

Revision history for this message
Julián Unrrein (junrrein) wrote :

There is a corner case that my fix doesn't cover.

Let's say you have ~/Test1/Test2/Test3/ bookmarked in your sidebar.
So you access Test3 from the sidebar (caching it).
Then you go back to Home, and delete Test1.
What will happen is that Test3 will not be removed from cache. My fix removes Test1 and tries to remove its children. As Test2 is not cached, the algorithm will stop there.

I couldn't think of a good way to cover this case. Except recreating the entire folder structure for the deleted folder and check for all of its items, but I think this would be too much.

Revision history for this message
Cody Garver (codygarver) wrote :

In icon view, open a folder, then start a new tab (tab B). Delete the folder open in tab A. Observe that it is still present in tab A. In tab A click Back then Forward and observe that the folder appears to still exist, although empty.

Revision history for this message
Julián Unrrein (junrrein) wrote :

Cody, I think that the situation you presented belongs in another bug report. The same situation happens no matter what you use to delete the current location of Files (a terminal, another instance of a file manager, etc.) and is not related with a caching issue. It's more about "handling the disappearance of the current folder".

Revision history for this message
Cody Garver (codygarver) :
review: Approve
Revision history for this message
Cody Garver (codygarver) :
review: Abstain
Revision history for this message
Mario Guerriero (mefrio-g) wrote :

it works for me and the branch be merged in my opinion, waiting for Cody approval/disapproval.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: