From Bugzilla Helper: User-Agent: Mozilla/5.0 (Windows; N; NT; en-US) Mozilla/m13 BuildID: 2000022820 I'd like to, and I think it would be very useful, to change the location of the bookmark file to a file other than the profile directory. eg, UNC, web page, other browser's bookmark. I know I can do it in NS, I looked in moz's prefs.js and the bookmark location is not stored in there - I don't think it dhould be hard coded. I may be wrong about all this, could someone plz check it out? thx Reproducible: Always Steps to Reproduce: 1. Try to define your own location for the bookmark file 2. 3.

Confirming; at least, I can't see how to do it. And there should be UI. Gerv

Putting on [nsbeta2-] radar. Not critical to beta2.

Move to M20 target milestone.

Nominating for nsbeta3. NS 6 cannot ship without this - it's 4xp, for a start. Gerv

reporter or Gerv, could somebody please clarify here what exactly the desired feature is? What would be the steps in 4.x? I seem to be missing something here and I just don't get it.

The bookmarks file for a given profile is (under Mozilla) stored in Users50\profilename\bookmarks.html (or similar). There is a way in NS 4.x (Save Bookmarks As...) which allows you to change the path where your bookmarks for your profile are stored - if you share them between Windows and Linux versions of Navigator, or store them somewhere network-accessible, for example. A simple pref to set the bookmarks file path would do the trick. It would be nice to have UI, but not essential :-) Gerv

I see now, thanks. It's definitely 4xp, but may I suggest adding some more justification if you truly believe this to be nsbeta2 or even 3 and not just an RFE.

The justification is that there are many people who want to store their bookmarks somewhere else - to back them up, to share them with another install, or whatever. It's a very useful feature for a variety of users, and it's 4xp, and so it should be implemented :-) Also, it probably wouldn't be too hard - when you load and save bookmarks, replace the hard-coded relative URL with a call to find a pref. Gerv

re: gerv's notes, definitely. it's the single barrier preventing me from migrating to mozilla. I can't rely on mozilla yet, so I'd like to use both, mozilla for non-time critical browsing, and use netscape when I can't afford a crash or other thing stuffing me up. the problem is bookmarks - with a 150kb bookmark file (those are just the ones I haven't memorised) and links to all my important sites for work, I need to seamlessly access my store of bookmarks from both browsers. I can't speak for anyone else, but I know I find this a notable omission. Others would encounter problems in network installs, shared bookmark environments, remote bookmark storage, etc. Just adds flexibility to a browser that needs all the flexibility it can get to thrash MSIE.. and besides, NS and (i think) MSIE have variables storing the bookmark path, so there :) thx!

Nav triage team: [nsbeta3-]; changed summary to be more accurate

Reassigning 79 Bookmarks bugs to Ben. I was told this was going to be done shortly about two months ago, but it clearly hasn't been. I think that's long enough for all these bugs to remain assigned to nobody. Feel free to filter all this spam into the trashcan by looking for this string in the message body: ducksgoquack

Nominating for mozilla1.0. This is very useful for people who run more than one OS. Gerv

nav triage team: not a beta stopper, though we wouldn't mind a patch from the net ;-)

You what? I am now _highly_ confused. nsbeta_1_? Gerv

adding helpwanted keyword, cleaning up status whiteboard.

First go at fixing this one could be to add "bookmarks.html"s location to the prefs.js file. This way to mozilla profiles could share bookmarks.

*** Bug 70390 has been marked as a duplicate of this bug. ***

Marking nsbeta1- bugs as future to get off the radar.

I could probably do this in several hacky ways - but then my patch would get rejected. Is there any chance of someone familiar with the bookmarks code giving me some idea of at what point in retrieving the bookmarks filename this should be implemented? We are in the area of http://lxr.mozilla.org/mozilla/source/xpfe/components/bookmarks/src/nsBookmarksS ervice.cpp#3971 waterson: seems you originally wrote this code. Any chance of a tip? Gerv

Actually, I wonder if it is possible to extend this functionality so that bookmarks can be saved to two separate file at the same time. The reason for this is that if you open bookmarks in NS4.x, and then close NS, the file structure is preserved, but the ShortCutURL="?" tags are all lost. I think it is important to be able to specify a shared bookmark file, but to effectively lose the new shortcut functionality would be too high a price to pay (better to just have better export functionality). Simpler, perhaps two bookmark menu items "Export(default)" and "Export..." would be the answer.

Excuse me? export has nothing to do with this bug, if you have concerns about it please file a new bug. 3978 nsBookmarksService::GetBookmarksFile(nsFileSpec* aResult) change it so that it gets a preference (preferably the same one nc4 used) and then loads that file (w/ a fallback to the current behavior)

Timeless: the problem with that is that if there is ever UI for this, it makes it difficult to display the "default" bookmarks location just by getting the pref, and also difficult to unset the pref if the user puts the bookmarks back in the default place. Or not? Gerv

Ok, people - user-defined bookmarks location is a reality! This is definitely a much-requested feature. Patch attached. It uses the old 4.x pref of browser.bookmarks_file for max. compatibility when migrating prefs. Clearing nsbeta1- nomination in view of this - hoping that, if you don't have resource to do the patch, you have resource to review it and check it in :-) Looking for r= and sr=. Gerv

By the way, home@ant-roy.co.uk does have a point - if what he says is true, sharing bookmarks with NS 4.x will be incompatible with any new bookmarks features - most notably, the Personal Keywords feature - because NS 4.x will eat the attributes. Gerv

For now we could include a warning in profile migration, but that's beyond the scope of this bug, as is the fact that the code in bookmarks is using char* instead of something else. (i'll test the code and give you an r= instead of complaining about preexisting style).

r=timeless

+ if (NS_SUCCEEDED(rv) && (prefServ)) { + char *prefVal = nsnull; + + if (NS_SUCCEEDED(rv = prefServ->CopyCharPref("browser.bookmark_file", &prefVal)) && + prefVal) { + *aResult = (const char *)prefVal; + } + else { + // Pref is not present + rv = NS_ERROR_FAILURE; + } + } You could replace that with: + if (NS_SUCCEEDED(rv)) { + nsXPIDLCString prefVal; + rv = prefServ->CopyCharPref("browser.bookmark_file", + getter_Copies(prefVal)); + if (NS_SUCCEEDED(rv)) { + *aResult = prefVal; + } + } This makes setting a breakpoint on the if (but after the CopyCharPref) easier. In the original code you were leaking the char* prefVal. CopyCharPref hands you a char* which you own and need to free. nsXPIDLCString and getter_Copies take care of that for you. This makes the assumption that neither prefServ nor prefVal will ever be nsnull if rv indicates success. I think it's a fair assumption, some people don't. If you want to test for nsnull, make sure you set rv to NS_ERROR_FAILURE in both the else cases (you cover only one in your patch). However, that throws away the rv value if !NS_SUCCEEDED(rv) so you should really split up that if and only set rv to NS_ERROR_FAILURE on the elses of |if (prefServ)| and |if (prefVal)| + if(NS_FAILED(rv)) { Nit: keep the same space-after-if style as for: if (NS_SUCCEEDED(rv) && ... ) (Or remove the space there too, just be consistent...) + // or nsILocalFile, this conversion from nsiFile to Minor case nit + nsXPIDLCString pathBuf; + rv = bookmarksFile->GetPath(getter_Copies(pathBuf)); + if (NS_SUCCEEDED(rv)) + *aResult = (const char *)pathBuf; I think you can drop the (const char *) here... See above :-) If not, use: *aResult = pathBuf.get(); You seem to be mixing 2 and 4 space indent. And the file is mostly 8 space, as also confirmed by the emacs mode line. Ugh! If Ben doesn't mind you owning that code, could you fix that to all be 8 space (or with his permission, all 4 or 2 space)?

We should remove the nsFileSpec usage. This makes me want to throw up: + if (NS_SUCCEEDED(rv)) { + nsXPIDLCString prefVal; + rv = prefServ->CopyCharPref("browser.bookmark_file", + getter_Copies(prefVal)); + if (NS_SUCCEEDED(rv)) { + *aResult = prefVal; Surprise! There's an *assignment* operator that constructs an nsFileSpec from a const char*! This is just way too clever. (N.B., I'm not criticizing the author of the patch: you did what you had to do. The nsFileSpec APIs are what blow choad.) How much extra work would it be to replace this stuff with nsIFile and friends?

*LOL* I ranted about that in #mozilla to Gerv too. I'll look into what needs to be done to move this all to nsIFile, though I suspect it to be non-trivial...

Well, what I'd hope you'd allow is to check it in as-is (with other review changes made.) Then, when it all gets moved to nsIFile, this will go with it... Moving Bookmarks to nsIFile is a long-term job - bug 36974, it would seem. This is futured, and is blocked by the fact that Bookmarks use nsInputFileStream, which doesn't work with anything apart from nsIFileSpec. That's bug 46394 which, amusingly, is titled "necko file streams should be moved from necko to xpcom" and marked helpwanted. There has been much argument, and DougT concludes with "not going to happen soon". :-( In the mean time, this patch provides much-requested functionality ;-) Gerv

I don't think this comment is correct anymore: + // Otherwise, we look for bookmarks.html in the current profile + // directory. This is as convoluted as it seems because we + // want to 1) not break viewer (which has no profiles), and 2) + // still deal reasonably (in the short term) when no + // bookmarks.html is installed in the profile directory. We just rely on the magic directory service to find the bookmarks, right? Fix the comment, or explain why it's still appropriate, and sr=waterson. Also, find and/or file a bug on converting nsBookmarksService.cpp to using nsIFile instead of nsFileSpec, and note that bug# here. thanks!

The conversion bug is bug bug 36974, blocked by (futured, helpwanted) bug 46394. The comment isn't mine :-) But I'll try and find out what it should say. Would: // Otherwise, we look for bookmarks.html in the current profile // directory using the magic directory service. do? :-) Gerv

Wonderful!

I just pulled a tree that had this checked in (there weren't any comments above, but it was checked in by timeless%mac.com at 2001-04-20 14:14 PDT). Since my profile was imported from 4.x, it had a vestigial pref from 4.x pointing to my 4.x bookmarks file (which is pretty much empty). When I pulled Mozilla with this patch, Mozilla suddenly stopped using my bookmarks and started using the ones I had in 4.x (practically none). Maybe we should name the pref something different from what it was in 4.x so this doesn't happen? I tend to think we shouldn't share a bookmarks file with 4.x by default since we have otherwise separate profiles.

Do we want to change this for 0.9? I wouldn't be surprised if we see lots of "my bookmarks disappeared" comments if we don't.

Probably not. timeless, please back this out and we'll get it in for mozilla-0.9.1.

He's away for the weekend. He asked me to sort out a backout if necessary. I'll get on IRC and do it when everyone wakes up. Is there not some sort of third solution here? The reason for having the pref name the same is so that migration Does The Right Thing, in line with all the user's other 4.x prefs. (Note that this pref will only be set if the user set up a non-default bookmarks file, which is a small minority of users.) If this pref is not going to be auto-migrated with the rest (i.e. we go out of our way to break this) we need to explain why we've honoured the rest of the user's 4.x prefs and not this one. To look at it another way, if the user's 4.x bookmarks are in their profile directory, then they get them in Mozilla. Why should they not get them if they happen to be elsewhere? Gerv

I never set up a non-default bookmarks file in 4.x. Are you sure that pref isn't set all the time, at least on the Unix version?

Gerv, please back this out for 0.9.

Not only is mozilla suddenly looking at an old set of bookmarks, but when I run 4.x and mozilla simultaneously 4.x notices that the bookmark file is being periodically written and keeps popping up dialogs stating "Bookmarks have changed on disk and are being reloaded." This should be backed out for now.

Hurricane has backed this out. I noticed the 4.x messages - I thought that a) It won't be seen by many users as very few run two browsers at once b) Why on earth are we periodically writing the bookmarks in the first place? Unless we fix Moz to be more intelligent about this (which we should) then this is a problem to be expected if you have two apps using the same file. It's not specific to this fix. <rant>Why on earth does NS 4.x not silently reload bookmarks which have changed if it hasn't changed them since it loaded them last?</rant> As regards when the pref is set, I think it is set by using the "Save As..." command in the File menu of the Bookmarks Manager. So, I suppose that if you do a Save As..., even if you save on top of your bookmarks file in the prefs, the pref will then be set. Possible solutions to this: a) Leave it as-is, because it Does The Right Thing for most people. b) Rename the pref. This means that the user who upgrades from NS4 to Mozilla and wants to continue using the same bookmarks (a reasonable request) has to edit prefs.js to set that up as we currently don't have any UI for setting it. It breaks migration. c) Some funky migration thang where it says "You are currently using file X for your NS 4 bookmarks. Do you want to keep using it?" Any other ideas? Gerv

Backed out for real this time.

How did timeless check in without driver approval? /be

I got a=asa on IRC. I don't have a log, though. I sort-of assumed a comment would appear here. The checkin comment correctly records his approval. Gerv

Ok, filed: Bugzilla Bug 77133 Why on earth are we periodically writing the bookmarks file I suggest gerv or someone write a newsgroup post asking for ideas about how to deal with multiple applications contending for the bookmarks file. Well.. whenever we do implement this feature and its migration behavior, we'll have to relnote it. Does mozilla have a first run property? netscape4 used to have one [it's a js preference setting] which we used to see if you had ever accepted the license for the current (or later) version. If we tagged mozilla preferences and checked to see if the gecko engine is newer than the specific build where this feature was added... We can prompt once (the first time a user runs with this new feature), and afterwards the bookmarks preference will be set correctly so there will be no concern. I'm sure no one likes this solution, but there it is.

Having discussed this on IRC with shaver, we may well be writing last-visited information or something like that. This is not necessarily the right thing to do, but it may be why it's happening. The problem we have here is that 4.x gives an (annoying and pointless) dialog _even_ if you haven't changed the bookmarks with 4.x. It says "Bookmarks have changed on disk and are being reloaded. [OK]." Who on earth thought of that? Therefore, there is no way of getting 4.x to play nice with simultaneous bookmark file sharing. However, this is not the point of this bug, really. Surely the most common use case is for people to run 2 Mozillas on different platforms, or Mozilla at one time and NS 4.x at the other. In this case, there is no problem, and this is a useful feature. So, we could change the name of the pref. - but then that removes the convenience of the thing. <sigh> Gerv

tor, dbaron, brendan: If we change the pref name, can we still have this for 0.9? That way, users have to set it up for themselves by editing prefs.js, but at least the feature is _there_. My crystal ball tells me that some Mozilla contributors will make releases from the 0.9 branch, and this is a much-requested feature. Changing the pref name means that it won't clash and unexpected things won't happen (as dbaron and tor reported.) Gerv

Seems reasonable to do for 0.9.1 but I'm not sure whether it's worth pushing for 0.9 at this point. (Sorry for not responding sooner.)

So, what are we doing about this for 0.9.1? I really want this capability to get in for that milestone in some form, even if it does involve the user editing their prefs.js (although I had a go at providing UI for it a while back - I might be able to resurrect that. However, it's a different bug.) If there are going to be a significant number of people running the Netscape 4 and Mozilla alongside one another at the same time, then using the same pref name and auto-migrating it will cause them problems, because the two apps will contend for the shared bookmark file, and we can't patch 4.x to be smart about it. If there's nothing we can do about that in a 0.9.1 timeframe, I'd rather change the name of the pref and just check the code in. Anyone got a good name? browser.bookmarks.file? Gerv

The above patch has no changes but the pref name. This patch already has r=timeless and sr=waterson (it doesn't say that explicitly here, but he did give it :-) So, it just needs checking in. Gerv

Checked in. Gerv

tested and verified

is it possible to redirect mozilla to a URL for getting it's bookmark file? I have a web based database format that can output in mozilla compatible bookmarks file and would love to have mozilla use this file as it's default bookmark file.

Have you actually tried setting the pref to a URL? Gerv

yeah, and my bookmarks menu is blank. I don't seem to get any visible error though ..

Then it's not possible :-) As I remember, when I implemented this we thought about it but doing it meant migrating the bookmarks code from nsIFileSpec to nsIFile (or the other way) so it would transparently support URLs, and that was hairy and horrible. Gerv

*** Bug 147531 has been marked as a duplicate of this bug. ***