Closed Bug 939481 Opened 11 years ago Closed 10 years ago

No sync preferences defined for some Privacy & Security settings

Categories

(SeaMonkey :: Sync UI, defect)

defect
Not set
minor

Tracking

(seamonkey2.23 wontfix, seamonkey2.24 fixed, seamonkey2.25 fixed, seamonkey2.26 fixed)

RESOLVED FIXED
seamonkey2.26
Tracking Status
seamonkey2.23 --- wontfix
seamonkey2.24 --- fixed
seamonkey2.25 --- fixed
seamonkey2.26 --- fixed

People

(Reporter: rsx11m.pub, Assigned: rsx11m.pub)

References

Details

Attachments

(1 file, 1 obsolete file)

I've noticed that some services.sync.prefs.sync.{privacy,security}.* default preferences are missing in browser-prefs.js; one of them I forgot myself as part of bug 835134, the other may have been omitted on purpose in bug 576970.
Attached patch Add missing sync prefs (obsolete) — Splinter Review
This adds services.sync.prefs.sync.privacy.donottrackheader.value for which the UI was introduced in bug 835134; the second preference missing from sync is services.sync.prefs.sync.privacy.sanitize.promptOnSanitize, where bug 576970 only introduced services.sync.prefs.sync.privacy.sanitize.sanitizeOnShutdown.

Jens, if the latter preference setting isn't supposed to be synced on purpose, please let me know and I'll remove it from the patch
Assignee: nobody → rsx11m.pub
Status: NEW → ASSIGNED
Attachment #833388 - Flags: review?(iann_bugzilla)
Flags: needinfo?(jh)
(In reply to rsx11m from comment #1)
> the second preference missing from sync
> is services.sync.prefs.sync.privacy.sanitize.promptOnSanitize, where bug
> 576970 only introduced
> services.sync.prefs.sync.privacy.sanitize.sanitizeOnShutdown.
> 
> Jens, if the latter preference setting isn't supposed to be synced on
> purpose, please let me know and I'll remove it from the patch

I think we only left it out because the initial set of preferences was based on Firefox's set, and promptOnSanitize is SM-only.

Anyway, from my POV there are only two checks needed to decide whether we should configure a pref for syncing by default or not:
1) Whether the pref name is static. Dynamic ones, like used in some parts of MailNews, are currently not supported by the Preferences Sync engine.
2) Whether we expect users to have the pref value equal on multiple computers. Ones that might for example have different values on different platforms (with equal pref name) should not be added since Sync works cross-platform.

Of course, the usual review chain applies, so the reviewer might disagree, or me (as the SM Sync owner), or the SM Council (unlikely, but for the sake of completeness...).
Flags: needinfo?(jh)
Oh, I figured that preferences imply Ian's review without realizing that you are actually the module owner for Sync, sorry about that (Ian is listed as a peer, thus I hope that I'm still fine).

Bug 765398 introduced privacy.donottrackheader.value and also added it to the Sync prefs for Firefox, so I'd assume that there is no question whether or not to add it given that privacy.donottrackheader.enabled is already synced and the two together determine the configuration of the DNT feature.

Similarly, I'd think that privacy.sanitize.promptOnSanitize should be synced given that it's sibling privacy.sanitize.sanitizeOnShutdown already is (why would you set one the same on all installations but keep the others different?).
(In reply to rsx11m from comment #3)
> Oh, I figured that preferences imply Ian's review without realizing that you
> are actually the module owner for Sync, sorry about that (Ian is listed as a
> peer, thus I hope that I'm still fine).

Hehe, choosing a peer is perfectly fine, and especially a Preferences expert. :-)

Regarding which prefs to sync, I'd like to point out that it doesn't matter much what Firefox does or which similar/related prefs are already synced. I'm not implying that your judgment as described in comment 3 falls into either category; just wanted to make sure you get the idea that "what makes sense for the average/targeted user" is all that should count.

That said, I agree that the prefs you cited should be synced (according to your description; don't take this as "I checked those pref names").
Flags: needinfo?(jh)
(Weird, bugzilla seems to be loosing track of the flags again. I didn't request another needinfo?...)
Comment on attachment 833388 [details] [diff] [review]
Add missing sync prefs

[Triage Comment]

r=me, a=me for c-a/c-b too
Attachment #833388 - Flags: review?(iann_bugzilla)
Attachment #833388 - Flags: review+
Attachment #833388 - Flags: approval-comm-beta+
Attachment #833388 - Flags: approval-comm-aurora+
> pref("services.sync.prefs.sync.privacy.sanitize.sanitizeOnShutdown", true);
>+pref("services.sync.prefs.sync.privacy.sanitize.promptOnSanitize", true);

As mentioned on IRC, these needed to be switched to retain alphabetical order.
Keywords: checkin-needed
Whiteboard: [c-n: attachment 8345273 for comm-central/aurora/beta]
Attachment #833388 - Attachment is obsolete: true
https://hg.mozilla.org/comm-central/rev/a958e35a8119
https://hg.mozilla.org/releases/comm-aurora/rev/e7cc5e3c989a
https://hg.mozilla.org/releases/comm-beta/rev/b3fbfd93e668
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [c-n: attachment 8345273 for comm-central/aurora/beta]
Target Milestone: --- → seamonkey2.26
Comment on attachment 8345273 [details] [diff] [review]
Final patch for checkin [r+a=IanN]

Thanks. I can't carry forward the a+ flags, but for reference let's set the r+ from attachment 833388 [details] [diff] [review] at least.
Attachment #8345273 - Attachment description: Final patch for checkin → Final patch for checkin [r+a=IanN]
Attachment #8345273 - Flags: review+
You need to log in before you can comment on or make changes to this bug.