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)
Tracking
()
RESOLVED
FIXED
Firefox 3 beta1
People
(Reporter: ria.klaassen, Assigned: moco)
References
Details
Attachments
(2 files, 1 obsolete file)
(deleted),
image/png
|
Details | |
(deleted),
patch
|
asaf
:
review+
mconnor
:
approval1.9+
|
Details | Diff | Splinter Review |
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
Assignee | ||
Comment 1•17 years ago
|
||
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.
Reporter | ||
Comment 2•17 years ago
|
||
(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
Comment 3•17 years ago
|
||
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 → ---
Comment 4•17 years ago
|
||
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?
Comment 5•17 years ago
|
||
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
Assignee | ||
Comment 6•17 years ago
|
||
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
Assignee | ||
Comment 7•17 years ago
|
||
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
Assignee | ||
Comment 8•17 years ago
|
||
a side effect of this change is that our Ts will go down.
Attachment #284702 -
Flags: review?(dietrich)
Assignee | ||
Comment 9•17 years ago
|
||
updated to trunk.
Attachment #284702 -
Attachment is obsolete: true
Attachment #284705 -
Flags: review?(mano)
Attachment #284702 -
Flags: review?(dietrich)
Assignee | ||
Updated•17 years ago
|
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 10•17 years ago
|
||
Comment on attachment 284705 [details] [diff] [review]
updated to trunk
r=mano
Attachment #284705 -
Flags: review?(mano) → review+
Assignee | ||
Comment 11•17 years ago
|
||
Comment on attachment 284705 [details] [diff] [review]
updated to trunk
seeking approval
Attachment #284705 -
Flags: approval1.9?
Updated•17 years ago
|
Attachment #284705 -
Flags: approval1.9? → approval1.9+
Updated•17 years ago
|
Flags: blocking-firefox3? → blocking-firefox3+
Assignee | ||
Comment 12•17 years ago
|
||
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 ago → 17 years ago
Resolution: --- → FIXED
Comment 13•17 years ago
|
||
(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.
Reporter | ||
Comment 14•17 years ago
|
||
Gee, right at the moment using my default profile I see two Places folders. Maybe because of the recent checkin in bug 387996 someway?
Reporter | ||
Comment 15•17 years ago
|
||
(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
Comment 16•17 years ago
|
||
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
Assignee | ||
Comment 17•17 years ago
|
||
ria, once two "Places" folders got created, you'd be stuck with the second one.
Comment 18•17 years ago
|
||
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.
Comment 19•17 years ago
|
||
So, upgrading to today's nightly brings yet another Places folder to my bookmarks toolbar. That makes #4 so far. :/
Comment 20•17 years ago
|
||
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 → ---
Assignee | ||
Comment 21•17 years ago
|
||
reed / dave, could something else (like sync extension) be clearing (or setting it to false) browser.places.createdDefaultQueries for you?
Status: REOPENED → ASSIGNED
Assignee | ||
Comment 22•17 years ago
|
||
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.
Assignee | ||
Updated•17 years ago
|
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
Assignee | ||
Comment 23•17 years ago
|
||
the window of builds that you don't want to run is between: 2007-10-11 13:30 and 2007-10-12 20:45
Comment 24•17 years ago
|
||
(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?
Assignee | ||
Comment 25•17 years ago
|
||
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 ago → 17 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
Assignee | ||
Comment 26•17 years ago
|
||
> that's a separate bug that I'll log (but mark as wontfix
see bug #400252
Comment 27•17 years ago
|
||
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.
Comment 28•15 years ago
|
||
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.
Description
•