[tbb-bugs] #25543 [Applications/Tor Browser]: Rebase Tor Browser patches for ESR60

Tor Bug Tracker & Wiki blackhole at torproject.org
Wed May 16 13:29:40 UTC 2018

#25543: Rebase Tor Browser patches for ESR60
 Reporter:  gk                              |          Owner:
                                            |  arthuredelstein
     Type:  task                            |         Status:
                                            |  needs_revision
 Priority:  Very High                       |      Milestone:
Component:  Applications/Tor Browser        |        Version:
 Severity:  Normal                          |     Resolution:
 Keywords:  TorBrowserTeam201805, ff60-esr  |  Actual Points:
Parent ID:  #25741                          |         Points:
 Reviewer:                                  |        Sponsor:

Comment (by gk):

 Alright, here come the remaining bits:

 8a17d0bb0f00987477233cdb0103b5cda724a491 -- okay (009934b82a3c)
 a0fb3774961eacdeaaee6ad5bef1a93c9fb69722 -- not okay (fba536f97fe2)

 "Only ship the e10srollout system extension and pdfjs." should probably be
 "Only ship pdfjs."

 Why do we need `if os.path.exists("$(DIST)/bin/browser/features") else
 []};`? In which case is this available and in which not? Does this affect
 pdf.js bundling?

 The fixup (569a412d41a020b8c75ab6d62f4fae42f2802b89 is okay)

 a30b878113f442895c907066b70941374d2e0c0b -- not okay (c542fb08d725)

 I think there is no need to take this as a separate commit due to
 https://bugzilla.mozilla.org/show_bug.cgi?id=1433507 and
 https://bugzilla.mozilla.org/show_bug.cgi?id=1433357. So, what we should
 do is adding two fixup commits instead: one for the defense-in-depth pref
 and one for adding the no-proxy-bypass flag to our .mozconfig files.

 aa9b5e2811ab86a66d7ee20eec981c110a23c7a0 -- not okay (c79b911518ed)

 This got upstreamed in
 https://bugzilla.mozilla.org/show_bug.cgi?id=1382359. We should do a fixup
 commit setting `dom.securecontext.whitelist_onions` to `true` and that's
 it. So "C c79b911518ed" -> "U c79b911518ed"

 aba739b085d1499a4ac31f4a1be0235c3c12b093 -- not okay (576f4e90158a)

 This got upstreamed in
 https://bugzilla.mozilla.org/show_bug.cgi?id=1382359. We should just leave
 this patch I think, until we seriously fix the tests run with our config.
 Then we should uptream that fix. So "C 576f4e90158a" -> "U 576f4e90158a".

 d999aa5f22a8081cf01d3a62e0be3cecb71df3df -- okay (f7e646dd976c)
 13a0b11f84b776a82d03070215b0e9285be5dce3 -- not okay (fc9f5757efd6)

 That's not needed anymore/upstreamed. We'd take newer patches tjr is
 making if there are things left. So, I'd say "F[inspect] fc9f5757efd6" ->
 "U fc9f5757efd6".

 5be9a93363c20a348316e4d6ec10e979255be437 -- not okay (a19fd1255901)

 Not needed. We'll take the patch for
 https://bugzilla.mozilla.org/show_bug.cgi?id=1460882 once it landed (as
 well as a bunch of other patches to fix compile/runtime issues).

 8a45f06acad1cf8640e0487f632b5fbd10bdd97d -- okay (87b15309e159)
 45571449efe1bb047314dc5d90931666bfc87697 -- okay (6794707e2b3a)
 ee6b576bc2a9f5c885ae9f027dd1bd4e78ab86ce -- not okay (2646633951fe)

 That's not needed anymore given that
 https://bugzilla.mozilla.org/show_bug.cgi?id=1418052 landed.

 9684e260d0cb2abd4fb62d7960f9f25e60b48c5f -- not okay (009bc0a8f600)

 I think this could be a fixup to our .mozconfig commit.

 f297c39eb75ffc7bfd71a42916684dc98c904ca0 -- okay (230cb85895bc)
 b654a7f58a15c500b44f69b81386eca2dc68264a -- not okay (ab9be0575af0)

 Could we fix up the commit message a bit while we are at it (s/bug/big/)?
 The fixup (commit d8bcb696fddc1d779c40994a96af52f4a8759520) looks good.

 8093f4a07c9b7efc46421dfbf49095128693c9e5 -- okay (b91202db5ef3)
 8068477fe20d947b074aee854538b0f6ddf73815 -- okay (6e2c459fa66a)
 ed8959815ed38d415fe1b71d0fb4e312bffd5b57 -- not okay (f5eebe23eda5)

 Seems to be a good candidate for a fixup for our prefs file and not an own

 a4ae1392fffd7aa6ae1ba28575bad6de3b3489a3 -- not okay (ba141b6054ea)

 Not needed anymore as `showModalDialog()` is gone (see:

 50f255b69aba42ea05fcc11b315000274500c38d -- not okay (95ad1e098907)

 As said in a previous comment, this and the commit originally implementing
 this feature should go for now as it is broken and we need a replacement.

 cac8d0158e81c7809afbe922c00c60d418b32850 -- not okay (93999a363c76)

 Let's use the upstreamed patch in
 https://bugzilla.mozilla.org/show_bug.cgi?id=1441327 instead.

 4ac551d9d7d1c3b6aebc5a2fccc7ea5b7aa79da4 -- not okay (c70454fd10ef)
 +static bool
 +IsSecureHost(nsIURI *aHostURI) {
 Please add a newline before "+static bool".
 You need to patch the `isSecure` usage in `AddInternal()` as well as in
 patch on our esr52 branch.

 5b566bf19806ebf1fb56750b549a59cc9c9d3191 -- not okay (82cd8ae9a5de)

 It's fixed with the fixup commit

 abf40ca35cf8e42b88d19a915d6f27d3893c399f -- not okay (90e16dd25b6e)

 I think we should take what we got after some reviews by Moz folks in
 https://bugzilla.mozilla.org/show_bug.cgi?id=859782. If we are lucky we
 can just backport a patch that landed before we ship anything. Still, what
 we have right now in the ticket is preferable to what we have I think.

 ca2d7fbabb2b596fd34c14c069affa177d55b2c9 -- okay
 03c5dd35093e9b57923ffd7bae684f4410852f4c -- okay
 34407382d48ebbbc6deb74dcf5f67e95a74d8346 -- okay
 31530f47b84a0df2c1b9371089f7f8211cccd8a9 -- not okay

 It seems to me with the telemetry pref flip in
 34407382d48ebbbc6deb74dcf5f67e95a74d8346 we don't need the `_enabled`
 check. (see my comment in #25909 as well)

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

More information about the tbb-bugs mailing list