Last Comment Bug 381365 - No default bookmarks for profiles with a non-relative profile location
: No default bookmarks for profiles with a non-relative profile location
Status: VERIFIED FIXED
: regression
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: P1 normal with 1 vote (vote)
: Firefox 3
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
:
Mentors:
: 411114 424645 (view as bug list)
Depends on: 433185
Blocks: 386392
  Show dependency treegraph
 
Reported: 2007-05-20 13:36 PDT by Ria Klaassen (not reading all bugmail)
Modified: 2010-02-24 06:24 PST (History)
15 users (show)
mconnor: blocking‑firefox3-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Don't copy default profile files on initial profile creation, rev. 1 (2.06 KB, patch)
2008-01-07 07:07 PST, Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg]
no flags Details | Diff | Splinter Review
v2 (3.20 KB, patch)
2008-01-08 21:20 PST, Dietrich Ayala (:dietrich)
benjamin: review+
Details | Diff | Splinter Review
v3 (3.20 KB, patch)
2008-01-09 17:51 PST, Dietrich Ayala (:dietrich)
mbeltzner: approval1.9+
Details | Diff | Splinter Review
patch v4 (2.36 KB, patch)
2008-04-12 14:02 PDT, Gabriele Best [:Gabri]
no flags Details | Diff | Splinter Review
patch v4.1 (2.57 KB, patch)
2008-04-13 00:36 PDT, Gabriele Best [:Gabri]
no flags Details | Diff | Splinter Review
better patch (1.75 KB, patch)
2008-05-01 11:49 PDT, :Gavin Sharp [email: gavin@gavinsharp.com]
dietrich: review+
mbeltzner: approval1.9+
Details | Diff | Splinter Review
test fix (3.51 KB, patch)
2008-05-01 17:51 PDT, Dietrich Ayala (:dietrich)
no flags Details | Diff | Splinter Review

Description Ria Klaassen (not reading all bugmail) 2007-05-20 13:36:16 PDT
Steps to reproduce:

- Download and unzip a trunk build on your desktop
- Make a batch file with:
  md profile
  start C:\......\firefox\firefox.exe -Profile "profile/"

Result: bookmarks missing.
When I make a profile via the profile manager this does not happen.
Regression since about 2007-05-18 19.
Comment 1 Ria Klaassen (not reading all bugmail) 2007-06-28 13:56:14 PDT
This is still happening. A bit annoying when you want to test with the default set, but normal users won't even notice it. 
Comment 2 Ria Klaassen (not reading all bugmail) 2007-11-11 11:42:02 PST
It still doesn't copy the default profile data to the new profile. 
I get an error now when I try to drag a link to the toolbar:

Error: data has no properties
Source File: chrome://browser/content/places/utils.js
Line: 786
Comment 3 cmtalbert 2007-11-15 15:46:15 PST
Seth S and I are seeing the same sort of problems with the Minotaur partner/l10n test tool.  It creates its own profile directory for testing, and it cannot export the bookmarks.html file (which it needs) because the profile is not properly created when performing this type of profile creation.

Steps to reproduce:
1. mkdir c:\tmp\foo
2. firefox.exe -CreateProfile 'test c:\tmp\foo'

This will create a profile called test in c:\tmp\foo with only a prefs.js file.  If you run:

firefox.exe -CreateProfile bar

and create a profile in the standard location, then the newly created profile will have the following files in it.
* chrome (directory)
** userChrome-example.css
** userContent-example.css
* bookmarks.html
* localstore.rdf
* mimeTypes.rdf
* prefs.js

Because the bookmarks.html file is not created in the profile in the c:\tmp\foo directory, then running Firefox in that profile will not generate the default bookmark list properly.

Tested with the 3.0b1 RC 3 (Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b1) Gecko/2007110904 Firefox/3.0b1), Win XP.  Also found the same issue on MacOS X with 3.0 b1, RC 3.

In Firefox 2.0.0.9, this call creates profiles properly in the specified directories.
Comment 4 Sylvain Pasche 2007-12-23 15:23:43 PST
(In reply to comment #3)
> Seth S and I are seeing the same sort of problems with the Minotaur
> partner/l10n test tool.  It creates its own profile directory for testing, and
> it cannot export the bookmarks.html file (which it needs) because the profile
> is not properly created when performing this type of profile creation.
> 
> Steps to reproduce:
> 1. mkdir c:\tmp\foo
> 2. firefox.exe -CreateProfile 'test c:\tmp\foo'

For -CreateProfile to work, you need that you have no "test" profile in your profiles.ini and make sure the c:\tmp\foo directory does not exist. So you shouldn't create that directory before using -createprofile. If either you already have a profile called "test" or the c:\tmp\foo directory is there, it will just put the prefs.js file inside. I see the same behavior between branch and trunk.

More generally, there are two ways to create a profile "foo" located in /tmp/foo (using unixish conventions below):
1) * Make sure you have no profile called "foo" in your profiles.ini *and* that the /tmp/foo directory does not exist
   * invoke ./firefox -no-remote -createprofile "foo /tmp/foo"

2) * Type "mkdir /tmp/foo"

When using option 1), bookmarks.html is copied in the profile, and the default bookmarks are created.

Up to now, I usually used option 2) above, as it is less to type and you do not need to care if the profile/directory is already there. However, I don't get the default bookmarks in that case.

Option 1) above should be close to what happens when you create a profile using the Profile Manager or when you start Firefox with no profile directory (like the first time you install it).

So the question would be to decide if option 2) is supported or not. If it is not, this bug should be WONTFIX or INVALID.
Comment 5 Ria Klaassen (not reading all bugmail) 2007-12-23 16:23:01 PST
This has been broken more times in the past, but was always fixed.
The last time it regressed between the nightlies 2006-12-18 04 and 2006-12-19 04 downloaded at http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/experimental/places/

I think most people who are creating a new profile with the -profile switch, don't need the default set but use their own bookmarks instead. But it is a regression because it used to work.
Comment 6 Sylvain Pasche 2007-12-23 16:37:28 PST
I don't see commits related to places in this timeframe: http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-12-18+04&maxdate=2006-12-19+04&cvsroot=%2Fcvsroot

I guess these builds where made from a branch? Do you know which one?
Comment 7 Ria Klaassen (not reading all bugmail) 2008-01-07 06:49:56 PST
(In reply to comment #6)
> I don't see commits related to places in this timeframe:
> http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-12-18+04&maxdate=2006-12-19+04&cvsroot=%2Fcvsroot
> 
> I guess these builds where made from a branch? Do you know which one?
>
No, but the people working on Places those days will know it. 

Comment 8 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2008-01-07 06:54:47 PST
The problem here is almost certainly that places is somewhere relying on the "new profile" code to copy files from the default profile to the new profile. That is incorrect behavior: it should be initializing the new profile from code. The old code to do this was at nsBrowserDirectoryProvider::EnsureProfileFile but was removed in bug 386392:

http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&file=nsBrowserDirectoryProvider.cpp&branch=&root=/cvsroot&subdir=/mozilla/browser/components/dirprovider&command=DIFF_FRAMESET&rev1=1.17&rev2=1.18

What I'd really like to do is stop copying the profile default files in any circumstance and require the code to get it right... I'll attach a patch to that effect momentarily.
Comment 9 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2008-01-07 07:07:33 PST
Created attachment 295771 [details] [diff] [review]
Don't copy default profile files on initial profile creation, rev. 1

We obviously can't land this until the places bug is fixed, but I've been meaning to do this for a while.
Comment 10 Henrik Skupin (:whimboo) 2008-01-07 10:06:30 PST
*** Bug 411114 has been marked as a duplicate of this bug. ***
Comment 11 Henrik Skupin (:whimboo) 2008-01-07 10:11:42 PST
This is not due to the -profile switch. It happens for each new profile which is not created on the default location, like you see in comment 3.

Benjamin, which places bug you are referring to?
Comment 12 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2008-01-07 10:59:03 PST
This one... it's primarily a places bug that the default bookmarks are not populated correctly.
Comment 13 Dietrich Ayala (:dietrich) 2008-01-08 21:20:28 PST
Created attachment 296088 [details] [diff] [review]
v2
Comment 14 u88484 2008-01-09 05:01:48 PST
Is this bug's summary still completly accurate?  Readin Comment #8 it doesn't appear so.  I am wondering because I created a new profile last night (in default location) using the profile manager and got no bookmarks, blank bookmark toolbar and no smart tags...which seems along the lines of this bug but not sure.
Comment 15 Tracy Walker [:tracy] 2008-01-09 07:44:23 PST
Kurt the builds from Jan 8 will exhibit bug 411353.  Try again with a build from Jan. 9th.
Comment 16 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2008-01-09 07:55:53 PST
Comment on attachment 296088 [details] [diff] [review]
v2

>Index: browser/components/nsBrowserGlue.js

Did you try this? I would expect this not to work correctly:

>+    if (!bookmarksFile.exists()) {
>+      // if bookmarks file does not exist, import default bookmarks
>+      var bookmarksFileName = "bookmarks.html";
>+      bookmarksFile = dirService.get("profDef", Ci.nsILocalFile);

bookmarksFile = "C:\Program Files\Mozilla Firefox\defaults\profile"

>+      bookmarksFile.setLeafName(bookmarksFileName);

bookmarksFile = "C:\Program Files\Mozilla Firefox\defaults\bookmarks.html" (incorrect)

I think you want bookmarksFile.append(bookmarksFileName);

r=me with that change or an explanation what I'm missing
Comment 17 Dietrich Ayala (:dietrich) 2008-01-09 17:51:46 PST
Created attachment 296258 [details] [diff] [review]
v3

For check-in.

No, previous patch doesn't work. I hadn't built in /toolkit/src, so that change wasn't applied when I tested.
Comment 18 Dietrich Ayala (:dietrich) 2008-01-09 17:54:44 PST
Comment on attachment 296258 [details] [diff] [review]
v3

drivers: regression fix

This shouldn't cause a Ts regression, as it will only affect the first run, when the profile is created, which is thrown out by the test.
Comment 19 Mike Beltzner [:beltzner, not reading bugmail] 2008-01-09 20:02:38 PST
Comment on attachment 296258 [details] [diff] [review]
v3

a=beltzner for 1.9
Comment 20 Dietrich Ayala (:dietrich) 2008-01-10 11:09:21 PST
Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.47; previous revision: 1.46
done
Checking in toolkit/profile/src/nsToolkitProfileService.cpp;
/cvsroot/mozilla/toolkit/profile/src/nsToolkitProfileService.cpp,v  <--  nsToolkitProfileService.cpp
new revision: 1.21; previous revision: 1.20
done
Comment 21 Dietrich Ayala (:dietrich) 2008-01-10 11:56:05 PST
looking like this might have regressed Ts. i'll keep an eye on it.
Comment 22 Henrik Skupin (:whimboo) 2008-01-10 15:09:26 PST
Now it's fine when creating profiles on another locations. The default bookmarks are present. VERIFIED with Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b3pre) Gecko/2008011021 Minefield/3.0b3pre

I don't mark it verified yet due to comment 21.
Comment 23 Dietrich Ayala (:dietrich) 2008-01-10 15:38:38 PST
Reverting due to Ts and Txul regressions, as well as other potential issues:

http://forums.mozillazine.org/viewtopic.php?p=3214293#3214293

Checking in browser/components/nsBrowserGlue.js;
/cvsroot/mozilla/browser/components/nsBrowserGlue.js,v  <--  nsBrowserGlue.js
new revision: 1.48; previous revision: 1.47
done
Checking in toolkit/profile/src/nsToolkitProfileService.cpp;
/cvsroot/mozilla/toolkit/profile/src/nsToolkitProfileService.cpp,v  <--  nsToolkitProfileService.cpp
new revision: 1.22; previous revision: 1.21
done
Comment 24 Peter van der Woude [:Peter6] 2008-01-10 15:49:30 PST
It looks like I lost my bookmarks after this landed.
Suddenly I only had the default set after the startup of an hourly build.
Reconstuction by importing my bookmarks.html was the only way to fix it.
Comment 25 Ria Klaassen (not reading all bugmail) 2008-01-26 02:25:21 PST
At the moment when I make a new profile with the -profile switch, not only the default set but also the whole Places / Smart Bookmarks folder is missing. It is present if I start it up after creating the profile with a branch build. This started to happen a few months ago and I assume this issue will be fixed with this bug (?).
Comment 26 Axel Hecht [:Pike] 2008-01-29 16:36:07 PST
Dietrich, any ETA on this one? I guess it could use a priority, too.
Comment 27 cmtalbert 2008-01-30 15:58:58 PST
This patch (v3) changes behavior for bug 414056, makes and makes the overall experience of that bug worse.  I'm not exactly sure how to codify this relationship in bugzilla.
Comment 28 XtC4UaLL [:xtc4uall] 2008-03-15 10:41:42 PDT
dare to ping on this ...
Comment 29 Ria Klaassen (not reading all bugmail) 2008-03-23 07:28:06 PDT
*** Bug 424645 has been marked as a duplicate of this bug. ***
Comment 30 Mike Connor [:mconnor] 2008-03-28 11:58:37 PDT
This isn't going to block.  Creating arbitrary profile locations is a useful feature, but deliberately not exposed to users who don't know it exists.
Comment 31 Gabriele Best [:Gabri] 2008-04-11 12:08:42 PDT
This feature is used to run Firefox on USB pendrive and by specific application programs, such as Portable firefox.

I think it should be fixed!
Comment 32 Gabriele Best [:Gabri] 2008-04-12 14:02:11 PDT
Created attachment 315300 [details] [diff] [review]
patch v4

It should works perfectly
Comment 33 Gabriele Best [:Gabri] 2008-04-13 00:36:38 PDT
Created attachment 315346 [details] [diff] [review]
patch v4.1

I'm sorry, yesterday I've posted a bugged patch :P
This works.
Comment 34 Gabriele Best [:Gabri] 2008-04-16 13:42:10 PDT
Can't someone check/review the last patch?
thanks.
Comment 35 u88484 2008-04-16 13:57:29 PDT
A dev requested another dev to review it so hold your horses and it will get reviewed when they have a chance.
Comment 36 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-01 11:49:51 PDT
Created attachment 318861 [details] [diff] [review]
better patch
Comment 37 Dietrich Ayala (:dietrich) 2008-05-01 12:06:13 PDT
Comment on attachment 318861 [details] [diff] [review]
better patch

r=me, thanks. 

(sorry for the delay mrtb)
Comment 38 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-01 12:08:46 PDT
Comment on attachment 318861 [details] [diff] [review]
better patch

Very low risk patch to fall back to the defaults bookmarks file for first-run import if the profile doesn't already have one, helps portable firefox.
Comment 39 Gabriele Best [:Gabri] 2008-05-01 13:16:09 PDT
It works perfectly.
But, I don't know if it is a bug, profile folder hasn't bookmarks.html file.

May it cause problems?
Comment 40 Mike Beltzner [:beltzner, not reading bugmail] 2008-05-01 13:20:14 PDT
Comment on attachment 318861 [details] [diff] [review]
better patch

a1.9=beltzner
Comment 41 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-01 14:11:30 PDT
(In reply to comment #39)
> It works perfectly.
> But, I don't know if it is a bug, profile folder hasn't bookmarks.html file.
> 
> May it cause problems?

That is expected, and it shouldn't cause any problems. This code will always fall back to the "default" bookmarks.html if the profile one doesn't exist.
Comment 42 :Gavin Sharp [email: gavin@gavinsharp.com] 2008-05-01 15:03:15 PDT
mozilla/browser/components/nsBrowserGlue.js 	1.91 
Comment 43 Dietrich Ayala (:dietrich) 2008-05-01 17:51:49 PDT
Created attachment 318943 [details] [diff] [review]
test fix

updated the test for 425884, to fix test failure from this check-in. just moved the tests into a separate container, for more tolerance for change in the default bookmarks data.

Checking in browser/components/places/tests/browser/browser_425884.js;
/cvsroot/mozilla/browser/components/places/tests/browser/browser_425884.js,v  <--  browser_425884.js
new revision: 1.2; previous revision: 1.1
done
Comment 44 Henrik Skupin (:whimboo) 2008-05-03 01:10:21 PDT
Verified with Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9pre) Gecko/2008050206 Minefield/3.0pre ID:2008050206

> Steps to reproduce:
> 1. mkdir c:\tmp\foo
> 2. firefox.exe -CreateProfile 'test c:\tmp\foo'

Clint, running these commands you have given in comment 3 does nothing for me. Firefox doesn't create a new profile on that location and shutdown immediately. Is this another already known bug?

Should this be included into the test-suite?
Comment 45 Gervase Markham [:gerv] 2009-11-26 06:46:04 PST
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

Note You need to log in before you can comment on or make changes to this bug.