Closed Bug 399460 Opened 17 years ago Closed 17 years ago

if browser crashes or is killed before we write out prefs, you can get two Places folders on the toolbar

Categories

(Firefox :: Bookmarks & History, defect)

x86
All
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 3 beta1

People

(Reporter: ria.klaassen, Assigned: moco)

References

Details

Attachments

(2 files, 1 obsolete file)

Attached image screenshot (deleted) —
Steps to Reproduce: - Make a new profile with a branch build - Run it with a trunk build (you'll see one Places folder) - Paste a recent places.sqlite file in the new profile (or delete everything except places.sqlite) - Start Minefield Result: you'll see two functional Places folders
creating the "Places" folder is controlled by a hidden pref: browser.places.createdDefaultQueries if you were to run with a build with the fix for bug #387996, delete your prefs and run again, you'd get a second Places folder, as browser.places.createdDefaultQueries would not be true. I'm not sure this bug is valid.
(In reply to comment #1) > creating the "Places" folder is controlled by a hidden pref: > browser.places.createdDefaultQueries > > if you were to run with a build with the fix for bug #387996, delete your prefs > and run again, you'd get a second Places folder, as > browser.places.createdDefaultQueries would not be true. > > I'm not sure this bug is valid. > Ok, makes sense :). Resolving INVALID.
Status: NEW → RESOLVED
Closed: 17 years ago
Resolution: --- → INVALID
Reopening, this happened to me (and cbarrett). I've just been using my normal profile. Yesterday I saw the Places folder added, and then it happened again today. All my normal bookmarks and settings are still in place. I've switched back and forth with my own builds, was there a window where code might have added the folder but not set the pref?
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
Could you perhaps have crashed the first time it added the folder, such that it stayed was present in your Places DB, but the pref indicating that it had been added hadn't been saved?
I now have three Places folders in my toolbar. :( Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a9pre) Gecko/2007101104 Minefield/3.0a9pre
OS: Windows XP → All
as gavin points out, we could create mutliple places folder if you quit or crashed before prefs were flushed to disk. see bug #399476 comment #4 dolske / cbarrett / reed: could you be launching firefox in a way where we don't look at the user prefs or we clear them? (like safe mode)
Assignee: nobody → sspitzer
Status: REOPENED → NEW
gavin / mano / dietrich: I'm going to add the call to savePrefFile (and switch to use the existing gPrefService in browser.js (which I should have done originally, instead of using doing prefBranch). as we have in http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#1374, it will: 1374 // We need to persist this preference change, since we want to 1375 // check it at next app start even if the browser exits abruptly flushing prefs to disk will make the first run of Ts slower, but the subsequent Ts runs faster (as killing firefox won't cause us to do this work each time.) note, for real world, the user will only hit the savePrefFile() call that I plan on adding once, when the pref is not set.
Status: NEW → ASSIGNED
Attached patch updated to trunk (deleted) — Splinter Review
updated to trunk.
Attachment #284702 - Attachment is obsolete: true
Attachment #284705 - Flags: review?(mano)
Attachment #284702 - Flags: review?(dietrich)
Flags: blocking-firefox3?
Summary: Two Places folders on my toolbar → if browser crashes or is killed before we write out prefs, you can get two Places folders on the toolbar
Target Milestone: --- → Firefox 3 M9
Comment on attachment 284705 [details] [diff] [review] updated to trunk r=mano
Attachment #284705 - Flags: review?(mano) → review+
Comment on attachment 284705 [details] [diff] [review] updated to trunk seeking approval
Attachment #284705 - Flags: approval1.9?
Attachment #284705 - Flags: approval1.9? → approval1.9+
Flags: blocking-firefox3? → blocking-firefox3+
fixed. Checking in browser.js; /cvsroot/mozilla/browser/base/content/browser.js,v <-- browser.js new revision: 1.870; previous revision: 1.869 done
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
(In reply to comment #6) > dolske / cbarrett / reed: could you be launching firefox in a way where we > don't look at the user prefs or we clear them? (like safe mode) I normally only start Minefield by clicking it in the OS X dock, or by running it from my objdir. I don't specifically remember it crashing on startup, but who knows.
Gee, right at the moment using my default profile I see two Places folders. Maybe because of the recent checkin in bug 387996 someway?
(In reply to comment #14) > Gee, right at the moment using my default profile I see two Places folders. > Maybe because of the recent checkin in bug 387996 someway? > Build ID = 2007101223
Hmm. I got two Places folders as I updated to: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a9pre) Gecko/2007101304 Minefield/3.0a9pre ID:2007101304
ria, once two "Places" folders got created, you'd be stuck with the second one.
I'm pretty dubious that this doubling up is caused by crashing before saving the prefs file. Since the initial checkin I've been running with just one places folder just fine. Only today a second one sprung up.
So, upgrading to today's nightly brings yet another Places folder to my bookmarks toolbar. That makes #4 so far. :/
Reopening this as yet another places folder has appeared on my toolbar. Until earlier today I had not crashed since 4th so the pref was most definitely flushed to disk.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
reed / dave, could something else (like sync extension) be clearing (or setting it to false) browser.places.createdDefaultQueries for you?
Status: REOPENED → ASSIGNED
thanks to hint from sayre (who told me that he is frequently switching between builds), I know what's causing this. when this feature first landed, there was no default value for browser.places.createdDefaultQueries in firefox.js. for a certain window of time, to back it out due to Ts and Txul regressions, I defaulted browser.places.createdDefaultQueries to true in firefox.js now that it's back on, the default value is false in firefox.js. But, here's the problem: 1) run a build with the feature enabled, create the "Places" folder and we set browser.places.createdDefaultQueries to true 2) run a build when the feature was disabled. the pref has a default value (true) which matches the user value (true), so when we write out the prefs.js file we will not write out the user value (because it matches the default, see http://lxr.mozilla.org/seamonkey/source/modules/libpref/src/prefapi.cpp#334). now in your prefs.js, the browser.places.createdDefaultQueries is not set. 3) run a build where the feature was enabled. the pref has a default value of false, so we'll recreate the places folder. I've just verified that each time I go back, run the build where the feature was disabled, and then run a build where it is enabled, I create a new "Places" folder. Unless there is another scenario, I don't think this blocks m9.
Summary: if browser crashes or is killed before we write out prefs, you can get two Places folders on the toolbar → if browser crashes or is killed before we write out prefs (or if you switch between certain builds), you can get two Places folders on the toolbar
the window of builds that you don't want to run is between: 2007-10-11 13:30 and 2007-10-12 20:45
(In reply to comment #22) Hm, how hard would it be to ensure that, regardless of the pref setting, if there already is a Places folder we don't create another one but reuse the existing one? Or are there circumstances where we absolutely _want_ to create a second Places folder?
re-marking original bug as fixed, based on my comment #22. as for what happens if you run a build built betwee between 2007-10-11 13:30 and 2007-10-12 20:45 and the run a newer build, that's a separate bug that I'll log (but mark as wontfix, as I don't think it's worth fixing.)
Status: ASSIGNED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Summary: if browser crashes or is killed before we write out prefs (or if you switch between certain builds), you can get two Places folders on the toolbar → if browser crashes or is killed before we write out prefs, you can get two Places folders on the toolbar
> that's a separate bug that I'll log (but mark as wontfix see bug #400252
Ok this makes sense. And I don't think there is much value in going to additional lengths to avoid it, we impact us nightly testers in a fairly minor fashion.
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
Component: Places → Bookmarks & History
QA Contact: places → bookmarks
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: