#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
_______________________________________________ tor-bugs mailing list tor-bugs@lists.torproject.org https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs