Closed Bug 429811 Opened 14 years ago Closed 14 years ago

Tag container isn't updated immediately when bookmark is removed

Categories

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

defect

Tracking

()

VERIFIED FIXED
Firefox 3

People

(Reporter: whimboo, Assigned: dietrich)

References

Details

(Keywords: regression)

Attachments

(1 file)

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.
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?
Flags: blocking-firefox3?
Summary: Tag containers aren't updated immediately when adding/removing bookmarks → Tag container isn't updated immediately when bookmark is removed
could be, this is a live-update problem
We either need to fix this or disable copy/paste in these folders :(
Flags: blocking-firefox3? → blocking-firefox3+
comment #1 is about delete, so there's something more to check
(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

Marco, shall I file a new bug for the copy&paste issue so we can concentrate on the delete action here?
i'd wait to see what we can solve here, we will always be able to fill a followup later if needed.
Assignee: nobody → dietrich
Whiteboard: [ETA ?]
Whiteboard: [ETA ?] → [swag: 1d]
Attached patch fixSplinter Review
always refresh if the container is open

marco, can you do first-r please?
Attachment #318639 - Flags: review?(mak77)
Status: NEW → ASSIGNED
Keywords: regression
Priority: -- → P3
Whiteboard: [swag: 1d] → [has patch][needs review marco]
Target Milestone: --- → Firefox 3
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.
Attachment #318639 - Flags: review?(mak77) → review+
Attachment #318639 - Flags: review+ → review?(mano)
Whiteboard: [has patch][needs review marco] → [has patch][needs review mano]
Will this fix just the Tag containers or does it also address the broader scope of bug 407443
should affect only tag containers as it is
Comment on attachment 318639 [details] [diff] [review]
fix

r=mano
Attachment #318639 - Flags: review?(mano) → review+
Comment on attachment 318639 [details] [diff] [review]
fix

a=mconnor on behalf of 1.9 drivers
Attachment #318639 - Flags: approval1.9+
Keywords: checkin-needed
Whiteboard: [has patch][needs review mano] → [has patch][has reviews][has approval]
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
Status: ASSIGNED → RESOLVED
Closed: 14 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [has patch][has reviews][has approval]
Verified fixed in Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9pre) Gecko/2008050604 Minefield/3.0pre.
Status: RESOLVED → VERIFIED
Flags: in-litmus?
Test case https://litmus.mozilla.org/show_test.cgi?searchType=by_id&id=7477 has been created in litmus for regression testing.
Flags: in-litmus? → in-litmus+
(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.
Depends on: 410196
Flags: in-litmus+ → in-litmus?
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.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.