[tor-bugs] #31286 [Applications/Tor Browser]: Include bridge configuration into about:preferences
Tor Bug Tracker & Wiki
blackhole at torproject.org
Fri Oct 11 16:34:16 UTC 2019
#31286: Include bridge configuration into about:preferences
-------------------------------------------------+-------------------------
Reporter: gk | Owner:
| pospeselr
Type: task | Status:
| needs_revision
Priority: High | Milestone:
Component: Applications/Tor Browser | Version:
Severity: Normal | Resolution:
Keywords: tbb-9.0-must-alpha, ff68-esr, ux- | Actual Points:
team, TorBrowserTeam201910 |
Parent ID: #10760 | Points: 15
Reviewer: | Sponsor:
| Sponsor44-can
-------------------------------------------------+-------------------------
Changes (by acat):
* keywords: tbb-9.0-must-alpha, ff68-esr, ux-team, TorBrowserTeam201910R
=> tbb-9.0-must-alpha, ff68-esr, ux-team, TorBrowserTeam201910
* status: needs_review => needs_revision
Comment:
Some more comments:
* Do you think it's fine to fix automatic eslint issues via `./mach lint
-l eslint --fix browser/modules/TorStrings.jsm
browser/modules/TorProtocolService.jsm browser/modules/BridgeDB.jsm
browser/components/torpreferences`, or do you fear it might break
something? In principle, it should only fix those which are safe.
* When opening `about:preferences`, the 'Tor' text quickly disappears and
then appears again.
* After setting "local proxy" (I tried localhost:9050) and restarting, the
Tor section does not work again. See
`browser/components/torpreferences/content/torProxySettings.jsm` comments,
I think it's related to that.
* This is for UX probably, but in `Use local proxy` there's no feedback
about whether the introduced proxy was correctly configured or not (e.g.
was there some error due to port being empty, or some typo, etc.)
* In browser console, there are some errors:
{{{
XML Parsing Error: undefined entity
Location: moz-nullprincipal:{3424fa6d-a047-43e6-ad56-6c3b351213d8}
Line Number 1, Column 143:
}}}
perhaps this is expected because of missing strings, but not sure.
----
* browser/components/preferences/in-content/main.js:
* browser/components/preferences/in-content/main.xul:
* In other patches (e.g. #26345) we went for hiding via css
(`display:none`). I'm not sure what is best though, so it's up to you (or
if someone else has an opinion...).
* browser/components/torpreferences/content/torCategory.inc.xul
* These are localized via JS (not fluent), so perhaps we can remove the
data-l10n attributes?
* browser/components/torpreferences/content/parseFunctions.jsm:
* nit: in the error `Invalid PORT value, needs to be on range
[0,65535] : '${port}'` -> shouldn't say range [1,65535]?
* browser/components/securitylevel/content/securityLevel.js
* // TODO: I'm pretty sure these lines are NO-OPs now as they
return references to objects, rather
// than populating the global scope?
* Unfortunately, *I think* `ChromeUtils.import` does have side-
effects, meaning that the exported symbols are populated as "globals" in
the scope it's executed. If that's a *.jsm, then those will be available
only in that *.jsm. If it's a XUL script, then it will be available for
other XUL scripts too. But that's ok here, I think.
* browser/components/torpreferences/content/requestBridgeDialog.jsm
* The `TorProtocolService` import is unused (eslint error).
* browser/components/torpreferences/content/requestBridgeDialog.jsm:
* typo: `use strinct;`
* `// user may have opened a Request Bridge dialog in another tab, so
update the` -> It seems the end of the comment got cut.
* Not a blocker, but FWIW: Almost all usage of `self` is not needed,
since it's done inside arrow functions, which capture the value of `this`
when they are created, so in all those cases self === this.
It might be needed in the only place where a regular function is used:
{{{
window.setTimeout(function() {
self._populateXUL(dialog);
}, 0);
}}}
Which might be converted into an arrow function and then `self` would
not be needed for
sure (This also valid in other files).
* this._captchaHbox is unused.
* Separe `_captchaGuessIsEmpty` and `init` methods with a blank line?
* nit: comment `// disable submit` ... if input value is empty?
* When you click "reload/get another captcha", shouldn't the "solution
not correct" text
be hidden when you get the new captcha? It's not happening for me.
* browser/components/torpreferences/content/torBridgeSettings.jsm:
* A couple of try/catch for `Services.prefs.getCharPref` can be removed
by providing a
default value (e.g. `let defaultBridgeType =
Services.prefs.getCharPref(TorStrings.preferenceKeys.defaultBridgeType,
null);`. ESlint is
complaining about those.
* browser/components/torpreferences/content/torFirewallSettings.jsm:
* `parseAddrPortList` is undefined, I think it should be imported.
* browser/components/torpreferences/content/torPane.js
* eslint complains about `gSubDialog` not being defined, but I see this
also happening on `privacy.js` or `home.js` which we did not touch so I
guess we can ignore this.
* browser/modules/TorStrings.jsm:
* I think at some point we should avoid the hardcoded english
string fallbacks, perhaps using the fact that we have en-US torbutton
locales always available and use those. But perhaps we can use a different
ticket for that later.
* browser/components/torpreferences/content/torProxySettings.jsm:
* several parse* functions are undefined, I think they should be
imported (actually I thought these would not really throw errors but at
least for me they do, on a restart in readSettings).
* in `readSettings`, `tlps` and `reply` are never used.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/31286#comment:33>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list