[tbb-bugs] #26233 [Applications/Tor Browser]: Rebase Tor Browser patches for FF61
Tor Bug Tracker & Wiki
blackhole at torproject.org
Sat Jun 9 06:49:01 UTC 2018
#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
More information about the tbb-bugs
mailing list