[tor-bugs] #25750 [Applications/Tor Launcher]: update Tor Launcher for ESR 60

[Tor Bug Tracker & Wiki]

#25750: update Tor Launcher for ESR 60
--------------------------------------------+------------------------------
 Reporter:  mcs                             |          Owner:  brade
     Type:  defect                          |         Status:  needs_review
 Priority:  Very High                       |      Milestone:
Component:  Applications/Tor Launcher       |        Version:
 Severity:  Normal                          |     Resolution:
 Keywords:  ff60-esr, TorBrowserTeam201805  |  Actual Points:
Parent ID:                                  |         Points:
 Reviewer:                                  |        Sponsor:
--------------------------------------------+------------------------------
Changes (by sysrqb):

 * status:  needs_revision => needs_review


Comment:

 Replying to [comment:15 mcs]:
 > Replying to [comment:9 sysrqb]:
 > > I'll set this to needs-review for the tor-launcher patches I have:
 > > Branch bug25750 on git.tp.o/user/sysrqb/tor-launcher.git
 >
 > Kathy and I reviewed and ran the revised Tor Launcher in a ESR 60-ish
 browser. It is working pretty well – good work! We have a few comments:

 Thanks!

 >
 > 1) I am surprised that XPCOM extensions still work as well as they do.
 We should set `extensions.legacy.enabled = true` and add `tor-
 launcher at torproject.org` to `extensions.legacy.exceptions` so the
 about:addons UI does not label Tor Launcher as LEGACY (and also add
 Torbutton's extension ID if we keep them both as regular add-ons).

 Yes, I was surprised, too. I think FF61 brings a lot more breakage - but I
 think these should be set in/by tor-browser instead of tor-launcher.

 >
 > 2) For 5e5fba0951b0837948bd9c074a3710afe137d0eb, please omit the first
 change.

 Done.

 >
 > 3) For 6105ea64e74cf65e09755d917234fe63b49b84ba, you could use a shorter
 commit message, e.g.,
 >  Mozilla dropped support for Ci.nsIProgrammingLanguage.JAVASCRIPT
 >  See https://bugzilla.mozilla.org/show_bug.cgi?id=1149830#c18

 Done.

 >
 > 4) For 068b9b5e8bb0408879b984c7c7ebd3726f25dd6c you could shorten the
 commit message and reference the entire bugzilla.mozilla.org URL. Maybe
 just use:
 >     Gecko now requries "0o"-prefixed octal literals
 >     See https://bugzilla.mozilla.org/show_bug.cgi?id=1263074
 >

 Done.

 > 5) For 374b64f3057ab4b703e3e79473cd4cfc8abc295e:
 > * How can we guarantee that the call to `_loadPreferences()` occurs
 before any code which might rely on the preference values? Kathy and I
 don't have an answer, but maybe we should add an explicit initialization
 call inside `components/tl-process.js` (either in the `"profile-after-
 change"` case within the `observe()` implementation or at the end of `tl-
 process.js`.

 This was a little tricky because torlauncher uses preferences very early
 in its initialization (such as initializing the TorLauncherLogger module).
 As a result, I moved loading the prefs into the TorProcessService
 constructor. In addition, now an exception is thrown if certain methods
 are called before we load the default prefs (get{Bool,Int,Char}Pref(),
 TorStartAndControlTor(), etc).

 I think we want Tor Browser to fail closed in these situations. Further, I
 think we want Tor Browser failing closed if any of the TorLauncher default
 prefs are invalid or aren't set for any reason.

 > * Please use `let` instead of `var` in new code when possible (we are
 migrating in that direction).
 > * Please use the Cc and Ci constants.
 > * Please log a message when ignoring invalid pref names (near the top of
 the `pref()` function).

 All done.

 > * You can use `this.mPrefsSvc` inside `_getPrefDefaultBranch()`; if you
 do that, it might not be worthwhile to have a `_getPrefDefaultBranch()`
 function.

 I don't see how we can access the DefaultBranch from `this.mPrefsSvc`
 because mPrefsSvc is a `Ci.nsIPrefBranch` but we need `Ci.nsIPrefService`
 for retrieving the default branch. If there's a way to get the default
 branch from nsIPrefBranch, then I'll do that.

 One option, which requires more changes, is changing `mPrefsSvc` a
 `nsIPrefBranch`. Then we can use both `getBranch()` and
 `getDefaultBranch()` from that.

 https://developer.mozilla.org/en-US/docs/Archive/Add-
 ons/Code_snippets/Preferences#Default_preferences

 >
 > 6) For d1efcd33ca6389479337f70a603c73c8264888b8:
 > * Mozilla now uses Services.intl.DateTimeFormat(). We should too. See:
 https://hg.mozilla.org/releases/mozilla-beta/rev/650bd331efd0
 > * I don't think we should add a blank line after this one:
 >   let fracSecsStr = ...
 > (that declaration is tied to the `for` loop that follows immediately
 after).

 Done.


 I have pushed a new branch: bug25750_1

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