[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #21952 [Applications/Tor Browser]: Onion-location: increasing the use of onion services through automatic redirects and aliasing
#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
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs