GNOME Bugzilla – Bug 770823
Items are not removed from FAVORITES when unfavorited
Last modified: 2016-09-11 22:28:45 UTC
This can be seen in two parts: (a) Favorite "star" is unresponsive (b) Deleting an image under "Favorties" tab (a) Steps to reproduce: Case I: 1) Mark an image as favorite under selection mode. 2) Hop over to "Favorites" tab and unmark the image as favorite from selection mode. 3) You'll see the "star" disappears but the image is still present in the view 4) Hop over to "Photos" tab and try to mark the same image as favorite again. No "star" icon appears on the image. At point #3, the image should be removed from the view. But it does not happen. Case II: 1) Mark an image as favorite under selection mode. 2) Hop over to "Favorites" tab and terminate the application. Restart. 3) Now, in the overview, find the image you marked as favorite earlier. 4) Unmark it as favorite from selection mode. 5) Hop over to "Favorites" tab and come back to "Photos" tab. 5) Again try to mark the image as favorite from selection mode. 6) You should get output similar to : https://paste.gnome.org/p4nub0d0u (b) Steps to reproduce: 1) Mark an image as favorite under selection mode. 2) Hop over to "Favorites" tab and try to delete the image using selection mode. 3) **ERROR:photos-item-manager.c:988:photos_item_manager_hide_item: 'item == item1' should be TRUE encountered. NOTE: Try resetting the tracker's database if you cannot reproduce using these steps.
There might be one more use case which needs to be fixed i.e. marking/unmarking favorite if image is a part of collection but I think that should be figured out exactly after we land patches for https://bugzilla.gnome.org/show_bug.cgi?id=770824
Created attachment 334735 [details] [review] item-manager: Clear mode specific items from all child base managers
Created attachment 334736 [details] [review] item-manager: Fix logic of marking favorites
Created attachment 334749 [details] [review] item-manager: Reconnect "info-updated" signal handler while unhiding item Case in which this patch will be useful: 1) Mark image as favorite 2) Hop over to "Favorites" tab 3) Delete that image using selection mode. 4) "Undo" delete (unhiding logic) 5) Try to unmark the image as favorite. You will notice that the "star" on the image disappears but the image still remains in the view. This is because while removing the image, the "info-updated" signal handler got disconnected and thus, the handler is not called while unmarking the image as favorite (step 5). Therefore, we should re-connect the handler while unhiding item.
Review of attachment 334735 [details] [review]: Thanks for testing things, Umang. It is a big help. ::: src/photos-item-manager.c @@ +867,1 @@ + photos_base_manager_remove_object_by_id (self->item_mngr_chldrn[i], id); Unfortunately, this is wrong. Clearing a child base manager should not affect the other children. Or at least I don't see why.
Review of attachment 334736 [details] [review]: Good catch! ::: src/photos-item-manager.c @@ +359,3 @@ photos_base_manager_add_object (self->item_mngr_chldrn[PHOTOS_WINDOW_MODE_FAVORITES], G_OBJECT (item)); + else + photos_base_manager_remove_object (self->item_mngr_chldrn[PHOTOS_WINDOW_MODE_FAVORITES], G_OBJECT (item)); Would be better if done a few lines below where we do all the removals.
Created attachment 334854 [details] [review] item-manager: Remove items from FAVORITES when unfavorited
(In reply to Umang Jain from comment #0) > (b) Steps to reproduce: > > 1) Mark an image as favorite under selection mode. > 2) Hop over to "Favorites" tab and try to delete the image using selection > mode. > 3) **ERROR:photos-item-manager.c:988:photos_item_manager_hide_item: 'item == > > item1' should be TRUE encountered. > > NOTE: Try resetting the tracker's database if you cannot reproduce using > these steps. This seems to be fixed too. I did see that if all items inside the view are deleted, we don't get the 'no-results' message. This is because the items are still present in the DB (which means that the queries are fooled), even though we fake their removal in the application. I would suggest filing a separate bug for that.
Review of attachment 334749 [details] [review]: Nice catch, Umang. Thanks. ::: src/photos-item-manager.c @@ +1017,3 @@ + { + photos_base_manager_add_object (self->item_mngr_chldrn[i], G_OBJECT (item)); + g_signal_connect_object (item, "info-updated", G_CALLBACK (photos_item_manager_info_updated), self, 0); We don't need to connect multiple times, so better to move this outside.
Created attachment 334921 [details] [review] item-manager: Reconnect to BaseItem::info-updated when unhiding item
Review of attachment 334921 [details] [review]: Looks good to me. All cases listed in above comments have been again tested with relevant patches applied. They are fixed now. Can be closed after all patches have been pushed to master.
Thanks for testing, Umang.