Opened 4 years ago

Closed 3 years ago

#18093 closed defect (fixed)

Torbutton UI flow improvement

Reported by: bugzilla Owned by: arthuredelstein
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: tbb-usability, tbb-torbutton, tbb-security-slider, TorBrowserTeam201610R
Cc: arthuredelstein, brade, mcs Actual Points:
Parent ID: Points:
Reviewer: Sponsor: SponsorU

Description

In Privacy and Security Settings user presses Reset to Defaults button and then presses Cancel to discard changes, but nothing happens.

Child Tickets

Change History (22)

comment:1 Changed 4 years ago by bugzilla

Keywords: tbb-usability added; tbb-usablility removed

And a lot of users think that they discarded changes, and don't open dialog again to check.

comment:2 Changed 4 years ago by gk

Keywords: tbb-security-slider added

comment:3 Changed 3 years ago by gk

Keywords: TorBrowserTeam201609 added
Sponsor: SponsorU

Getting this on our radar for September.

comment:4 Changed 3 years ago by arthuredelstein

Cc: arthuredelstein added

comment:5 Changed 3 years ago by arthuredelstein

Owner: changed from tbb-team to arthuredelstein
Status: newaccepted

comment:6 Changed 3 years ago by arthuredelstein

Status: acceptedneeds_information

The "Restore Defaults" button runs torbutton_prefs_reset_defaults in preferences.js. The comment there says the function will

 //  1. Clear torbutton settings    
 //  2. Clear browser proxy settings 
 //  3. Reset Security Slider settings

Because this does much more than simply reset the security slider and privacy settings visible in the window, the button is confusing. It's also unclear to the user what settings "Restore Defaults" will restore -- all Firefox prefs are not restored; only some select ones hardcoded in torbutton.

So I can think of a few alternative designs:

  1. Make "Restore Defaults" apply only to the Security Slider and privacy checkboxes in the Privacy and Security Settings window.
  2. Move "Restore Defaults" to the main torbutton menu, to avoid confusing it with the Privacy and Security settings. (We could also consider restoring ALL prefs to default, instead of an arbitrary few.)
  3. Remove the "Restore Defaults" button altogether.

I lean toward option 2.

If we do retain the "Restore Defaults" button, we should probably show a confirm dialog. We should also add a tooltip explaining exactly what the button does.

Any opinions on this?

comment:7 Changed 3 years ago by bugzilla

reset_defaults mutated to "Restore Defaults", but it is usually used in Save/Restore pair.
"Clear" means make empty/""/0/NULL, so the comments are really scary.
"Clear browser proxy settings" is not expected by users.
About Options:

  1. That's how UI logic works.
  2. Not move, add to option 1. Reset Torbutton's prefs only (as now). #13346, #16441, #19135 are about restoring ALL.
  3. And how to revert custom changes then?

No confirm dialog and tooltip are needed for option 1, but make sense for option 2.

comment:8 in reply to:  6 ; Changed 3 years ago by brade

Cc: brade mcs added

Replying to arthuredelstein:

Any opinions on this?

As of 1b41636c9351b46bbcb2094a65c65a4407b60a37 (the fix for #15852), the browser proxy preferences are not reset. I guess the "2. Clear browser proxy settings" should have been replaced with "2. Reset some browser preferences" (or with a comment that more accurately describes what torbutton_reset_browser_prefs() does).

I do not know the history of "Restore Defaults" and I am not sure how the list of preferences that is hard-coded into torbutton_reset_browser_prefs() was generated, but I agree that if we are going to change more settings that those that are controlled from that dialog we should remove the button and make this feature accessible from somewhere else.

Do we need this feature? I wonder how many people need it? Or as bugzilla suggests, maybe we should we fix #19135 instead? Note that "Refresh Tor Browser" may work correctly on OSX now that we have moved the browser profile out of the .app area and made it so the browser can create a working profile during startup.

comment:9 in reply to:  8 Changed 3 years ago by brade

Replying to brade:

Note that "Refresh Tor Browser" may work correctly on OSX now that we have moved the browser profile out of the .app area and made it so the browser can create a working profile during startup.

A quick test on OSX shows that clicking "Refresh Tor Browser" on the about:support page appears to work OK, but we should take a closer look to be sure.

comment:10 Changed 3 years ago by arthuredelstein

Here's a patch for review that implements option (1), and defers applying the settings until the OK button is pressed:
https://github.com/arthuredelstein/torbutton/commit/18093

comment:11 in reply to:  10 Changed 3 years ago by mcs

Status: needs_informationneeds_revision

Replying to arthuredelstein:

Here's a patch for review that implements option (1), and defers applying the settings until the OK button is pressed:
https://github.com/arthuredelstein/torbutton/commit/18093

Kathy and I reviewed this and have a few comments:

  1. The implementation of torbutton_reset_browser_prefs() should be removed since that function is no longer used.
  2. You need to pass document to the revised torbutton_prefs_reset_defaults() function.
  3. Normally, when the user tries to check or uncheck the torbutton_blockDisk checkbox, a "Restart Tor Browser?" prompt is displayed. If "Restore Defaults" modifies that pref, the same prompt should be displayed. I think this problem exists without your changes though.

(For 1. and 2., maybe you just forgot to include your preferences.xul and torbutton.js changes).

comment:12 Changed 3 years ago by gk

Keywords: TorBrowserTeam201610 added; TorBrowserTeam201609 removed

Moving SponsorU items to October.

comment:13 Changed 3 years ago by arthuredelstein

Here's my proposed new version that simply removes the "Restore Defaults" button. It only makes sense if #20244 fixes are used. In this situation, we have no checkboxes, so to "restore defaults" the user only needs to drag the security slider back to the position labeled Low (Default), or possibly uncheck the "Custom Values" box.

https://github.com/arthuredelstein/torbutton/commit/18093+1

Of course this needs to be critically evaluated to decide both (a) if the approach makes sense from a usability standpoint, and (b) if the code changes are correct.

Last edited 3 years ago by arthuredelstein (previous) (diff)

comment:14 Changed 3 years ago by arthuredelstein

Status: needs_revisionneeds_review

comment:15 Changed 3 years ago by gk

Keywords: TorBrowserTeam201610R added; TorBrowserTeam201610 removed

comment:16 in reply to:  13 ; Changed 3 years ago by bugzilla

Replying to arthuredelstein:
It's really great to see how you've started to go step by step into the right direction of redesigning UI, so that it's now a real "Torbutton UI flow improvement"!

uncheck the "Custom Values" box.

Hint: it's better to replace this checkbox with a new Security Slider position "Expert" above High, especially for mobile UX, which will have the same settings as High for the first time (so text to the right will be the same), and text will disappear when custom settings will have been made (and this position will preserve (!) custom settings while switching back and forth from other positions!)

(b) if the code changes are correct.

It depends on whether all settings you want could be restored without "Restore Defaults".

comment:17 Changed 3 years ago by mcs

The changes look okay to me (I did not try to run with them applied).
Since most users simply discard their entire browser or their entire data directory when they want to restore to a known state, I do not think Restore Defaults will be missed.

comment:18 Changed 3 years ago by arthuredelstein

Here is a new version based on patch 20244+1:
https://github.com/arthuredelstein/torbutton/commit/18093+2

comment:19 in reply to:  16 Changed 3 years ago by arthuredelstein

Replying to bugzilla:

It's really great to see how you've started to go step by step into the right direction of redesigning UI, so that it's now a real "Torbutton UI flow improvement"!

Thanks!

Hint: it's better to replace this checkbox with a new Security Slider position "Expert" above High, especially for mobile UX, which will have the same settings as High for the first time (so text to the right will be the same), and text will disappear when custom settings will have been made (and this position will preserve (!) custom settings while switching back and forth from other positions!)

That's an interesting suggestion. I opened #20347.

comment:20 in reply to:  18 ; Changed 3 years ago by mcs

Replying to arthuredelstein:

Here is a new version based on patch 20244+1:
https://github.com/arthuredelstein/torbutton/commit/18093+2

This still looks okay (same changes as before, just rebased, right?)

comment:21 in reply to:  20 Changed 3 years ago by arthuredelstein

Rebasing again, this time on top of 20244+3:
https://github.com/arthuredelstein/torbutton/commit/18093+3

comment:22 Changed 3 years ago by gk

Resolution: fixed
Status: needs_reviewclosed

Cherry-picked to torbutton master with commit 7a0efdf6a99969007792d418a6bbfd5e0da4b3cf.

Note: See TracTickets for help on using tickets.