753289 - Sync overwrites option added manually to the services.sync.prefs. when adding new sync client

Status: RESOLVED FIXED

(Firefox :: Sync, defect)

Description

[jan.dechent]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20100101 Firefox/13.0
Build ID: 20120501201020

Steps to reproduce:

Added manually some options to be synced across different firefoxes

Added to about:config on client A:
services.sync.prefs.sync.browser.sessionstore.privacy_level = true
services.sync.prefs.sync.browser.sessionstore.privacy_level_deferred = true
browser.sessionstore.privacy_level = 2
browser.sessionstore.privacy_level_deferred =2
Then added new synclilent on different PC (same ff version)


Actual results:

Firefox sync the option what will be synced, but overwirtes the option itself:

services.sync.prefs.sync.browser.sessionstore.privacy_level = true
services.sync.prefs.sync.browser.sessionstore.privacy_level_deferred = true
browser.sessionstore.privacy_level = default
browser.sessionstore.privacy_level_deferred = default


Expected results:

firefox should sync the options correctly:
browser.sessionstore.privacy_level = 2
browser.sessionstore.privacy_level_deferred = 2

Comment 1

[Gregory Szorc [:gps]]

This bug makes it sound like preferences sync is broken. We should probably try to reproduce this...

Comment 2

[jan.dechent]

I want to re-report this bug since I installed firefox 13 on a mac today and connected my sync account. The same as stated above happens...
Perhaps my user profile is damaged. Can anyone reproduce this error?

Comment 3

[jan.dechent]

Still buggy in 14.0.1

Comment 4

[Tracy Walker [:tracy]]

questions: is it necessary to restart after adding and setting new prefs?
If so, was that done before you added client B to the Sync account?

Comment 5

[Richard Newman [:rnewman]]

More STR:

https://discourse.mozilla-community.org/t/how-to-sync-preferences-of-a-bootstrapped-extension-via-sync/3024

Comment 6

[tabmix.onemen]

The problem is that the preferences save in the server not sorted
sync.extensions.foo.xxx is saved before services.sync.prefs.sync.extensions.foo.xxx

when the pref engine import the saved record on new device that don't have both sync.extensions.foo.xxx and services.sync.prefs.sync.extensions.foo.xxx, it import sync.extensions.foo.xxx first so PrefStore.prototype._isSynced return false.

The fix for this issue is to sort the imported record with services.sync.prefs.sync. preferences before other preferences.

Comment 7

[Richard Newman [:rnewman]]

That seems like a legitimate fix to me.

Comment 8

[tabmix.onemen]

Attachment: Update 'services.sync.prefs.sync' preferences first

This patch fix this bug.

Comment 9

[Richard Newman [:rnewman]]

Comment on attachment 8654635 [details] [diff] [review]
Update 'services.sync.prefs.sync' preferences first

Review of attachment 8654635 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with that change.

::: services/sync/modules/engines/prefs.js
@@ +89,5 @@
>                        .getBranch(WEAVE_SYNC_PREFS)
>                        .getChildList("", {});
>      // Also sync preferences that determine which prefs get synced.
> +    let weavePrefs = syncPrefs.map(function (pref) { return WEAVE_SYNC_PREFS + pref; });
> +    return weavePrefs.concat(syncPrefs);

Remove these changes; it doesn't look like they do anything, and we don't want to reintroduce uses of 'weave'.

@@ +114,5 @@
>      let selectedThemeIDBefore = this._prefs.get(selectedThemeIDPref, null);
>  
> +    // Update 'services.sync.prefs.sync.foo.pref' before 'foo.pref' otherwise
> +    // _isSynced return false when 'foo.pref' doesn't exist in a new device.
> +    let prefs = Object.keys(values).sort(a => -a.indexOf(WEAVE_SYNC_PREFS));

We only want to bother searching at the start of the string, so I think this is what we want:

  .sort(a => !a.startsWith(WEAVE_SYNC_PREFS));

Comment 10

[Richard Newman [:rnewman]]

Do you need me to land this?

Comment 11

[tabmix.onemen]

Thank you Richard,

Please land this patch, I'm away until September 20.

Comment 12

[Richard Newman [:rnewman]]

I applied my nits and some other cleanup; waiting for try to open. Will land. Thanks!

Comment 13

[Richard Newman [:rnewman]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=aac718b7ccf9

Comment 14

[Richard Newman [:rnewman]]

url:        https://hg.mozilla.org/integration/fx-team/rev/4378a00214c5b591c4c4dce77e03a87619539f64
changeset:  4378a00214c5b591c4c4dce77e03a87619539f64
user:       Richard Newman <[e-mail]>
date:       Thu Sep 10 17:33:50 2015 +0100
description:
Bug 753289 - Pre: rename WEAVE_SYNC_PREFS to PREF_SYNC_PREFS_PREFIX.

url:        https://hg.mozilla.org/integration/fx-team/rev/06f396c07ac9ae10158eec2834faf4d5febbb167
changeset:  06f396c07ac9ae10158eec2834faf4d5febbb167
user:       onemen <[e-mail]>
date:       Thu Sep 10 17:33:50 2015 +0100
description:
Bug 753289 - Apply prefs-related prefs before applying other synced prefs. r=rnewman

Comment 15

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4378a00214c5
https://hg.mozilla.org/mozilla-central/rev/06f396c07ac9

Comment 16

[Nagy Mária]

[Tracking Requested - why for this release]: