380997 - migrating from no-places-bookmarks to places-bookmarks gives me the "old places style" favicon for the personal toolbar

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

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

migrating from no-places-bookmarks to places-bookmarks gives me the "old places style" favicon for the personal toolbar

with a new places bookmark profile, we just use a folder icon.

here comes a screen shot

Comment 1

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

Attachment: screen shot of the "old places style" favicon

Comment 2

[Ria Klaassen (not reading all bugmail)]

I thought this was intended for some reason :S.

Comment 3

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

dietrich points out that this problem will be fixed with a schema change, which is coming for bug #371827

Comment 4

[Dietrich Ayala (:dietrich)]

this is caused because nsNavBookmarks::InitDefaults is never called when sidegrading from non-bookmarks to bookmarks.

when turning on bookmarks, we need to bump the schema version, in order to trigger a full re-initialization of the bookmarks tables to avoid this problem.

that change will be added to the patch to bug 370099.

Comment 5

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

we might be getting this old style favicon here:

http://lxr.mozilla.org/seamonkey/source/browser/components/places/src/nsPlacesImportExportService.cpp#1125

As for the other reference to I don't think this is used anymore:

/toolkit/components/places/content/places.properties, line 17 -- PlacesBookmarksToolbarIconURI=chrome://browser/skin/places/bookmarksToolbar.png

Comment 6

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

I think we may want to clean up this code as well:

129 #define BOOKMARKSS_MENU_ICON_URI "chrome://browser/skin/places/bookmarksMenu.png"
130 #define BOOKMARKSS_TOOLBAR_ICON_URI "chrome://browser/skin/places/bookmarksToolbar.png"

also, it looks like we have some unused properties in http://lxr.mozilla.org/seamonkey/source/toolkit/components/places/content/places.properties#17

Comment 7

[Dan Mills [:thunder]]

Attachment: Remove toolbar icon + #define and properties entry

Doesn't set the toolbar icon, removes the #define for the chrome:// uri to it, and removes the property.

Thanks for a well-documented bug, Seth!

Comment 8

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

Comment on attachment 265228 [details] [diff] [review]
Remove toolbar icon + #define and properties entry

r=sspitzer

Comment 9

[Dan Mills [:thunder]]

Checking in browser/components/places/src/nsPlacesImportExportService.cpp;
/cvsroot/mozilla/browser/components/places/src/nsPlacesImportExportService.cpp,v  <--  nsPlacesImportExportService.cpp
new revision: 1.14; previous revision: 1.13
done
Checking in toolkit/components/places/content/places.properties;
/cvsroot/mozilla/toolkit/components/places/content/places.properties,v  <--  places.properties
new revision: 1.2; previous revision: 1.1
done

Comment 10

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

thanks for the fix, dan.

Can you log some spin off bugs?

1) let's clean up this code as well:

#define BOOKMARKSS_MENU_ICON_URI
"chrome://browser/skin/places/bookmarksMenu.png"

 PlacesBookmarksRootIconURI=chrome://browser/skin/places/bookmarksMenu.png

1104           rv = SetFaviconForFolder(ourID,
NS_LITERAL_CSTRING(BOOKMARKSS_MENU_ICON_URI));
1105           NS_ENSURE_SUCCESS(rv, rv);

I think we can also clean up more code for handling chrome urls special, see 

1474   if (!faviconScheme.EqualsLiteral("chrome")) {
1475     // only store data for non-chrome URIs

2)  should we setting the descriptions?

 PlacesBookmarksToolbarDescription=Add bookmarks to this folder to see them
displayed on the Bookmarks Toolbar
 PlacesBookmarksRootDescription=Add bookmarks to this folder to see them
displayed on the Bookmarks Menu

Comment 11

[Dan Mills [:thunder]]

Done!  Filed (1) as 381141, and (2) as 381142.

Comment 12

[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