407443 - menus for query based folders (smart bookmarks, saved searches) not refreshed after deleting an item

Status: VERIFIED FIXED

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

Description

[Jo Hermans]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2pre) Gecko/2007120705 Minefield/3.0b2pre

When I open "Smart Bookmarks -> Recently Bookmarked", I can right-click on an entry, and choose delete, while the list of recent bookmarks statys open (underneath the context menu). This deletes the bookmark from the Places database, but doesn't refresh the list. When I click away (clikc on "Most Visited" for instance), and then come back, the list is correctly refreshed.

Deleting bookmarks in other pulldown lists works correctly, but not in the Smart menu.

Comment 1

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

Especially on OS X I don't get the context menu. Is there a difference between the OS or was it also changed for Windows?

Comment 2

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

It's still visible on WinXP. Seems that the missing context menu is an issue on OS X. Will file a new bug about.

Comment 4

[David Naylor]

This bug seems to apply for all saved searches. Perhaps the description should be changed?

Comment 5

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

"Most Visited" and "Recent Tags" are working fine for me with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b4pre) Gecko/2008030120 Minefield/3.0b4pre ID:2008030120

What saved searches are you talking about?

Comment 6

[David Naylor]

If I do a bookmark search in Library and save it as a saved search, and then delete an item from the saved search folder I just created, the bookmark is deleted from the database but the list isn't refreshed. Just as the bug description, but for other saved searches as well as the aforementioned "recently bookmarked".

Comment 7

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

David, this bug is about the "Smart Bookmarks > Recently Bookmarked" entry under the Bookmarks Toolbar. If you have problems within the Library please file a new bug or try to find an existing one which covers your issue.

Comment 8

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

JFI: The refresh happens now when reopening the "Recently Bookmarked" sub menu.

Comment 9

[David Naylor]

Sorry, I wasn't clear:

I created a saved search and put it in the bookmark toolbar. Then I deleted a bookmark in the search folder on the toolbar. The folder view didn't refresh.

My point is that it must be the same code which is causing my saved search not to refresh, as the code that is causing the folder "recently bookmarked" not to refresh. They are both saved searches, only one is provided as a default in Firefox.

I suggest the description of this bug be changed to "List of entries not refreshed when deleting a bookmark within Smart Bookmarks *or saved search*".

Comment 11

[Marco Bonardo [:mak]]

IIRC dietrich has pointed to a live update query problem in one of his blockers

Comment 12

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008041904 Minefield/3.0pre

Deleting "Most Visited" works for me in many (I believe not all) cases instantly, but "Recently Bookmarked" still needs a refresh. 

Comment 13

[Tracy Walker [:tracy]]

This, lack of refresh happens in places like Tag container lists of bookmarks as well. It is confusing in that the "Delete" doesn't initially appear to have worked.

Comment 14

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

Is this a dupe of/related to bug 429811?

Comment 15

[Tracy Walker [:tracy]]

Yes, bug 429811 is a specific instance of this broader bug.

Comment 16

[Dietrich Ayala (:dietrich)]

(In reply to comment #14)
> Is this a dupe of/related to bug 429811?
> 

(In reply to comment #15)
> Yes, bug 429811 is a specific instance of this broader bug.
> 

These are both regressions from bug 419731.

However, that bug is specific to tag containers, and is caused by a specific performance optimization marco made. Whereas this bug will require a different fix for these other container types.

Comment 17

[Dietrich Ayala (:dietrich)]

The most recent confirmation of this is 4/19. Tracy (or anyone), can you reproduce in the latest nightly? I'm not able to on Mac. I tried:

- saved searches
- the default queries

(Note: tag containers is a different case, handled in bug 429811)

Comment 18

[Tracy Walker [:tracy]]

I can only reproduce this in tag containers with builds from 20080430 on Mac and Windows. Entries from query based folders delete cleanly (if you ignore the flickering)  Perhaps the flickering bug is producing the side affect of refresh?  I'd like to wait 'til that bug is fixed to check this one again.

Comment 19

[Ria Klaassen (not reading all bugmail)]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9pre) Gecko/2008043004 Minefield/3.0pre

Problem is still present in the menus, Recently Bookmarked folder.

Comment 20

[Marco Bonardo [:mak]]

why depends on bug 431140 ??

Comment 21

[Dietrich Ayala (:dietrich)]

i can reproduce this on Windows. the steps, for clarification:

1. open Bookmarks Menu
2. select Recently Bookmarked
3. right-click on a bookmark, choose "delete"

expected: item is removed

actual: item is still there

workaround: close and re-open the menu

Comment 22

[Dietrich Ayala (:dietrich)]

adding qa-wanted, would help to have a regression range for this.

Comment 23

[Ria Klaassen (not reading all bugmail)]

It is not a regression; has never worked from the beginning.

Comment 24

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

Alexej, does it also happen under Linux?

Comment 25

[Chris]

I can reproduce this under Linux, exactly as above, through the Smart Bookmarks thing.

Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9b5) Gecko/2008041514 Firefox/3.0b5
HTH

Alternative workaround: only close the submenu. must work for the same reason as closing the whole menu though.

Comment 26

[Mike Connor [:mconnor]]

Need a patch ASAP, bottle of... something to the first to fix this. Flipping to Marco for now.

Comment 27

[Marco Bonardo [:mak]]

Attachment: patch

in the case of bookmarks query we have a QUERYUPDATE_COMPLEX_WITH_BOOKMARKS that ends up calling Refresh().
So we don't directly remove at a certain index like we do for folders (and that calls itemRemoved in the viewer), instead we simply do a invalidateContainer and set the popup _built attribute to false, this will rebuild the popup at the next opening (onPopupShowing calls _rebuild()).
Still in this case the popup is already open, so we don't call _rebuild, this patch check if the parent of the popup has the open attribute, in that case force a _rebuild that will then set again _built to true for future calls.

Comment 28

[Dietrich Ayala (:dietrich)]

Comment on attachment 319565 [details] [diff] [review]
patch

looks ok, r=me

Comment 29

[Mike Schroepfer]

Comment on attachment 319565 [details] [diff] [review]
patch

a+ schrep

Comment 30

[Dietrich Ayala (:dietrich)]

Checking in browser/components/places/content/menu.xml;
/cvsroot/mozilla/browser/components/places/content/menu.xml,v  <--  menu.xml
new revision: 1.134; previous revision: 1.133
done
Checking in browser/components/places/content/toolbar.xml;
/cvsroot/mozilla/browser/components/places/content/toolbar.xml,v  <--  toolbar.xml
new revision: 1.156; previous revision: 1.155
done

Comment 31

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

Tested with default and manually created saved searches. The menus are updated immediately.

Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050806 Minefield/3.0pre ID:2008050806

Comment 32

[Andrey Melnikov]

The menus are updated immediately.

Verified with Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.9pre) Gecko/2008050906 Minefield/3.0pre

Comment 33

[Tracy Walker [:tracy]]

Checking that deleting an item shows it deleted in all standard points is already in Litmus

Comment 34

[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