[tbb-bugs] #25658 [Applications/Tor Browser]: Activity 2.1: Improve user understanding and user control by clarifying Tor Browser's security features

[Tor Bug Tracker & Wiki]

#25658: Activity 2.1: Improve user understanding and user control by clarifying Tor
Browser's security features
-------------------------------------------------+-------------------------
 Reporter:  isabela                              |          Owner:
                                                 |  antonela
     Type:  project                              |         Status:
                                                 |  needs_revision
 Priority:  High                                 |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ux-team, GeorgKoppen201812,          |  Actual Points:
  TorBrowserTeam201903R, tbb-8.5                 |
Parent ID:                                       |         Points:
 Reviewer:                                       |        Sponsor:
                                                 |  Sponsor17
-------------------------------------------------+-------------------------
Changes (by mcs):

 * status:  needs_review => needs_revision


Comment:

 Here are our comments on the tor-browser changes (by the way, thanks for
 writing code that is nicely organized and easy to read):

 High level questions for Georg and others:
 1. Should we add copyright notices to the
 `browser/components/securitylevel/` files?
 2. Should we rename the `security_slider` pref and migrate the old value
 at this time, or later, or never? Maybe use the name
 `torbrowser.security_level` and migrate to contiguous values (1-3).


 Commit message:
 1. The first line is very long. Maybe shorten to `Bug 25658: Replace
 security slider with security level UI`.
 2. s/hangar/hanger/ (two occurrences).

 browser/app/profile/000-tor-browser.js - there are some additional
 NoScript and HTTPS-E references that should be removed from the
 `browser.uiCustomization.state` value:
 * https-everywhere-button
 * https-everywhere_eff_org-browser-action
 * _73a6fe31-595d-460b-a920-fcc0f8843232_-browser-action

 securityLevelPanel.css - please combine all of the margin-related rules
 into one within each CSS selector block, or at least put them on adjacent
 lines so it is easy to spot the margin things.

 securityLevel.js - SecurityLevelStrings:
 1. The `intialize our strings with en-US defaults...` comment seems a
 little backwards. Maybe replace with `read localized strings from
 Torbutton; use hard-coded en-US strings as fallback`
 2. Is it worth it to rely on the `getlocale()` function from Torbutton?
 3. There is one `/` missing after `https:` in the URL. It still works
 though!

 securityLevel.js - SecurityLevelPrefs:
 1. The `_str` suffix for the pref name constants is a little confusing.
 Maybe use `_pref` instead.

 securityLevel.js - SecurityLevelButton:
 1. Consider renaming `securitySlider` to `securityLevel` (and maybe rename
 the pref getter too).
 2. Kathy and I think the function name `_readPrefsInternal` could be
 better, e.g., replace all occurrences with `_configUIFromPrefs()`.

 securityLevel.js - SecurityLevelPanel:
 1. In the `panel` getter, `_panel` is set but not used anywhere else.
 Maybe you meant to cache that value?
 2. Remove the comment inside `openAdvancedSecuritySettings()`.
 3. For `_populateXUL()`: if you have time and can do it cleanly, consider
 adding a helper function that sets the strings within each of the three
 radio sections (the code is very similar for each).

 securityLevel.js - SecurityLevelPreferences:
 1. _populateXUL(): remove blank line before closing brace within
 `vboxStandard` section.

 securityLevel.js - miscellaneous typos:
  s/abotu/about/
  s/hangar/hanger/
  s/lable/label/
  s/re-render/re-renders/
  s/scurity/security/
  s/upate/update/
  s/if(/if (/

 securityLevel.js - remove blank lines at the beginning of function bodies:
  SecurityLevelButton.init()
  SecurityLevelButton.onCustomizeEnd()
  SecurityLevelPreferences.init()
  SecurityLevelPreferences.selectSecurityLevel()

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/25658#comment:87>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online