Closed
Bug 467766
Opened 16 years ago
Closed 2 years ago
user settings for pref keys with defaults in extension get reset on upgrade
Categories
(Core :: Preferences: Backend, defect)
Tracking
()
RESOLVED
INCOMPLETE
People
(Reporter: asac, Unassigned)
References
Details
Attachments
(1 file)
4.00 KB,
patch
|
benjamin
:
review-
|
Details | Diff | Splinter Review |
This bug is due to the fact, applications are restarted before extension defaults are loaded. To reproduce, choose any preference and set the values like: system default: pref("prefkey",systemvalue); extension default: pref("prefkey", extensiondefault); user pref: user_pref("prefkey", systemvalue); Next, trigger application behaviour similar to upgrade by removing compreg.dat from profile and start the application. Result: User sees extensiondefault after upgrade, because the user_pref has been eliminated ... which is definitly not what the user expects because he explicitly had *systemvalue* set before the upgrade. Evaluation: The bug happens because restart is performed *before* extension defaults have been loaded and the prefapi.cpp always eliminate user preference if the user preference is equal to the actual default (which happens to be extensiondefault normally - so no reset, but is systemvalue during restart). Proposed Fix: 1. savePrefs should not try to be smart ... this patch removes the heuristic that guesses whether a setting can be eliminated or not; it should be sufficient to only eliminate prefs in hashPrefs. 2. This patch prevents hashPrefs from eliminating the user pref in case we are in *startup* ... unfortunately no such state info exists, which lets us guess that we are in startup for the previously not dealt case: !set_default && !pref_ValueChanged(pref->defaultPref, value, type) && !PREF_HAS_USER_VALUE(pref). If this is the case we explicitly remember that this setting is a user-pref ... even though it might be temporarily equal to the default pref.
Reporter | ||
Comment 1•16 years ago
|
||
ubuntu xulrunner-1.9.1 patch.
Assignee: nobody → asac
Status: NEW → ASSIGNED
Updated•16 years ago
|
Attachment #351173 -
Flags: review?(benjamin)
Comment 2•16 years ago
|
||
Comment on attachment 351173 [details] [diff] [review] for 1.9.1 also punting this to bsmedberg (keeping a running tally, now up to 3)
so, this will fail for this case: system: pref = 1 ext_a1: pref = 2 user: pref = 1 user upgrades and ext_a1 is incompatible w/ firefox system: pref = 1 user: pref = 1 user quits. pref is lost. later, user runs the browser and installs ext_a2 (upgraded extension ext_a version 2, which is compatible w/ the upgrade) and gets: ext_a2: pref = 2 I think to handle this case, you'd need to scan All extensions, enabled and disabled. for any pref which exists in multiple locations set a flag "don't wipe". and when you quit, only wipe prefs if they aren't tagged "don't wipe". it's probably also ok to maintain a file in the profile dir listing prefs which shouldn't be wiped, they can use the same logic, it should be append only, that'd save you the trouble of scanning disabled things.
Reporter | ||
Comment 4•16 years ago
|
||
I wonder if we really want this automagic reset of user prefs at shutdown at all ... why not keep them as user prefs unless you explicitly say "reset" (either in about:config or through API) ... for me it seems that if a user ever explicitly selected a specific pref you hardly ever want to go back to ride the default again. Thoughts?
I've wanted to remove the feature for 8+ years. I think we should probably remove the feature, and have someone write an extension similar to regclean/regmaid http://support.microsoft.com/default.aspx/kb/299958 http://support.microsoft.com/kb/156078 our extension should basically tell users when a preference name/value became obsolete and let them choose to remove it. RegMaid is the advanced version of RegClean - RegClean or a mozilla equivalent would merely create a backup of the removed prefs and remove all prefs which don't make sense to the current run. It should warn the user about any prefs which are for extensions which aren't enabled.
Reporter | ||
Comment 6•16 years ago
|
||
(In reply to comment #5) > I've wanted to remove the feature for 8+ years. > Do you have any references for previous discussions? I would like to read the arguments before actually complaining ;).
finding them would take more effort than i'm willing to expend, they could be in irc logs (which would entail finding someone w/ logs from the right period), or one of a number of mailboxes (this should cover bugzilla), or newsgroups. one instance for this was switching a profile between netscape 6 and mozilla where they differed on a pref value.... but the problem is older so i'm not sure when it would have been discussed. alecf might remember it, although i'm sure he'd rather forget, possibly bnesse....
Updated•15 years ago
|
QA Contact: preferences → preferences-backend
Updated•15 years ago
|
Attachment #351173 -
Flags: review?(benjamin) → review-
Comment 9•15 years ago
|
||
Comment on attachment 351173 [details] [diff] [review] for 1.9.1 I cannot accept this without a test (probably several tests, given its nature)
Updated•15 years ago
|
See Also: → https://launchpad.net/bugs/548866
Comment 10•13 years ago
|
||
Also note that saving a user pref when it is equal to the default pref breaks with Firefox Sync and complex prefs (see https://launchpad.net/bugs/643899)
Comment 11•2 years ago
|
||
The bug assignee didn't login in Bugzilla in the last 7 months.
:KrisWright, could you have a look please?
For more information, please visit auto_nag documentation.
Assignee: asac → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(kwright)
Comment 12•2 years ago
|
||
I don't believe this is an issue anymore, and the affected code has been moved and changed so I'm not sure if this is simply not reproducible on my end. I'm going to resolve this with the intent to reopen with new references if someone does reproduce it.
Status: NEW → RESOLVED
Closed: 2 years ago
Flags: needinfo?(kwright)
Resolution: --- → INCOMPLETE
You need to log in
before you can comment on or make changes to this bug.
Description
•