381280 - Places bookmarks.html importer ignores "browser.bookmarks.file" pref

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect)

Description

[Steffen Wilberg]

The Places bookmarks.html importer ignores the "browser.bookmarks.file" pref.
I've set that pref like this in my prefs.js:
user_pref("browser.bookmarks.file", "f:\\bookmarks.html");

That's the location of my bookmarks file (which I use for both my Windows and my Linux profiles and which works just fine in non-places-bookmarks builds).
After updating to the places-bookmarks-enabled nightly (Windows 2007051904), Minefield presented me the profile's default bookmarks ("Getting Started", "Latest Headlines" etc.).
It should have imported my own bookmarks from the above mentioned location.

Workaround is to import manually:
Menu Bookmarks -> Bookmarks Manager -> Menu File -> Import.

Comment 1

[Steffen Wilberg]

When closing Minefield, the bookmarks get exported to the correct location specified by the browser.bookmarks.file pref.

Comment 2

[Joey Minta]

-> me, should have a patch as soon as i get my tree updated and rebuilt to test.

Comment 3

[Joey Minta]

Attachment: patch

Checks for the pref and imports the proper file.  Note that the pref is normally undefined, so it'll throw an error when we try to read it, putting us back in the normal bookmarks.html path.

Comment 4

[Asaf Romano (gone)]

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

1. Set the pref in Firefox  2/1.5.
2. Apply this patch and start with the same profile in a places-bookmarks build, do some change to your bookmarks, quit.

At this point, we export the bookmarks.html file to profiledir/bookmarks.html

3. Bump the schema version and force import (we might do that in a future release).
4. Start with the same profile again

Your changes are lost, and we'll import from the old bookmarks.html (the one set in the pref).

So the pref should be either supported by the export code-path, or get unset after initial import.

Comment 5

[Joey Minta]

I'm confused.  Comment #1 points out that the export code already does support this.

http://mxr.mozilla.org/seamonkey/source/browser/components/places/src/nsPlacesImportExportService.cpp#2471 calls http://mxr.mozilla.org/seamonkey/source/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp#122 which respects the pref.

Comment 6

[Asaf Romano (gone)]

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

you win ;) but then shouldn't we use the same technique for the import case?

Comment 7

[Joey Minta]

Attachment: patch v2

This time using the dir-service.

Comment 8

[Asaf Romano (gone)]

Comment on attachment 265393 [details] [diff] [review]
patch v2

r=mano.

Comment 9

[Joey Minta]

Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.26; previous revision: 1.25
done

Comment 10

[Asaf Romano (gone)]

I backed this out due to both Ts and Txul hits.

Comment 11

[Joey Minta]

We could try checking in the first patch, to see whether it's the dirService that is slow here.  If it's the getCharPref call, then I don't see how the hit is avoidable.  I thought prefs were supposed to be really fast however, so as a wild-guess, I'm suspicious of NS_NewNativeLocalFile http://mxr.mozilla.org/seamonkey/source/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp#129

Comment 12

[Joey Minta]

Attachment: perf patch

I'm going to land this patch along with re-landing "patch v2" in just a minute.  This should fix the Ts problems previously reported. r=mano over irc.

Comment 13

[Joey Minta]

Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.28; previous revision: 1.27
done
Checking in browser/components/dirprovider/nsBrowserDirectoryProvider.cpp;
/cvsroot/mozilla/browser/components/dirprovider/nsBrowserDirectoryProvider.cpp,v  <--  nsBrowserDirectoryProvider.cpp
new revision: 1.17; previous revision: 1.16
done

Comment 14

[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