399739 - The "Recently Starred Pages" and "Most Visited Starred Pages" in "Places" folder shows live bookmarks as starred pages.

Status: VERIFIED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[alreiten]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101304 Minefield/3.0a9pre
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101304 Minefield/3.0a9pre

In the new Places dropdown menu on the the bookmarks toolbar, I noticed that the "Recently Visited Starred Pages" menu showed several sites that were from my live bookmarks.  I think that most users would think that live bookmarks are not the same as regular bookmarks (starred pages) and that any sites from the rss/atom feeds should not appear in this category. 

Reproducible: Always

Steps to Reproduce:
1. Visit a site using the built-in live bookmark functionality. 
2. Check the "Recently Visited Starred Pages" in Places.
3.
Actual Results:  
The live bookmark site appears in the "Recently Visited Starred Pages" menu in Places.

Expected Results:  
I think that as live bookmarks sites are not actually starred or saved like regular bookmarks, they should not appear in this menu.

Comment 1

[(not reading, please use [e-mail] instead)]

good catch, thanks!

I'm able to reproduce this.

the problem is in initPlacesDefaultQueries(), I failed to add

"&excludeItemIfParentHasAnnotation=livemark%2FfeedURI"

mostVisitedBookmarksItem and recentlyVisitedBookmarksItem 

(but I did for recentlyCreatedBookmarksItem).

I'll attach a simple fix.

unfortunately, it will only fix the problem for new profiles (ones that have not already run the code to create these special queries)

Comment 2

[(not reading, please use [e-mail] instead)]

Attachment: patch

again, this does not fix this issue for existing users (who already have a "Places" folder), only for new profiles and for those who haven't run a recent trunk build yet.

Comment 3

[Dietrich Ayala (:dietrich)]

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

>Index: browser/base/content/browser.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/base/content/browser.js,v
>retrieving revision 1.870
>diff -u -8 -p -r1.870 browser.js
>--- browser/base/content/browser.js	13 Oct 2007 02:43:48 -0000	1.870
>+++ browser/base/content/browser.js	17 Oct 2007 00:50:43 -0000
>@@ -367,25 +367,27 @@ function initPlacesDefaultQueries() {
>             "&excludeItemIfParentHasAnnotation=livemark%2FfeedURI" +
>             "&maxResults=" + maxResults +
>             "&excludeQueries=1"),
>             defaultIndex, recentlyCreatedBookmarksTitle);
> 
>       var recentlyVisitedBookmarksItem = bmsvc.insertBookmark(placesFolder,
>         IO.newURI("place:folder=" + bookmarksRoot + "&folder=" + unfiledRoot +
>             "&queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
>-            "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_DESCENDING + 
>+            "&sort=" + Ci.nsINavHistoryQueryOptions.SORT_BY_DATE_DESCENDING +
>+            "&excludeItemIfParentHasAnnotation=livemark%2FfeedURI" + 
>             "&minVisits=1&maxResults=" + maxResults),
>             defaultIndex, recentlyVisitedBookmarksTitle);
> 
>       var mostVisitedBookmarksItem = bmsvc.insertBookmark(placesFolder,
>         IO.newURI("place:folder=" + bookmarksRoot + "&folder=" + unfiledRoot +
>             "&queryType=" + Ci.nsINavHistoryQueryOptions.QUERY_TYPE_BOOKMARKS +
>             "&sort=" + 
>             Ci.nsINavHistoryQueryOptions.SORT_BY_VISITCOUNT_DESCENDING + 
>+            "&excludeItemIfParentHasAnnotation=livemark%2FfeedURI" +

nit: could also remove the whitespace at the end of the preceding 2 lines.

Comment 4

[(not reading, please use [e-mail] instead)]

Thanks for the prompt review, dietrich.

I'll log a spin off bug (for completeness) about existing profiles, but that is going to be low priority / wontfix.

> nit: could also remove the whitespace at the end of the preceding 2 lines.

will do, on check in (those and a few others.)

Comment 5

[(not reading, please use [e-mail] instead)]

fixed

Checking in browser.js;
/cvsroot/mozilla/browser/base/content/browser.js,v  <--  browser.js
new revision: 1.871; previous revision: 1.870
done

Comment 6

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

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

Comment 7

[Reed Loden [:reed]]

Are the default bookmarks supposed to show up in the "Recently Starred Pages" query? They aren't really recently starred... they are just the default bookmarks.

Comment 8

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

Making something a bookmark and starring it are the same thing. If you go to a bookmarked page, you'll see the star.

Since the default aren't "recent" really, you may have a point if you are talking about a new profile.

Comment 9

[Reed Loden [:reed]]

(In reply to comment #8)
> Since the default aren't "recent" really, you may have a point if you are
> talking about a new profile.

Yeah, I'm talking about a new profile.

Comment 10

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

Of course, technically they are new. The creation date will be recent...

Comment 11

[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