GNOME Bugzilla – Bug 771038
Undo can put the item back in the wrong collection
Last modified: 2018-01-23 10:06:07 UTC
Delete an image that is inside a collection. Then open a different collection and undo the deletion. The image is placed inside the current collection, even though it isn't a part of it.
(In reply to Debarshi Ray from comment #0) > Delete an image that is inside a collection. Then open a different > collection and undo the deletion. The image is placed inside the current > collection, even though it isn't a part of it. but if we go back and again open that collection then it is gone. basically the function photos_delete_notification_undo_clicked in photos_delete_notification.c takes every item that was hidden in the other collection(in which we deleted the image) and calls photos_item_manager_unhide_item function on it, which simply takes the hidden image and without checking whether it is a part of the collection or not places it back in the view. so if we just reload the collection again after the loop in undo_clicked function(same way as we click back button and open the collection again), that image will not create any problem(it will work well if the hidden items are small enough in number so that they are not displayed for a lot of time,after the undo button is clicked and after that we reload the collection again.), we can't stop the un_hide function from getting called as that would mean that the hidden image is not displayed in the collection in which it was a part of,so calling unhide function is must. i'm not able to figure out how to re-open the active collection, i got the collection info using get-active-collection function but how can i use it to re-open the existing collection?
actually this problem is a little more complicated, 1. if after deleting the image we go back just once we reach the collections and if we click undo the photo will be displayed there in the collections tab and it doesn't go away until we open any collection again and click back button. 2. if 2 collections contain the same photo and we delete it from any one collection and go back and open the other collection we see that the photo is there, it is not hidden and if we click undo nothing happens but if now we again try to delete the image from this collection we get this error ERROR:photos-item-manager.c:1029:photos_item_manager_hide_item: 'item == item1' should be TRUE the photos_item_manager_hide_item function and photos_item_manager_unhide_item function must be modified to hide and unhide the items at every place they are present. for that we will need to store information in the item itself about the collections it is present and use that info to somehow make them hidden in all of the collections and also in the main photos view so that we don't get into an inconsistent state as described in problem no. 2 above how can i open the collections in the background and use hide and unhide function without freezing the application? how to recreate the collections view so that problem no. 1 is also solved? and where should i look to add the info about collections in which the images are present when they are loaded?
This bug is a bit complicated. The problem lies in the functioning of these two methods: * photos_item_manager_hide_item * photos_item_manager_unhide_item I think the first step to solve this would be to add a separate mode for displaying the contents of a single collection. Right now, if you click on a collection in the collection view (ie. PHOTOS_WINDOW_MODE_COLLECTIONS) or in the search results (ie. PHOTOS_WINDOW_MODE_SEARCH), it is still the same mode (and view) that is showing the items inside the collection. We should introduce a PHOTOS_WINDOW_MODE_COLLECTION_BROWSE or similar to separate this. This will also simplify a bunch of other things. Secondly, each BaseItem should keep track of its parent collections, so that when we are restoring it, we can determine whether it should be added to the collection that is being browsed.
now this bug is linked with an enhancement, so first someone will have to do this enhancement and then we can solve this bug. can you give me some reference where I can find standard code to create a new view like PHOTOS_WINDOW_MODE_COLLECTION_BROWSE ?
(In reply to Kartikeya Sharma from comment #4) > now this bug is linked with an enhancement, so first someone will have to do > this enhancement and then we can solve this bug. Some bugs are easier to solve, some need more effort. This is the latter. > can you give me some reference where I can find standard code to create a > new view like PHOTOS_WINDOW_MODE_COLLECTION_BROWSE ? There is no "standard code to create a new view". You will have to grep and read the code to get the general feel of it. After that I can answer more specific questions.
Created attachment 341675 [details] [review] photos-item-manager: Added PHOTOS_WINDOW_MODE_COLLECTION_BROWSE
Created attachment 341676 [details] [review] photos-base-item: Added GSList parent_collections to _PhotosBAseItemPrivate to keep track of collections in which item is present.
(In reply to Debarshi Ray from comment #5) > There is no "standard code to create a new view". You will have to grep and > read the code to get the general feel of it. After that I can answer more > specific questions. how should the functionality of hide function be changed? we call hide function on a photo to hide it in the collection we have opened and also hide it in all other collections in which it is present. i am not able to figure out how to make it work on achieving the latter part. And what do you want me to do with that new mode (PHOTOS_WINDOW_MODE_COLLECTION_BROWSE)?, what is it going to simplify?
(In reply to Kartikeya Sharma from comment #8) > (In reply to Debarshi Ray from comment #5) > > There is no "standard code to create a new view". You will have to grep and > > read the code to get the general feel of it. After that I can answer more > > specific questions. > > how should the functionality of hide function be changed? The first step would be to understand how it works right now, and why it doesn't work in this particular situation. Currently, when you hide an item we track both the item and all the modes in which it was present. Each 'view' in the application is represented as a mode. That way, when you unhide it, we can restore it in all the different places where it was before. If an item is hidden from inside a collection, and we come out of the collection before hiding it, then it breaks because knowing that it was inside MODE_COLLECTIONS is not enough to tell us which collection it was. > And what do you want me to do with that new mode > (PHOTOS_WINDOW_MODE_COLLECTION_BROWSE)?, what is it going to simplify? Didn't attachment 341675 [details] [review] generate quite a few warnings during the build? You could start by looking at those to understand how the different modes are used.
Review of attachment 341675 [details] [review]: Thanks for the patches, Kartikeya. We can only merge these once they are more complete. ::: src/photos-item-manager.h @@ +72,3 @@ PHOTOS_WINDOW_MODE_PREVIEW, + PHOTOS_WINDOW_MODE_SEARCH, + PHOTOS_WINDOW_MODE_COLLECTION_BROWSE We should at least silence the -Wswitch-enum warnings.
Comment on attachment 341675 [details] [review] photos-item-manager: Added PHOTOS_WINDOW_MODE_COLLECTION_BROWSE Obsoleted by bug 786936
Review of attachment 341676 [details] [review]: This is obviously incomplete. :)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/gnome-photos/issues/52.