381365 - No default bookmarks for profiles with a non-relative profile location

Status: VERIFIED FIXED

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

Description

[Ria Klaassen (not reading all bugmail)]

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)]

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)]

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]

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]

(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)]

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]

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)]

(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]

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]

Attachment: 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 11

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

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]

This one... it's primarily a places bug that the default bookmarks are not populated correctly.

Comment 13

[Dietrich Ayala (:dietrich)]

Attachment: v2

Comment 14

[u88484]

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]]

Kurt the builds from Jan 8 will exhibit bug 411353.  Try again with a build from Jan. 9th.

Comment 16

[Benjamin Smedberg]

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)]

Attachment: 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)]

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]]

Comment on attachment 296258 [details] [diff] [review]
v3

a=beltzner for 1.9

Comment 20

[Dietrich Ayala (:dietrich)]

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)]

looking like this might have regressed Ts. i'll keep an eye on it.

Comment 22

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

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)]

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]]

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)]

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]]

Dietrich, any ETA on this one? I guess it could use a priority, too.

Comment 27

[cmtalbert]

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

[(mostly gone) XtC4UaLL [:xtc4uall]]

dare to ping on this ...

Comment 30

[Mike Connor [:mconnor]]

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]]

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]]

Attachment: patch v4

It should works perfectly

Comment 33

[Gabriele Best [:Gabri]]

Attachment: patch v4.1

I'm sorry, yesterday I've posted a bugged patch :P
This works.

Comment 34

[Gabriele Best [:Gabri]]

Can't someone check/review the last patch?
thanks.

Comment 35

[u88484]

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: [e-mail]]]

Attachment: better patch

Comment 37

[Dietrich Ayala (:dietrich)]

Comment on attachment 318861 [details] [diff] [review]
better patch

r=me, thanks. 

(sorry for the delay mrtb)

Comment 38

[:Gavin Sharp [email: [e-mail]]]

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]]

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]]

Comment on attachment 318861 [details] [diff] [review]
better patch

a1.9=beltzner

Comment 41

[:Gavin Sharp [email: [e-mail]]]

(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: [e-mail]]]

mozilla/browser/components/nsBrowserGlue.js 	1.91 

Comment 43

[Dietrich Ayala (:dietrich)]

Attachment: 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][⌚️UTC+1]]

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]]

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