620465 - about:support doesn't show modified sync prefs

Status: RESOLVED DUPLICATE of bug 720997

(Firefox :: General, defect)

Description

[Daniel Holbert [:dholbert]]

STEPS TO REPRODUCE:
 1. Open about:config
 2. Set pref services.sync.log.appender.debugLog.enabled = true
 3. Open about:support
 4. Examine "Modified Preferences"

ACTUAL RESULTS:
services.sync.log.appender.debugLog.enabled doesn't show up there.  (Nor do any of my services.sync.* prefs, actually)

Other prefs do, though -- e.g. "security.warn_viewing_mixed = false" is listed there in my profile.

This affects Desktop firefox:
 Mozilla/5.0 (X11; Linux x86_64; rv:2.0b9pre) Gecko/20101220 Firefox/4.0b9pre
...as well as the latest Fennec nightly on android.  (that's where I actually discovered this, when trying to debug why I wasn't getting certain types of sync logging output)

Comment 1

[Daniel Holbert [:dholbert]]

Cww & gavin tell me that about:support has a whitelist, and only shows prefs that match a whitelisted prefix.  (to protect user privacy) Whitelist is currently at:
http://mxr.mozilla.org/mozilla-central/source/toolkit/content/aboutSupport.js#47

It'd probably be useful to add some sync prefs there.  I personally would like to see "services.sync.log." there, so that I could use about:support to confirm that I've set up sync correctly to log my sync output.  (since the about:config UI is still a bit broken on mobile)

Comment 2

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Attachment: patch adding services.sync.* preference branch to whitelist of modified preferences to show on about:support

This patch adds the preference branch services.sync.* to whitelist so that modified preferences of this branch will be shown in about:support. The sub-branch services.sync.prefs.sync. gets added to the blacklist and will be ignored.

Philikon (I know you are on B2G, maybe CC someone working on Sync?): If there should be more sub-branches on the blacklist, please tell me.

Comment 3

[Richard Newman [:rnewman]]

Comment on attachment 594476 [details] [diff] [review]
patch adding services.sync.* preference branch to whitelist of modified preferences to show on about:support

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

Looks good to me. That's the correct prefs branch.

Comment 4

[Philipp von Weitershausen [:philikon]]

Comment on attachment 594476 [details] [diff] [review]
patch adding services.sync.* preference branch to whitelist of modified preferences to show on about:support

Sync has a ton of prefs that aren't really user preferences, more like internal settings (as is quite common throughout Gecko.) Stuff like timestamps, for instance. We don't want to show those there, I think.

Here's my shortlist of prefs that would be useful to track in about:support (may be incomplete):
* services.sync.serverURL
* services.sync.account (this is the user's Sync account name... do we want to show this in about:support?)
* services.sync.numClients
* services.sync.syncInterval
* services.sync.engine.*
* services.sync.addons.*
* services.sync.prefs.*

Comment 5

[Richard Newman [:rnewman]]

(In reply to Philipp von Weitershausen [:philikon] from comment #4)

> Sync has a ton of prefs that aren't really user preferences, more like
> internal settings (as is quite common throughout Gecko.) Stuff like
> timestamps, for instance. We don't want to show those there, I think.

I'd actually disagree; it doesn't hurt to have a little bit more insight into internal state, because we don't know the cause of every bug we'll ever see.

If an engine isn't syncing because somehow its local timestamp is in the far future, seeing timestamp snapshots would be really handy, no?

And I'd rather have this set be simple and inclusive, rather than find out months down the line that we failed to add a new prefs branch to this whitelist.

Comment 6

[Dave Townsend [:mossop]]

(In reply to Philipp von Weitershausen [:philikon] from comment #4)
> Comment on attachment 594476 [details] [diff] [review]
> patch adding services.sync.* preference branch to whitelist of modified
> preferences to show on about:support
> 
> Sync has a ton of prefs that aren't really user preferences, more like
> internal settings (as is quite common throughout Gecko.) Stuff like
> timestamps, for instance. We don't want to show those there, I think.
> 
> Here's my shortlist of prefs that would be useful to track in about:support
> (may be incomplete):
> * services.sync.serverURL
> * services.sync.account (this is the user's Sync account name... do we want
> to show this in about:support?)
> * services.sync.numClients
> * services.sync.syncInterval
> * services.sync.engine.*
> * services.sync.addons.*
> * services.sync.prefs.*

How much harm is there in including them? We could also use a regexp to blacklist /^services.sync.*.lastSync$/ etc. to hide many of the timestamps.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

The bar for showing prefs in about:support is "would you be OK with an oblivious user posting all of that data into a web support forum". I think a user's Sync account name (and maybe sync server URL?) don't meet that criterion.

Comment 8

[Philipp von Weitershausen [:philikon]]

My objection was based on (a) Gavin's point about automatically assuming that all this data is OK to be posted and (b) the current nature of about:support's pref listing which is very concise and actually seems to only contain user-changed values, not system-internal state that is stored in the key-value store that is nsIPreferencesService.

Using a blacklist instead of a whitelist is fine by me.

Comment 9

[Dave Townsend [:mossop]]

Comment on attachment 594476 [details] [diff] [review]
patch adding services.sync.* preference branch to whitelist of modified preferences to show on about:support

Sounds like there is still some work to do to decide which of the prefs should/shouldn't be included here. rnewman can you come up with a suggested list of things that aren't privacy sensitive in any way and are going to be useful for debugging.

I wonder if adding something specifically for timestamps would be useful for about:support. We have a number of prefs that store the last time something happened and showing them only if they are far enough in the future or the past might make sense.

Comment 10

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

Regarding the timestamps, are all timestamps set server-side or also some using date & time from the user machine?

Comment 11

[Philipp von Weitershausen [:philikon]]

(In reply to Archaeopteryx [:aryx] from comment #10)
> Regarding the timestamps, are all timestamps set server-side or also some
> using date & time from the user machine?

The client maintains lots of under-the-covers state in prefs. One of those states is various timestamps, but there are others. I suggest you look for yourself by going to to about:config and looking for services.sync.* prefs. There's a ton and most of them aren't relevant to the about:support use case IMHO.

Comment 12

[Richard Newman [:rnewman]]

(In reply to Dave Townsend (:Mossop) from comment #9)
> rnewman can you come up with a suggested
> list of things that aren't privacy sensitive in any way and are going to be
> useful for debugging.

Will do so, but will have to wait until after the current Android rush is done.

(In reply to Archaeopteryx [:aryx] from comment #10)
> Regarding the timestamps, are all timestamps set server-side or also some
> using date & time from the user machine?

The timestamps stored on the client include both local and server-side timestamps. You can see for yourself in about:config, as Philipp mentioned.

Comment 13

[Matthew N. [:MattN]]

This was feature was implemented in bug 720997 and included the account and username.  The patch in this bug got r- because of the privacy issue of exposing this data so I filed bug 798160 to address the issue.