429811 - Tag container isn't updated immediately when bookmark is removed

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect, P3)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008041804 Minefield/3.0pre and on Windows XP

If you are adding new bookmark via copy&paste to a tag container or remove them on a later time the view isn't updated. This doesn't happen for bookmarks which were added via the tag field within the bookmark properties.

Steps to reproduce:

1. Create a fresh profile
2. Add a tag e.g. "Mozilla" to an already existing bookmark
3. Open history container and hit Cmd+C/Ctrl+C to copy the item
4. Open the tag container "Mozilla" and hit Cmd+V/Ctrl+V
=> You have to reopen the tag container to see the newly added bookmark
5. Delete the new bookmark from the tag container
=> You have to reopen the tag container again to see the updated view

As I said this only happens for bookmarks which were tagged via copy&paste into a tag container. I can see the mBatchInProgress Assertion here. So adding bug 423212 to the depends list.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+2]]

The behavior changes a bit. Following STR shows the miss-behavior:

1. Adding two bookmarks and give both the same tag 'XXX'
2. Open the tag container 'XXX' and remove one of these bookmarks
=> The deleted bookmark is still shown until you reopen the tag container

Marco, could this be a regression from the patch which makes tag containers special folders?

Comment 2

[Marco Bonardo [:mak]]

could be, this is a live-update problem

Comment 3

[Mike Beltzner [:beltzner, not reading bugmail]]

We either need to fix this or disable copy/paste in these folders :(

Comment 4

[Marco Bonardo [:mak]]

comment #1 is about delete, so there's something more to check

Comment 5

[Marco Bonardo [:mak]]

(In reply to comment #0)
> I can see the mBatchInProgress Assertion here. So adding bug
> 423212 to the depends list.

i doubt that's related in any way, hwv let's see when we will catch the actual cause

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Marco, shall I file a new bug for the copy&paste issue so we can concentrate on the delete action here?

Comment 7

[Marco Bonardo [:mak]]

i'd wait to see what we can solve here, we will always be able to fill a followup later if needed.

Comment 8

[Dietrich Ayala (:dietrich)]

Attachment: fix

always refresh if the container is open

marco, can you do first-r please?

Comment 9

[Marco Bonardo [:mak]]

Comment on attachment 318639 [details] [diff] [review]
fix

from what i see everything works and there is no big perf loose (that i have on tag remove if i kill the early return). So i'm fine with this.

Comment 10

[Tracy Walker [:tracy]]

Will this fix just the Tag containers or does it also address the broader scope of bug 407443

Comment 11

[Marco Bonardo [:mak]]

should affect only tag containers as it is

Comment 12

[Asaf Romano (gone)]

Comment on attachment 318639 [details] [diff] [review]
fix

r=mano

Comment 13

[Mike Connor [:mconnor]]

Comment on attachment 318639 [details] [diff] [review]
fix

a=mconnor on behalf of 1.9 drivers

Comment 14

[Dietrich Ayala (:dietrich)]

Checking in toolkit/components/places/src/nsNavHistoryResult.cpp;
/cvsroot/mozilla/toolkit/components/places/src/nsNavHistoryResult.cpp,v  <--  nsNavHistoryResult.cpp
new revision: 1.144; previous revision: 1.143
done

Comment 15

[Al Billings [:abillings - ex-MoCo]]

Verified fixed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050604 Minefield/3.0pre.

Comment 16

[Aakash Desai [:aakashd]]

Test case https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7477 has been created in litmus for regression testing.

Comment 17

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #16)
> Test case https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7477 has
> been created in litmus for regression testing.

It would be friendlier to restrict all of those steps to the Library window only, instead of having the need to open the history sidebar. Could you update this please?

Further we should deactivate this test until bug 410196 has been fixed.

Comment 18

[Aakash Desai [:aakashd]]

The test case has been changed to reflect Henrik's suggestions and de-activated. I've added a comment to bug 410196 about the de-activated test case.(In reply to comment #17)
> (In reply to comment #16)
> > Test case https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7477 has
> > been created in litmus for regression testing.
> 
> It would be friendlier to restrict all of those steps to the Library window
> only, instead of having the need to open the history sidebar. Could you update
> this please?
> 
> Further we should deactivate this test until bug 410196 has been fixed.

Comment 19

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv