[tor-bugs] #21952 [Applications/Tor Browser]: Onion-location: increasing the use of onion services through automatic redirects and aliasing

[Tor Bug Tracker & Wiki]

#21952: Onion-location: increasing the use of onion services through automatic
redirects and aliasing
-------------------------------------------------+-------------------------
 Reporter:  linda                                |          Owner:  acat
     Type:  project                              |         Status:
                                                 |  needs_review
 Priority:  Medium                               |      Milestone:
Component:  Applications/Tor Browser             |        Version:
 Severity:  Normal                               |     Resolution:
 Keywords:  ux-team, tor-hs, network-team-       |  Actual Points:  10
  roadmap-november, tbb-9.5, network-team-       |
  roadmap-2020Q1, TorBrowserTeam202003R          |
Parent ID:  #30024                               |         Points:  6
 Reviewer:  pospeselr, mcs, brade                |        Sponsor:
                                                 |  Sponsor27-must
-------------------------------------------------+-------------------------

Comment (by mcs):

 Here are some UX and code-related comments for the `21952+4` branch from
 Kathy and me:
 * UX: The "Onion Services" and "Onion Services Authentication" sections
 are not near each other within about:preferences#privacy. Is that okay or
 should we move one of them?
 * UX: Should clicking the "Always Prioritize Onions" option flip the pref
 as well as take the user to about:preferences? It surprises us that
 clicking it does not carry out the action.
 * Please expand the commit message to mention a little bit about the new
 behavior (Onion-Location HTTP and meta http-equiv are both supported,
 etc.).
 * Please add a Tor copyright notice to the new files.
 * Should we move the new code under browser/components/onionservices or
 would that be too messy? When Kathy and I picked that name we were hoping
 some other .onion-related code could be placed there so we don't end up
 adding too many directories under browser/components.
 * A general comment: you could use `const` in more places, e.g., replace
 `let win = event.target.ownerGlobal;` with `const win =
 event.target.ownerGlobal;`.
 * Consider renaming the pref `privacy.prioritizeonions.notification` to
 `privacy.prioritizeonions.notification.shown` or reverse the meaning and
 rename it to `privacy.prioritizeonions.showNotification`.
 * In browser/base/content/browser.js, consider adding a blank line and/or
 comment above the `OnionLocationParent.onStateChange(browser);` statement
 (since that line is unrelated to the `clear out search-engine data` code).
 * In browser/components/onionlocation/OnionLocationChild.jsm
 `receiveMessage()`, consider renaming the `document` variable to `doc`
 (the former makes me think of the window's document property from HTML-
 land).
 * In browser/components/onionlocation/OnionLocationChild.jsm
 `onPageShow()` it looks like the second parameter to the
 `OnionLocation:Set` IPC message can be removed since it is not used on the
 receiving end.
 * In browser/components/onionlocation/OnionLocationParent.jsm
 `showNotification()`, remove the blank line after `let seen = ...` and
 move the `let win = ...` statement after the `if (seen)` block.

 Sorry if many of these seem nit-picky, but hopefully we are down to nits.
 Maybe Richard will find something bigger to complain about ;)

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