TBB prefs are set as user_prefs (may cause exceptions/settings errors?)

All of the prefs we change in TBB appear to be set as user prefs. This makes it hard for me in Torbutton to tell if the user has changed a value.

We also have occasional reports from people that some prefs seem inexplicably set (such as form fill options). Since user_prefs() sets a user value, it probably causes Firefox to emit observer events for each preference we change in this way. If one of them has an exception because the observer for that pref is buggy and/or tries to touch some not-yet-initialized piece of Firefox at startup, that could easily cause some prefs to fail to apply randomly.

changed milestone to %TorBrowserBundle 2.3.x-stable

added component::applications/tor bundles/installation milestone::TorBrowserBundle 2.3.x-stable owner::erinn priority::high resolution::fixed status::closed tbb-bounty tbb-usability type::defect labels

Trac:
Keywords: N/A deleted, tbb-2.2.32-4 added

Trac:
Keywords: tbb-2.2.32-4 deleted, N/A added

This bug also makes it hard for users to overwrite their old TBBs with new versions while still keeping their preferences, and they end up doing crazy, dangerous stuff as a result: https://lists.torproject.org/pipermail/tor-talk/2011-October/021771.html

We probably also need this to make our lives easier for Thandy, for the same reason we needed split user pref config files in Tor and vidalia.

Trac:
Cc: N/A to g.koppen@jondos.de

We need this for #1878 (moved).

Trac:
Priority: normal to major
Milestone: TorBrowserBundle 2.2.x-stable to TorBrowserBundle 2.3.x-stable

Note: In more than a couple places in torbutton (as well as the Firefox-close patch), I do things like:

if (this.prefs.prefHasUserValue("torbrowser.version"))

These checks will all need to be updated when we do this switch, because the switch will cause our pref settings to stop being "user values". If anyone decides to work on this bug, please don't forget to ping me when you start, so I can update the Torbutton checks before we deploy this.

Trac:
Cc: g.koppen@jondos.de to g.koppen@jondos.de, mcs, brade

mcs: Good find. When Firefox 17 is released, you guys can do this if we can get our own update server working in #4234 (moved).

Trac:
Keywords: N/A deleted, tbb-bounty added

Trac:
Keywords: N/A deleted, tbb-usability added

Trac:
torbutton-patch.txt

Part 1 of proposed fix: In Torbutton, use getComplexValue() to retrieve homepage pref.

Erinn and Mike -- please review the two patches. Some details:

• Our browser.startup.homepage value is now handled like the Firefox default value for that pref. It must come from a properties file so that code in Firefox that calls getComplexValue() will work. And we need to change Torbutton to always use getComplexValue() too.

• The other prefs. are now split between a #tor.js file which is placed in the browser application area (named so pref values in it override other Firefox default preferences) and a preferences/extension-overrides.js file which is placed in the user profile area (necessary in order to override default preferences contained within extensions such as NoScript).

• In windows-alpha.mk, we made a couple of somewhat unrelated changes: add /c prefix to MING path and remove some PYMAKE leftovers). Someone who better understands the implications should make sure those changes are desirable.

Also, Kathy and I removed the following preferences which we do not think need to be set: pref("app.update.lastUpdateTime.addon-background-update-timer", 1216566538); pref("app.update.lastUpdateTime.background-update-timer", 1216566535); pref("app.update.lastUpdateTime.blocklist-background-update-timer", 1216566538); pref("app.update.lastUpdateTime.microsummary-generator-update-timer", 1232574822); pref("app.update.lastUpdateTime.search-engine-update-timer", 1216566539); pref("urlclassifier.keyupdatetime.https://sb-ssl.google.com/safebrowsing/newkey", 1235166825); pref("urlclassifier.tableversion.goog-black-enchash", "1.55536"); pref("urlclassifier.tableversion.goog-black-url", "1.23256"); pref("urlclassifier.tableversion.goog-white-domain", "1.481"); pref("urlclassifier.tableversion.goog-white-url", "1.371");

I have a few questions/comments:

1. Shouldn't we place the Torbutton preferences in extension-overrides.js too? Is there a reason why we didn't? We can probably actually just eliminate most of these entirely, though. I can work on that bit in Torbutton.

2. If users change their addon preferences (NoScript for example), will those values take precedence over extension-overrides.js? Ie will prefs.js extension prefs override extension-overrides.js? I think we want this to be the case.

3. Can we entirely remove the prefs.js from the distribution? A key goal here is not overwriting user preferences during updates.

4. There may be some more prefHasUserValue checks in Torbutton. I'll get to those soon.

Replying to mikeperry:

1. Shouldn't we place the Torbutton preferences in extension-overrides.js too? Is there a reason why we didn't? We can probably actually just eliminate most of these entirely, though. I can work on that bit in Torbutton.

Yes, those should be placed in extension-overrides.js too (if they cannot be defaulted appropriately inside Torbutton itself). Good catch.

2. If users change their addon preferences (NoScript for example), will those values take precedence over extension-overrides.js? Ie will prefs.js extension prefs override extension-overrides.js? I think we want this to be the case.

Yes, the /prefs.js is where all user prefs are written and they always take precedence.

3. Can we entirely remove the prefs.js from the distribution? A key goal here is not overwriting user preferences during updates.

Kathy and I were thinking that when we implement an automatic update mechanism that that file would just be excluded (i.e., we would never overwrite it). But it is safer to just omit the prefs.js file entirely so there is no risk that the new (empty) file would overwrite a user's preferences.

4. There may be some more prefHasUserValue checks in Torbutton. I'll get to those soon.

Thanks. I think they are all gone now, but please double-check.

For prefs.js, we want to omit it entirely from the bundle archive so that alternate update mechanisms (such as Thandy, #5236 (moved), or even simply untarring a new tarball over an old TBB dir) don't have to deal with it.

Can you do this and move the torbutton prefs for now, and then I'll clean up and remove redundant ones when I merge?

Also, out of curiosity, do prefs set in extensions-overrides.js show up as UserValue, or as defaults?

Replying to mikeperry:

Can you do this and move the torbutton prefs for now, and then I'll clean up and remove redundant ones when I merge?

Done. I will post an updated patch in a minute. We removed these two because the same pref. values are included in Torbutton: pref("extensions.torbutton.locked_mode", true); pref("extensions.torbutton.restore_tor", true);

Also, out of curiosity, do prefs set in extensions-overrides.js show up as UserValue, or as defaults?

They show up as defaults.

Trac:
torbrowser-patch.txt