[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:  9
  roadmap-november, tbb-9.5,                     |
  TorBrowserTeam201912R                          |
Parent ID:  #30024                               |         Points:  6
 Reviewer:  pospeselr, mcs, brade                |        Sponsor:
                                                 |  Sponsor27-must
-------------------------------------------------+-------------------------

Comment (by mcs):

 Replying to [comment:85 acat]:
 > Revised branch for review in https://github.com/acatarineu/tor-
 browser/commits/21952+2.
 >
 > I split the changes into two commits for reviewing purposes, UI and
 "internals" needed for the http redirect. For the internals, I used a
 couple of flags (`LOAD_FLAGS_ONION_REDIRECT` and `LOAD_ONION_REDIRECT`)
 the value of which I think is not used anywhere else.

 Kathy and I reviewed all of your changes and overall everything looks
 really good to us. Nice work!

 We are not very up-to-date on the page load and networking code, so it
 would be good to get a review (even a quick one) from a Mozilla engineer
 who has expertise in those areas. But we can wait and see what Richard has
 to say after he completes his review.

 Kathy and I think it would be safer to use a higher bit for
 `LOAD_FLAGS_ONION_REDIRECT`, since we are not sure where or how the lower
 bits are used. The comment "reserved for internal use by nsIWebNavigation
 implementations" makes us think that other code may count on 0x1, 0x2,
 0x4, and 0x8 being available/unused by the core code.

 > But as I said, I'm not sure if it would be better to rewrite that commit
 and use the `redirectTo` JavaScript API instead.

 Do you think that would make the patches simpler or more complex? From
 Mozilla's perspective (assuming we could possibly uplift these patches
 someday), the C++ approach is probably better.

 Here are some UX comments related to the "Always prioritize Onions" prompt
 (but note that Kathy and I have not closely followed all of the
 discussions related to this feature):
 * Clicking the `[x]` has the same effect as clicking "Not Now" and
 therefore the `[x]` does not seem necessary.
 * Using a doorhanger for the prompt makes us think that it is safe to
 click the `[.onion available]` button to dismiss the prompt (because that
 is how most of Firefox's other doorhangers work). I am not sure how to fix
 this issue though.
 * There is no way to dismiss the prompt in such a way that it will be
 shown a second time, which surprises Kathy and me. But I guess the "Always
 prioritize Onions" prompt is only intended to serve as a form of
 onboarding. I wonder if the prompt should be adjusted to open
 about:preferences instead of directly modifying the
 `privacy.prioritizeonions.enabled` pref? Opening the preferences would
 teach users where to find the setting in the future.

 Other code-related comments:
 In modules/libpref/init/StaticPrefList.h, please add a comment that
 explains the purpose of the `privacy.prioritizeonions.enabled` preference.

 We will need a `torbutton.dtd` patch to add the strings. Since these are
 new strings, you could use a properties file instead; see our patch here
 for an example:
 https://gitweb.torproject.org/user/brade/tor-
 browser.git/diff/browser/modules/TorStrings.jsm?h=bug30237-03&id=509999fe8ef29fa3cb2915bdfb050d4512014081

 In OnionLocationParent.jsm inside the `setOnionLocation()` and
 `updateOnionLocationBadge()` functions, is it necessary to check `browser`
 against `...selectedBrowser`? As it is now, if an https page that returns
 an `Onion-Location` header finishes loading in the background the `[.onion
 available]` button is not displayed.

 A nit: in browser/components/onionlocation/content/onionlocation-
 urlbar.css, please add a space before each occurrence of `!important`
 (easier to read, at least for Kathy and me).

 Do we need another ticket to track creation of a new page for the
 "learnMore" link or is there an existing page that we can use?

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