I saw a few pref requests in bug 305004, but none to disable the automatic dated bookmark backup completely. I'm sure this won't be a common request as most users will appreciate the feature, but I'd prefer to handle this myself and without a pref you'd have to edit the source or screw with the permissions on the bookmarkbackups/ directory or something. :P

Maybe something like this? about:config -> browser.bookmarks.dated_backup.enabled -> false

I can't see us blocking the release for this enhancement but if you get a safe and fully reviewed patch, please request approval and we'll evaluate it then.

Comment on attachment 194953 [details] [diff] [review] patch v1 The indenting change accounts for most of the patch, the core of it is only a few lines, pretty simple. vlad might be busy, so I'll change review to shaver.

Comment on attachment 194953 [details] [diff] [review] patch v1 It'd be better to do an integer pref so people can keep extra/less backups, as desired, with 0 disabling the feature entirely. This also breaks the safe mode dialog to revert bookmarks in the preffed off case, which shouldn't be necessary. If you check the restore pref, and make the block you reindented |if (numberOfBackups > 0 || restoreDefaultBookmarks)| you should be fine.

The pref is now browser.bookmarks.max_backups and takes an int val. The default is 5 (same behavior as the old #define), 0 to disable. Hopefully this one is better. :P

With all due respect the old #define is 4 not 5. http://lxr.mozilla.org/seamonkey/source/browser/components/bookmarks/src/nsBookmarksService.cpp#4390

(In reply to comment #6) > With all due respect the old #define is 4 not 5. and my patch tests >=, not > ...

can I get a diff -up8 version of this, or maybe more like -up12 since its not a CVS diff and thus the patchview part of bugzilla can't give me more context.

I'm sorry for the bugspam (a little), but has this been abandoned? It's been almost two months since anything happened. A decent number of people would like this to be checked in. (Note: I have no idea if the patch is totally broken or anything. I don't have CVS set up at the moment to check. But still.)

The patch should still be good (trunk or branch), but it's never been reviewed. The last time I spoke with mconnor, it didn't sound like this had any priority. It's definitely too late for 1.5... In the meantime, this can be disabled by making the bookmarkbackups directory read-only or creating an empty file with the same name. I'm content with this as it's not a big hassle or completely aweful, but there's no joy for those looking to actually adjust the amount of backups kept.

I know, I do have it read-only. Hadn't thought about creating an empty file. I think that should've caused crashes before bug 313990 was fixed. Eh, even if it is only of enhancement severity, it still annoys me that mconnor hasn't looked at it in two months. I doubt he's really *that* busy. (Then again, it does take a while to patch, compile and test a new build, I guess...) Meh. I'll stop the bugspam now, I think. :P

I was unaware of bug 313990, but the patch should still cleanly apply with a little fuzz. An unreadable directory or a file would have cause a problem before that was fixed. Never tested, just assumed... I've actually heard WONTFIX mentioned a few times in relation to this bug, but I think that was more to the idea of simply disabling the feature. There's more value in being able to control the amount of and since a setting of 0 would disable it, everyone should be happy. The staff/drivers/reviewers have alot to do, so I wouldn't be too hard on 'em. A conversation on irc gave me the impression that he looked at this in some depth at one time or another and was more or less satisfied.

I don't want to be **** mconnor, since I'm sure he has a lot of work to do, especially now when Firefox 1.5 is a few weeks away, but I am annoyed that this has been neglected. Making bookmarkbackups read-only is a hack.

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

logan, does your patch still apply/hasn't it bitrotted? It would be nice to have this for Firefox 2.

looks ok on MOZILLA_1_8_BRANCH, some fuzz: -(~/tmp) patch -p 0 < ~/src/mozilla/307135/firefox-1.5-datedbackup-2.patch patching file mozilla/browser/components/bookmarks/src/nsBookmarksService.cpp Hunk #1 succeeded at 4331 (offset 8 lines). Hunk #2 succeeded at 4383 (offset 9 lines). Hunk #3 succeeded at 4392 (offset 9 lines). Hunk #4 succeeded at 4414 (offset 9 lines). Hunk #5 succeeded at 4596 (offset 9 lines). patching file mozilla/browser/components/bookmarks/src/nsBookmarksService.h patching file mozilla/modules/libpref/src/init/all.js

howdy y'all, [1] my tbird info ... Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.2) Gecko/20060308 Thunderbird/1.5.0.2 Mnenhy/0.7.4.666 - Build ID: 2006030803 [2] a reason for this pref i have NOT seen mentioned here is portable firefox. for usb versions of apps, every un-needed write is a real problem. both for speed reasons AND cuz flash memory has a limited number of writes before it dies. that's a _really_ good reason to have this pref. [3] one of the mozillazine threads about the portable app situation ... - Give Me My Freedom Back, Please! http://forums.mozillazine.org/viewtopic.php?t=413390 too-cutesy title, but still covers the subject. take care, lee

The portable firefox case is a good one, let's get this on trunk and land on branch after a3 ships.

Comment on attachment 195980 [details] [diff] [review] patch v2 This is obsolete now, right?

(In reply to comment #21) > (From update of attachment 195980 [details] [diff] [review] [edit]) > This is obsolete now, right? > ya, although it's the same code as the cvs diff

Landed on branch and trunk.

This bug version is currently marked Version: 1.5.0.x Branch, which seems to be its original state. Yet, from what I can tell here, it's only been fixed for 2.0 and 3.0. Can this be fixed for 1.5.0.5 as well?

It's not really something that's within the scope of a security release, in my opinion, so I really doubt it.

From what I've read in various Linux forums, it seems a lot of non-security things have gone in between 1.5 and 1.5.0.4. Seems like this might qualify for security related anyway, having impact on disk space and backup procedures, and maybe causing hang or crash when profiles are located where disk space is particularly precious.

why landed on Trunk ? no bookmark-backup on trunk. Where/which is bookmark-backup file on trunk ?

(In reply to comment #28) > why landed on Trunk ? bookmarks.html and bookmarkbackups are used if places is disabled