467766 - user settings for pref keys with defaults in extension get reset on upgrade

Status: RESOLVED INCOMPLETE

(Core :: Preferences: Backend, defect)

Description

[Alexander Sack]

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.

Comment 1

[Alexander Sack]

Attachment: for 1.9.1

ubuntu xulrunner-1.9.1 patch.

Comment 2

[Mike Connor [:mconnor]]

Comment on attachment 351173 [details] [diff] [review]
for 1.9.1

also punting this to bsmedberg (keeping a running tally, now up to 3)

Comment 3

[timeless]

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.

Comment 4

[Alexander Sack]

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?

Comment 5

[timeless]

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.

Comment 6

[Alexander Sack]

(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 ;).

Comment 7

[timeless]

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....

Comment 8

[timeless]

bug 58326 is perhaps one of the older bugs on the subject.

Comment 9

[Benjamin Smedberg]

Comment on attachment 351173 [details] [diff] [review]
for 1.9.1

I cannot accept this without a test (probably several tests, given its nature)

Comment 10

[Chris Coulson]

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

[BugBot [:suhaib / :marco/ :calixte]]

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.

Comment 12

[Kris Wright :KrisWright]

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.