Opened 3 months ago

Closed 2 months ago

#26233 closed enhancement (fixed)

Rebase Tor Browser patches for FF61

Reported by: sysrqb Owned by: arthuredelstein
Priority: Very High Milestone:
Component: Applications/Tor Browser Version:
Severity: Normal Keywords: TorBrowserTeam201806R, tbb-mobile
Cc: sysrqb, igt0, tbb-team Actual Points:
Parent ID: #26338 Points:
Reviewer: Sponsor:

Description

In #25543, the tor-browser patches were rebased from 52ESR onto 60ESR. Here we need the tor-browser patches rebased onto FF61 because that is what the TBA patches are based on.

This should be significantly easier than #25543.

Child Tickets

Change History (15)

comment:1 Changed 3 months ago by sysrqb

Owner: changed from tbb-team to sysrqb
Status: newaccepted

comment:2 Changed 3 months ago by sysrqb

I was originally doing this as part of #25741, but it'll be easier for review if we factor this into its own child ticket.

comment:3 Changed 2 months ago by arthuredelstein

Status: acceptedneeds_review

Here's my branch, which I created by rebasing tor/tor-browser-60.0.1esr-8.0-1 (30544586f8a98e985e0038b2e905bfddb14e999e) onto a recent gecko-dev/beta (c432661a14b1ad331e22e2524ae8d57f9161884f):

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

I left out three updater patches that are complex to rebase and I think won't be needed for Android in any case (please correct me if I'm wrong!). I also squashed the fixup commits.

Here's what happened to each patch:

C = cherry-picked
F = fixed up
U = upstreamed
? = left out for now (updater patches)

C 30544586f8a9 fixup! TB4: Tor Browser's Firefox preference overrides.
F b135c59f65db Fix MAR generation bashism
C 1ba7b6b25113 fixup! TB4: Tor Browser's Firefox preference overrides.
C b589ec74c427 Bug 20283: Tor Browser should run without a `/proc` filesystem.
C 3a6cb718e815 Bug 21537: Tests for secure .onion cookies
C 58e4a739a6ed Bug 21537: Mark .onion cookies as secure
U 440d2fe1b6f4 Bug 1441327 - Allow for seccomp filtering of socket(AF_INET/AF_INET_6) calls on Linux when using UNIX Domain Sockets for SOCKS Proxy. r=bagder
C ed1a45a69d15 Bug 22548: Firefox downgrades VP9 videos to VP8.
C 550d0bae6d40 Bug 24398: Plugin-container process exhausts memory
F 3206814bc291 Bug 23104: Add a default line height compensation
C 53f7ab7d844a Bug 24052: Handle redirects by blocking them early
C 5e0170a7ca05 Bug 13398: at startup, browser gleans user FULL NAME (real name, given name) from O/S
C c5544f727e46 Bug 21830: Copying large text from web console leaks to /tmp
C 962babebfc5e Bug 21321: Add test for .onion whitelisting
F d18befdee332 Bug 21431: Clean-up system extensions shipped in Firefox 52
C fe68460a72cd Bug 21569: Add first-party domain to Permissions key
C 2aa950923c66 Bug 16285: Exclude ClearKey system for now
C 32da0487944c Bug 21907: Fix runtime error on CentOS 6
C d010f98a92fe Bug 21849: Don't allow SSL key logging
F d29c1ddb254d Bug #5741: Prevent WebSocket DNS leak.
F c67a0c07fd5b Bug 14970: Don't block our unsigned extensions
F 3549c5324dda Omnibox: Add DDG, Startpage, Disconnect, Youtube, Twitter;  remove Amazon, eBay, bing
C e984b8c54c75 Bug 23916: Add new MAR signing key
? 70989b5211d8 Bug 16940: After update, load local change notes.
? 189164e100db Bug 13379: Sign our MAR files.
? 4b4dcd40abae Bug 4234: Use the Firefox Update Process for Tor Browser.
C 7ca562c26856 Bug 25909: disable updater telemetry
C 39f10aaa10ef Bug 19121: reinstate the update.xml hash check
C 9a3bb35800d5 Bug 19121: reinstate the update.xml hash check
F a4ac08e62457 Bug 13252: Do not store data in the app bundle
C cab08be85615 Bug 21724: Make Firefox and Tor Browser distinct macOS apps
C 75d638dddd7d Bug 18912: add automated tests for updater cert pinning
C b95e30974e71 Bug 18900: updater doesn't work on Linux (cannot find  libraries)
C 716067b4c679 Bug 11641: change TBB directory structure to be more like Firefox's
C 4e0aed04f7f7 Bug 9173: Change the default Firefox profile directory to be TBB-relative.
C 71a812c584aa Bug 19890: Disable installation of system addons
C 17367581443f Bug 19273: Avoid JavaScript patching of the external app helper dialog.
C 4da1d08fb2e2 Bug 18923: Add a script to run all Tor Browser specific tests
C 612aefdabd9b Regression tests for #2874: Block Components.interfaces from content
C 27fa6ab6fa2b Bug 18821: Disable libmdns for Android and Desktop
C 40752ee655eb Bug 18800: Remove localhost DNS lookup in nsProfileLock.cpp
C cc9862c27fd5 Bug 18799: disable Network Tickler
C 5823d75f953a Bug 16620: Clear window.name when no referrer sent
C 48b1c08e1fff Bug 16441: Suppress "Reset Tor Browser" prompt.
C 6734d99f40b0 Bug 14392: Make about:tor behave like other initial pages.
C 006dffb468ee Bug 2176: Rebrand Firefox to TorBrowser
C 16a1bcef4e15 Bug 18995: Regression test to ensure CacheStorage is disabled in private browsing
C c5b57b1bf1df Regression tests for "Omnibox: Add DDG, Startpage, Disconnect, Youtube, Twitter; remove Amazon, eBay, bing"
C 6b35333f3a3a Regression tests for TB4: Tor Browser's Firefox preference overrides.
C 93a8e5d1b523 Regression tests for Bug #2950: Make Permissions Manager memory-only
C 1dc1a4f7fedc Bug 12620: TorBrowser regression tests folder
C b6d8bf568ba6 Bug 14631: Improve profile access error msgs (strings).
C 10ac5a7be31f Bug 14631: Improve profile access error messages.
C 87cb0833ffdd Bug 14716: HTTP Basic Authentication prompt only displayed once
C ccebcbb95267 Bug 13028: Prevent potential proxy bypass cases.
C dfd201a96767 Bug 16439: remove screencasting code.
F e0cb606a47ac Bug 2874: Block Components.interfaces from content
C 58a737f021b2 Bug 12974: Disable NTLM and Negotiate HTTP Auth
C fe1e6ce7f8d8 Bug 10280: Don't load any plugins into the address space.
C 837f8e888cf5 Bug 8312: Remove "This plugin is disabled" barrier.
C 1cbd34d3b0b8 Bug 3547: Block all plugins except flash.
F 0e8dbb37c450 TB4: Tor Browser's Firefox preference overrides.
C 4bdb543b0ae7 TB3: Tor Browser's official .mozconfigs.

comment:4 Changed 2 months ago by gk

Cc: tbb-team added
Keywords: TorBrowserTeam201806R added; TorBrowserTeam201805 removed

comment:5 Changed 2 months ago by sysrqb

Initial testing with rebasing this branch on top of my current #25741 branch is looking good! Thanks!

I added two fixups on top of my branch[0] that disable (ifdef-out) unused code which cause build failure when compiled with -Werror. This allows for running Try builds[1][2]. The HEAD commit is needed because it's possible mozilla-central and mozilla-beta don't have any commits by ffxbld within the last 500 commits (as is currently the situation).

I'm trying to get the Tor Browser mochitests working - but I won't spend too much time on this. I think I'm hitting a local issue.

[1] https://gitweb.torproject.org/user/sysrqb/tor-browser.git/log/?h=26233%2b25741
[1] https://treeherder.mozilla.org/#/jobs?repo=try&revision=aeeb247553c2d713478083d3f02bb9c9a78bf0a7
[2] https://treeherder.mozilla.org/#/jobs?repo=try&revision=c26bc84a8caffb2000475016edf5114b4e049193

comment:6 Changed 2 months ago by gk

Keywords: TorBrowserTeam201806 added; TorBrowserTeam201806R removed
Status: needs_reviewneeds_revision

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:

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)? What's the relation to https://bugzilla.mozilla.org/1401260?

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.

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?).

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").

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.

f4c2dd434d4bb4aa5cbe4937c6f4e5f85eef74f9 -- okay (716067b4c679)
3f7ac7eb4d1fc0e66b3105fa105c639aea134733 -- not okay (b95e30974e71)

not needed for android as it is an updater patch

a5a2ed6a11b3b1042e02f568f61f8de45a715f49 -- not okay (75d638dddd7d)

not needed for android as it is an updater patch

999fe945d5c6f7cbc53c5546a11a4284fb2da71d -- okay (cab08be85615)
ca4a654280c0a04e89a4dc8b926c540cfb9fb554 -- okay (a4ac08e62457)

6dad47dd282fe46e888c56e3067869bf99cec47f -- not okay (9a3bb35800d5)

not needed for android as it is an updater patch

1478cf4bf40f50144ef053d0e27d5e39946c11ba -- not okay (39f10aaa10ef)

not needed for android as it is an updater patch

026597a98e8a75442bc59978d7f2b43bc98c0e2d -- okay (7ca562c26856)
013a9bd751a57e3b2d28adebe4c742b2300624a8 -- not okay (e984b8c54c75)

not needed for android as it is an updater patch

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.

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

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?

comment:7 in reply to:  6 Changed 2 months ago by sysrqb

Owner: changed from sysrqb to arthuredelstein
Status: needs_revisionassigned

Replying to gk:

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:

I don't see a problem using this same format. Unfortunately, as mentioned before, TB Android will begin overlapping with TB Desktop next year. So, either Desktop begins following mozilla-release at that time (next ~April), or we choose a unique name.

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?

Originally I thought we would use the same naming scheme, because "these are both tor browser", but after thinking about this more we should probably start at 1.0. There are 7 or 8 releases between every ESR, so the version would become very confusing. I think if we follow Mozilla's alpha, beta, stable then we can start at 1.0 and we'll reach 8.0 by next April.

comment:8 Changed 2 months ago by sysrqb

Status: assignedneeds_revision

comment:9 in reply to:  6 ; Changed 2 months ago by arthuredelstein

Keywords: TorBrowserTeam201806R added; TorBrowserTeam201806 removed
Status: needs_revisionneeds_review

Replying to 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.

comment:10 in reply to:  9 ; Changed 2 months ago by gk

Status: needs_reviewneeds_information

Replying to arthuredelstein:

Replying to gk:

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.

I see. What's the reason for doing

> -    rv = nsXREDirProvider::GetUserAppDataDirectory(getter_AddRefs(file));
> +    rv = mDirProvider.GetUserAppDataDirectory(getter_AddRefs(file));

essentially reverting bug 1443080 and deviating from the fix pattern using the GetSingleton()-approach?

comment:11 in reply to:  10 Changed 2 months ago by arthuredelstein

Status: needs_informationneeds_review

Replying to gk:

What's the reason for doing

> -    rv = nsXREDirProvider::GetUserAppDataDirectory(getter_AddRefs(file));
> +    rv = mDirProvider.GetUserAppDataDirectory(getter_AddRefs(file));

essentially reverting bug 1443080 and deviating from the fix pattern using the GetSingleton()-approach?

We're reverted that piece of 1443080 because in our patch, the methods are no longer static. (That bug was removing instance calls to static functions.) We could use either pattern, but because mDirProvider is already available and used commonly in that file it seemed OK.

comment:12 Changed 2 months ago by gk

Resolution: fixed
Status: needs_reviewclosed

Looks good to me. I think we are done here.

comment:13 Changed 2 months ago by gk

Resolution: fixed
Status: closedreopened

comment:14 Changed 2 months ago by gk

Parent ID: #25741#26338

comment:15 Changed 2 months ago by gk

Resolution: fixed
Status: reopenedclosed
Note: See TracTickets for help on using tickets.