[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #26233 [Applications/Tor Browser]: Rebase Tor Browser patches for FF61



#26233: Rebase Tor Browser patches for FF61
-----------------------------------------------+---------------------------
 Reporter:  sysrqb                             |          Owner:
                                               |  arthuredelstein
     Type:  enhancement                        |         Status:
                                               |  needs_review
 Priority:  Very High                          |      Milestone:
Component:  Applications/Tor Browser           |        Version:
 Severity:  Normal                             |     Resolution:
 Keywords:  TorBrowserTeam201806R, tbb-mobile  |  Actual Points:
Parent ID:  #25741                             |         Points:
 Reviewer:                                     |        Sponsor:
-----------------------------------------------+---------------------------
Changes (by arthuredelstein):

 * status:  needs_revision => needs_review
 * keywords:  TorBrowserTeam201806, tbb-mobile => TorBrowserTeam201806R,
     tbb-mobile


Comment:

 Replying to [comment:6 gk]:
 > Okay, here comes the review. I think MAR files and the signing machinery
 we have won't play a role for Android (see: #26242 for our update
 strategy), thus we can ignore all the signing related patches, I agree.
 I've been a bit more agressive when reviewing your changes. Here is all I
 got:

 Thanks for the review! Here's my new branch:

 https://github.com/arthuredelstein/tor-browser/commits/26233+1

 > c7036c883efebaf0ee6d27285e7a5f9d0abe8eb8 -- good (4bdb543b0ae7)
 > 2078afc6814a8f0303d9a83c050c068bda704ce3 -- not okay (0e8dbb37c450)
 > {{{
 > > +// Disable window.Components shim (substitue for
 https://bugs.torproject.org/2874)
 > > +pref("dom.use_components_shim", false);
 > }}}
 > "substitue" and could you mention the Mozilla bug number (1448048)?.

 Fixed.

 > What's the relation to https://bugzilla.mozilla.org/1401260?

 Looks like that bug has stalled, but in principle that telemetry proposed
 there would have been useful for the justifying final removal of the
 components shim. Since that's no longer needed, I removed the [tor 2874]
 whiteboard tag.

 > f2c4006d0958a61d671ad7ab8e4c097115ece39c -- okay (1cbd34d3b0b8)
 > dfca44dbb3011918f1860ff10ddf7fa825b33713 -- okay (837f8e888cf5)
 > 4df8383599da34d038e47980ce6005f6355d6a43 -- okay (fe1e6ce7f8d8)
 > 5e9c8426b5d485ecc02c85cbb98d11305310ef79 -- okay (58a737f021b2)
 >
 > F e0cb606a47ac Bug 2874: Block Components.interfaces from content <- I
 was confused by that because there is no bug 2874 related patch anymore.
 So, better "O" I guess.

 You're right.

 > 5de7b7cdcf120b1fcf29f51d2ce2659d317ec445 -- okay (dfd201a96767)
 > 9abd24dae812b2f3fc2918b2fb22285f9c3b1392 -- okay (ccebcbb95267)
 > 8657933084abd6cb6745239698b8d24e11d69dc1 -- okay (87cb0833ffdd)
 > ef0eee9cb5aa10e338c1979ff51d7fa6b90f6b0b -- okay (10ac5a7be31f)
 > c96db008adde9b8f692786ce5dab817eca961507 -- okay (b6d8bf568ba6)
 > ec4d2f41b75d3be3c239421fc6e6905d65efe8e7 -- okay (1dc1a4f7fedc)
 > a8e5b7264ecf03e4d554d7a5cfead42159144d0d -- not okay (93a8e5d1b523)
 >
 > I think that can go as the patch got upstreamed in bug 967812 (it seems
 I overlooked that by the review for ESR60). Or do we want to keep the test
 because the upstreamed patch does not contain tests? In that case I am
 fine with keeping it for now but we should get the test on the uplift
 > radar in this case (why is it marked as `tbb-no-uplift`?).

 I'm inclined to keep it so we can propose uplifting a test. I removed the
 `tbb-no-uplift` keyword from #2950.

 > 174ce1dc9a00c7af8fd1019cc30e24903fada4d7 -- okay (6b35333f3a3a)
 > 1a683a1d1d0079dafa15a4a3957da24182ea1f44 -- okay (c5b57b1bf1df)
 > 36d44849da69cf008023519cb4bcede18e96c99c -- okay (16a1bcef4e15)
 > 6da338503b55259c63160c5e24536ff1e77b5184 -- okay (006dffb468ee)
 > 1b35f4478c904b0990a45d4a5f191fcc62f4b9ba -- okay (6734d99f40b0)
 > 6300c93066d07ea3fa54c31852c1564992383877 -- okay (48b1c08e1fff)
 > 184d59cff2b987c608d9746badd7a007f00423b8 -- okay (5823d75f953a)
 > c4331c511caf1a2feaa95e6f6bfcbcd3393907b8 -- okay (cc9862c27fd5)
 > f3140e62aaa16e05e9202d6dd4e656886bb285fd -- okay (40752ee655eb)
 > e09d2897b4d67e3f65212269e6b774047bfe75fe -- okay (27fa6ab6fa2b)
 > f1e3011eb042f516c24de3734b42a1e71d8744c1 -- not okay (612aefdabd9b)
 >
 > It seems we don't need that test anymore given that the patch is
 basically obsolete (i.e. the fingerprinting defense got "upstreamed").

 Removed.

 > cee52946e9d5b16bb0c42d69e6896d7334995f58 -- okay (4da1d08fb2e2)
 > ead92b734879ab153db67da293ac9cb6add8a186 -- okay (17367581443f)
 > acef0857f3f6aeac13f4e94eb98f8b55d28e1127 -- okay (71a812c584aa)
 > 79289c71dc16e3064e1ceb17fb1900e0d08273d8 -- not okay (4e0aed04f7f7)
 >
 > Where are the changes in nsToolkitProfileService.cpp etc. coming from? I
 fail to find them on m-c. It seems to me we don't want to differe more
 than needed from Mozilla here.

 In our original patch, the `static` keyword has been removed from some
 functions in `nsXREDirProvider.h` and so all invocations need to be
 modified. Unfortunately some additional invocations appeared in other
 files so these need to be modified as well.

 > f4c2dd434d4bb4aa5cbe4937c6f4e5f85eef74f9 -- okay (716067b4c679)
 > 3f7ac7eb4d1fc0e66b3105fa105c639aea134733 -- not okay (b95e30974e71)
 >
 > not needed for android as it is an updater patch

 Removed.

 > a5a2ed6a11b3b1042e02f568f61f8de45a715f49 -- not okay (75d638dddd7d)
 >
 > not needed for android as it is an updater patch

 Removed.

 > 999fe945d5c6f7cbc53c5546a11a4284fb2da71d -- okay (cab08be85615)
 > ca4a654280c0a04e89a4dc8b926c540cfb9fb554 -- okay (a4ac08e62457)
 >
 > 6dad47dd282fe46e888c56e3067869bf99cec47f -- not okay (9a3bb35800d5)
 >
 > not needed for android as it is an updater patch

 Removed.

 > 1478cf4bf40f50144ef053d0e27d5e39946c11ba -- not okay (39f10aaa10ef)
 >
 > not needed for android as it is an updater patch

 Removed.

 > 026597a98e8a75442bc59978d7f2b43bc98c0e2d -- okay (7ca562c26856)
 > 013a9bd751a57e3b2d28adebe4c742b2300624a8 -- not okay (e984b8c54c75)
 >
 > not needed for android as it is an updater patch

 Removed.

 > c42570e07aaae5883cea1c05ff685b78e3c4de33 -- okay (3549c5324dda)
 > d5acddaef9c2884a3216febefa77389eb12d4c7e -- not okay (c67a0c07fd5b)
 >
 > {{{
 > +    // Make sure Torbutton, TorLauncher, EFF's HTTPS-Everywhere and
 meek
 >      if (XPIDatabase.mustSign(addon.type) &&
 > }}}
 > One comment line got lost during the rebase.

 Added back in.

 > 85226bc17e8ffc79bed66ea0e4464b4ff0c2c83e -- okay (d29c1ddb254d)
 > 1493d82415ad5df36fdd9c77810e49438a821e03 -- okay (d010f98a92fe)
 > 86846e65504ddb2f85ca9df1f547e8742d6b14f5 -- okay (32da0487944c)
 > 660b6baf00bd4ff2a5ddf4f63d9dc15df070e62d -- okay (2aa950923c66)
 > 90bff70c9ce677b27c2324359bd5217b07893613 -- okay (fe68460a72cd)
 > 82763bdd1d1f04237a6655ed3b201922dab7ddcd -- okay (d18befdee332)
 > 90fc7286242189f3e52aefe7b29809de1609130c -- okay (962babebfc5e)
 > 1b6e98a4c8d6669cbe6974dc854687358f7f5043 -- okay (c5544f727e46)
 > 34cf9e7affd2f28b89e3b51f97c7ae8fcbfac67a -- okay (5e0170a7ca05)
 > d47a03f002293d4e3521e0e873f0a5848db0d3a2 -- okay (53f7ab7d844a)
 > 346aa2832c7f53b77e6bdb54adfca0de1d21125e -- okay (3206814bc291)
 > db2c0629cc760f0479827f594f7a5f0b3ca2ee5d -- okay (550d0bae6d40)
 > 4ee877d89d24f4daa28ecf250fe218d6cbd83d9a -- okay (ed1a45a69d15)
 > fd88a6380aa544e114330a8cfc0524b7ebb40587 -- okay (58e4a739a6ed)
 > 0e25749a125ef5858f6d745d09cd2e4a4267ed9c -- okay (3a6cb718e815)
 > 7b68864ca39ad1ba2ea082312b66cac09024681d -- okay (b589ec74c427)
 > e4e28039ecab5bdc57c1c6d0db0d808cab6dd5cf -- not okay (b135c59f65db)
 >
 > not needed for android as it is an updater patch

 Removed.

 > Two things we should think about while preparing the new branch:
 >
 > 1) How would the naming scheme for our mobile branches look like?
 Currently we have "tor-browser-
 $ESR_VERSION-$TORBROWSER_MAJOR_VERSION-$BRANCH_NUMBER" which points to the
 second question:
 >
 > 2) What version number for Tor Browser for Android do we start with?
 Especially given that we are tracking Mozilla release and not esr anymore?
 Should we start over with 1.0?

 A random idea: what about using the same numbers as Firefox? Like Tor
 Browser Android 61.0, etc.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/26233#comment:9>
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