Move privacy checkboxes to about:preferences#privacy (proposed)

I would like to propose moving the Privacy Settings checkboxes to the about:preferences#privacy page. That means removing them from torbutton and adding them to tor-browser.git

Here is my reasoning:

1. The security slider can be separated from the privacy settings. The privacy checkboxes are dangerous, because the default (all checked) offers maximum privacy, and unchecking any of the checkboxes substantially reduces privacy. Whereas the security slider has low security by default, and moving the slider always increases security. So I think it's better to encourage use of the security slider and discourage use of the privacy checkboxes.

2. We unify the privacy user interface, which hopefully simplifies it for users. There are already privacy settings for Firefox on the about:preferences#privacy page, so it seems natural to move them there. Also the "Always Use Private Browsing Mode" checkbox on that page is more or less equivalent to torbutton's "Don't record browsing history" checkbox. And then we don't have to duplicate the "restart" prompt associated with that checkbox.

3. Hopefully we can uplift to mainline Firefox, one of our SponsorU goals, thereby exposing our great privacy features (first party isolation, fingerprinting resistance, etc.) to Firefox users in the privacy UI.

added TorBrowserTeam201610 component::applications/tor browser owner::tbb-team priority::medium resolution::fixed severity::normal status::closed tbb-fingerprinting tbb-no-uplift tbb-usability type::defect labels

Good idea, although that might not fit into SponsorU anymore (time-wise).

Trac:
Cc: N/A to gk

I've written patches for this, because it's going to make the problems I encountered in #18093 (moved) much easier to solve.

There are 6 patches here (in which I remove checkboxes one by one and then clean up the UI): https://github.com/arthuredelstein/torbutton/commits/20244 and 2 patches here (where I add two checkboxes to the about:preferences#privacy page): https://github.com/arthuredelstein/tor-browser/commits/20244

Note that the last torbutton patch adds an overlay so that the our translated checkbox labels can be temporarily applied to the about:preferences page. If Mozilla uplifts these to Firefox and translates the labels themselves, then we won't need this overlay any more.

Trac:
Status: new to needs_review

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

Replying to arthuredelstein:

I've written patches for this, because it's going to make the problems I encountered in #18093 (moved) much easier to solve. yay :) There are 6 patches here (in which I remove checkboxes one by one and then clean up the UI): https://github.com/arthuredelstein/torbutton/commits/20244 And rename "Privacy and Security Settings..." menu item? and 2 patches here (where I add two checkboxes to the about:preferences#privacy page): https://github.com/arthuredelstein/tor-browser/commits/20244 "Restrict third party..." has "strange" relations with FF's "Third party cookies" option, so the layout should reflect the dependencies. And what about "Custom settings" instead of "Never"? Would it be default or both would be good (after bugfixing)? Note that the last torbutton patch adds an overlay so that the our translated checkbox labels can be temporarily applied to the about:preferences page. If Mozilla uplifts these to Firefox and translates the labels themselves, then we won't need this overlay any more. It's good to get this upsteamed while Mozilla is in mozilla52: behind a pref as resistFingerprinting and first-party isolation, but also Mozillians could make suggestions about naming, layout, hints, etc in the FF's UI.

Replying to arthuredelstein:

I've written patches for this, because it's going to make the problems I encountered in #18093 (moved) much easier to solve.

Kathy and I reviewed and ran with all of the patches. Nice work! Comments below.

There are 6 patches here (in which I remove checkboxes one by one and then clean up the UI): https://github.com/arthuredelstein/torbutton/commits/20244

• There is a typo in the commit message for 38f3653b89ce3790d75b8652316f7381158d0af3 ("Bug Bug").
• You sometimes treat privacy.thirdparty.isolate as a Boolean pref, which does not seem correct.
• When I clicked the button in about:plugins to enable plugins, no warning prompt was displayed. Possibly related, there is no default value for plugins.disable which caused this to appear on the browser console:

16:26:17.523 NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]1 torbutton.js:2323:0

• Please remove the following from src/defaults/preferences/preferences.js:

extensions.torbutton.resist_fingerprinting
extensions.torbutton.restrict_thirdparty

• Can we reduce the height of the preferences window? I cannot remember how the height is determined, but on OSX there is now a lot of extra space below the slider. Or maybe it is just remembering an old height that I set by resizing.

and 2 patches here (where I add two checkboxes to the about:preferences#privacy page): https://github.com/arthuredelstein/tor-browser/commits/20244

Note that the last torbutton patch adds an overlay so that the our translated checkbox labels can be temporarily applied to the about:preferences page. If Mozilla uplifts these to Firefox and translates the labels themselves, then we won't need this overlay any more.

• In the tor-browser patches, should we use the same labels as we have in src/chrome/locale/en/torbutton.dtd? Or did you intentionally make them different to verify that the overlay was working?
• Should we include the access keys in the overlay so they can be localized?

Trac:
Status: needs_review to needs_revision

Replying to mcs:

Replying to arthuredelstein:

I've written patches for this, because it's going to make the problems I encountered in #18093 (moved) much easier to solve.

Kathy and I reviewed and ran with all of the patches. Nice work! Comments below.

Thanks for the review!

• There is a typo in the commit message for 38f3653b89ce3790d75b8652316f7381158d0af3 ("Bug Bug").

Fixed.

• You sometimes treat privacy.thirdparty.isolate as a Boolean pref, which does not seem correct.

Oops, good catch. Fixed.

• When I clicked the button in about:plugins to enable plugins, no warning prompt was displayed. Possibly related, there is no default value for plugins.disable which caused this to appear on the browser console: {{{ 16:26:17.523 NS_ERROR_UNEXPECTED: Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]1 torbutton.js:2323:0 }}}

Turns out I had consistently mistyped "plugin.disable" as "plugins.disable". Fixed.

• Please remove the following from src/defaults/preferences/preferences.js: {{{ extensions.torbutton.resist_fingerprinting extensions.torbutton.restrict_thirdparty }}}

Done. I have now also removed "extensions.torbutton.block_disk" in patch 20244.1 and use the "browser.privatebrowsing.autostart" pref instead.

• Can we reduce the height of the preferences window? I cannot remember how the height is determined, but on OSX there is now a lot of extra space below the slider. Or maybe it is just remembering an old height that I set by resizing.

I added maxwidth and maxheight properties to stop it from getting too big.

• In the tor-browser patches, should we use the same labels as we have in src/chrome/locale/en/torbutton.dtd? Or did you intentionally make them different to verify that the overlay was working?

I did, but now I have made them the same.

• Should we include the access keys in the overlay so they can be localized?

Added.

Here are the new versions: ​https://github.com/arthuredelstein/torbutton/commits/20244+1 ​https://github.com/arthuredelstein/tor-browser/commits/20244+1

Trac:
Status: needs_revision to needs_review

Replying to arthuredelstein:

Here are the new versions: ​https://github.com/arthuredelstein/torbutton/commits/20244+1 ​https://github.com/arthuredelstein/tor-browser/commits/20244+1

Very nice. My only comment is that the commit message for d1d6bdee6fa52d33ac3b004af1df2d472545b37a still mentions plugins.disable.

Replying to mcs:

Very nice. My only comment is that the commit message for d1d6bdee6fa52d33ac3b004af1df2d472545b37a still mentions plugins.disable.

Ack. Here is the fix: ​https://github.com/arthuredelstein/torbutton/commits/20244+2 ​https://github.com/arthuredelstein/tor-browser/commits/20244+1 (Nothing changed in tor-browser branch.)

First notes:

You did

-    if (!this.prefs.getBoolPref("extensions.torbutton.block_disk")) {
+    if (!this.prefs.getBoolPref("browser.privatebrowsing.autostart")) {

once in cookie-jar-selector.js. Any reason you left the other three extensions.torbutton.block_disk occurrences?

Nit: We don't have "Privacy and Security Settings" anymore on the preferences pane. I guess we can omit the comments now.

There is still one plugins.disable in the commit message of e79d9bebfb396c3ca3d48edc930e4f213c88ccc1.

It seems you need to remove torbutton.context_menu.networksettings.key in all non-en-US locales as well to make sure it gets the new value N assigned?

All the other things look good to me, nice! I've not tested the patches, yet, though.

Trac:
Keywords: TorBrowserTeam201610R deleted, TorBrowserTeam201610 added
Status: needs_review to needs_revision

Replying to gk:

All the other things look good to me, nice! I've not tested the patches, yet, though.

Thanks for the review, Georg! Here's the new version fixing the problems you found:

https://github.com/arthuredelstein/torbutton/commits/20244+3 ​​https://github.com/arthuredelstein/tor-browser/commits/20244+1 (Again nothing changed in the tor-browser branch.)

Trac:
Status: needs_revision to needs_review

Alright, that got fixed on tor-browser-45.4.0esr-6.5-1 with commits

1b92582f7ef5bec693b01bc78672122add405834 e5c3cd25ed459765a91d1b364151c46afaca36c0

and on torbutton master with commits

6d785fa1bf95f997a95b867538ba3353d9a76a58 aa05941ee0144026fb843b6f5a2098aed5e17b46 2978978e64fbc9164185564a19d56d5fea0b25d8 ee88e783a98b22e743b943cc144cb6b79c70706e ea0d6dc4ebc59c594162202e142549356075a981 7f16ada703d07b1b0140eaf12dd3e93864c14568

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

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

Trac:
Keywords: N/A deleted, tbb-no-uplift added

closed

mentioned in issue #20291 (moved)