See bug 725714 comment 3, bug 725714 comment 3, and comments 6, 18, 19, 20 and 21. When a user has Sync enabled in Fx >= 13 and Fx <= 12 (say by upgrading one computer before another, or one computer is on ESR, etc.), Fx <= 12 will suddenly get autofill enabled. This is a problem because the old autofill is buggy and also because having autofill suddenly enable itself in old versions is unexpected and initially confusing. This is true of any preference that is synced where the default is different between 2 versions that both use Sync but this bug is just to fix this specific problem. If you want to file a bug to fix this problem in general then please do (I think I've seen examples of other preferences flipping back and forth between 2 versions, as well as between the same version on different platforms where the default value is different).

This is quite problematic for all the rapid release process, any feature (not just autofill) can hit this problem and being enabled when it should not once it gets enabled by default in Nightly. We need someone to look into this sooner than later.

Sync only syncs preferences if they are in a whitelist. And, browser.urlbar.autoFill has been in the whitelist since Sync landed in Firefox. See https://hg.mozilla.org/mozilla-central/rev/924e4f41d023#l1.41. So, the simple fix for this bug is to remove that preference from the whitelist. If that is what you want, you should probably move this bug to the url bar component because I feel that component owners know best what preferences are safe to sync. The linked bug had ideas about not syncing preferences if the default value is different on different platforms. If this is the case, you should not be syncing those preferences. If those prefs are synced today, they should be removed from the syncable prefs whitelist. I think conditionally syncing based on platform would be a great feature. How exactly we would do that, I don't know. What would be the heuristic for determining whether to apply a different pref value? Could this be done without the prefs API knowing if a pref varies between platforms? Stuff to think about if someone ever requests that feature...

Let's make this bug cover the more fundamental problem discovered on IRC: sync seems to be syncing all prefs, whether they have a user-set value or not. I filed bug 747264 to cover short-term changes to the sync whitelist.

Let's think about this for v6.

Probably not a good *first* bug, but I'd expect it to be relatively easy.

This patch is fairly simple - most of it is test changes. The prefs engine already writes null for non-existing outgoing prefs, and already does a reset of the pref when the incoming value is null. This patch also writes a null for prefs with the default value. This is true for all prefs - both the pref itself, and the "meta-pref" that tells Sync whether to sync the pref or not. One subtlety is, eg, when you have 2 firefoxes on different channels, one with this patch and one without it. In that case one of the profile will still have the default values explicitly written (ie, not reset to default) due to the incoming preference having a value instead of null - but I think that's OK - both will recover once both channels are updated such that this patch exists (and we've lived with this bug for so long that this delay before things settle down seems fine) Another subtlety is prefs that have different default on different channels will now have different values on these channels - in that case both channels will remain with the different values - but that's actually a feature - see the early comments in this bug.

(In reply to Mark Hammond [:markh] from comment #6) > The prefs engine already writes null for non-existing outgoing prefs, and > already does a reset of the pref when the incoming value is null. This patch > also writes a null for prefs with the default value. So this patch doesn't disable syncing of unchanged preferences per se -- it syncs a sentinel, null, which means "use the default value". Why did you pick this approach rather than, say, omitting the key altogether?

(In reply to Richard Newman [:rnewman] from comment #7) > (In reply to Mark Hammond [:markh] from comment #6) > > > The prefs engine already writes null for non-existing outgoing prefs, and > > already does a reset of the pref when the incoming value is null. This patch > > also writes a null for prefs with the default value. > > So this patch doesn't disable syncing of unchanged preferences per se -- it > syncs a sentinel, null, which means "use the default value". > > Why did you pick this approach rather than, say, omitting the key altogether? 2 main reasons: * Prefs with the default value in profile 1 will currently existing in profile 2 with the same value as the default, but not be considered unmodified. This approach means such already synced prefs will correctly be marked as unmodified in other profiles. While bug 747264 excluded some prefs that have different defaults on different channels, there may be other prefs that either now or in the future have different defaults on different channels that would be bitten by this. * Consider a user that explicitly flips a pref to a non-default value, then Syncs, then resets the pref - because the pref is now the default value it would be excluded from the payload and thus would not be reset on the other profile - with this approach it would.

Comment on attachment 8725969 [details] [diff] [review] 0001-Bug-744626-ensure-default-prefs-are-synced-such-that.patch Review of attachment 8725969 [details] [diff] [review]: ----------------------------------------------------------------- Gosh, I hope this works. At least it only requires testing desktop-to-desktop. ::: services/sync/modules/engines/prefs.js @@ +102,5 @@ > _getAllPrefs: function () { > let values = {}; > for (let pref of this._getSyncPrefs()) { > if (this._isSynced(pref)) { > + // Missing and default prefs get the null value. Add a link to your explanation in this bug! ::: services/sync/tests/unit/test_prefs_store.js @@ +28,5 @@ > + Services.prefs.readUserPrefs(do_get_file("prefs_test_prefs_store.js")); > + // Now we've read from this file, any writes the pref service makes will be > + // back to this prefs_test_prefs_store.js directly in the obj dir. This > + // upsets things in confusing ways :) We avoid this by explicitly telling the > + // pref service to use a file in our profile dir. Yikes! Thanks for the explanation.