Rebase Tor Browser patches for Firefox ESR 68
We need to start rebasing our patches against Firefox 68. This is the ticket that tracks the whole effort.
It's helpful how we did it the last time: comment:6:ticket:25543. As mentioned there https://torpat.ch/ is a very valuable resource. It might need updating, though (which we should do while we are at it, or point Arthur to the things that need to get fixed).
Activity
Trac:
Child Ticket(s): #28822 (moved)Using commits
47aefe747950..e9b8de8c8d6afrom tor-browser-60.7.0esr-9.0.1.Most of the desktop patches are rebased in https://github.com/acatarineu/tor-browser/commits/30429.
Below you can find the list commits/patches split into several "categories". Order of commits was not preserved in all cases, but the set of the commits of all categories below should be the same as above (
47aefe747950..e9b8de8c8d6a).In that branch there is a WIP #10760 (moved) commit adding the browser parts of the torbutton integration, and also temp. moving the torbutton submodule to a WIP branch https://github.com/acatarineu/torbutton/tree/10760. This of course will have to change once/if torbutton migration proposal is approved and #10760 (moved) changes accepted. Most (or all?) torbutton features should be working with this.
I'm currently building with
mach build && mach packageand disabling tor-launcher (--disable-tor-launcher), since AFAIK it would still not work without #29197 (moved). I could also not run tbb-tests (although I did not try very hard yet). To launch Tor, I'm doing it manually withtor HashedControlPassword $(tor --quiet --hash-password mypassword) SocksPort 9150 ControlPort 9151 HTTPTunnelPort 0 DataDirectory /tmp/tortempthen launching the built browser with
TOR_CONTROL_PASSWD='"mypassword"' ./firefoxNoScript addon can be manually installed (and enabled on private windows, see #28896 (moved)) and then security levels UI should work fine.
Some TODOs:
- Port tor-launcher to 68: at least #29197 (moved), perhaps more.
- Integrate in tor-browser-build: is the toolchain ready? would it "just work" to change the firefox repo to point to this one?
- Be able to run tbb-tests (and make them green).
- Updater: will mcs/brade work on these?
- Onboarding: as described in #28822 (moved), this needs to be ported, since onboarding (bootstrapped) extension is not there anymore. I could take a look at this one.
- Onion security expectations: a couple of patches, depending on availability, perhaps pospeselr could work on these?
- Decide what to do with patches from #28711 (moved).
- Verify 'might not be needed' patches.
- Backport https://bugzilla.mozilla.org/show_bug.cgi?id=1330467 or wait if it's included in 68.
- Android: should we do this after desktop patches? here or in a separate ticket?
== [rebased]
724044f7deae Bug 28044: Integrate Tor Launcher into tor-browser 7b9d68ac9f9a Bug 28369: stop shipping pingsender executable b5ce43598da9 Bug 27503: Disabling accessibility on Windows breaks screen readers 350fb9de802c Bug 25658: Replace security slider with security level UI 784829dd21ce Bug 29120: Enable media cache in memory 4791a84c69f2 Bug 28885: notify users that update is downloading 0db452ba7d90 Bug 12885: Windows Jump Lists fail for Tor Browser 9a64fd2b6786 Bug 25702: Update Tor Browser icon to follow design guidelines ---[Notes: app.update.download.backgroundInterval is not there anymore; this changes onboarding assets] 000879d8c0d6 Bug 27623 - Export MOZILLA_OFFICIAL during desktop builds cf13c54c3725 Bug 26146: Spoof HTTP User-Agent header for desktop platforms ---[Move to pref overrides?] ae2a89c7e1e7 Bug 26048: potentially confusing "restart to update" message ---[Did not find a simple way to do this with Fluent, so reused another DTD entitity that is still there...] 6bcc9d3aea2d Bug 24056: Use en-US strings in HTML forms e8833081b428 Bug 27082: enable a limited UITour f6cfb16dcbab Bug 26514 - intermittent updater failures on Win64 (Error 19) dcb5386e668d Bug 26353: Prevent speculative connect that violated FPI. 898b402c2458 Bug 26045: Add new MAR signing keys 1ef28dca0a8f Bug 21537: Tests for secure .onion cookies d5cf5a3f1e89 Bug 21537: Mark .onion cookies as secure 97237186c6b5 Bug 22548: Firefox downgrades VP9 videos to VP8. 0a793927d7b0 Bug 23104: Add a default line height compensation 8ccd532c1007 Bug 21830: Copying large text from web console leaks to /tmp 75ea009946e5 Bug 21321: Add test for .onion whitelisting 7fe43a8ecdeb Bug 21431: Clean-up system extensions shipped in Firefox 52 a347487a1d1e Bug 16285: Exclude ClearKey system for now ---[There's a new MOZ_EME_WIN32_ARTIFACT, it should not be enabled.] 7162a6dd03b1 Bug 21907: Fix runtime error on CentOS 6 ---[It seems CentOS 6 is supported until Nov 2020, so this is still needed] e92cd56ed887 Bug 21849: Don't allow SSL key logging 7586806f59e4 Bug #5741: Prevent WebSocket DNS leak. ---[Uplifted, but there are bugzillas 1470411, 1361337, 896206, ...] 08be04a55490 Bug 14970: Don't block our unsigned extensions 169496549650 Omnibox: Add DDG, Startpage, Disconnect, Youtube, Twitter; remove Amazon, eBa.. ---[Forcing legacy xml in Desktop, instead of new webext] 93c05f56d671 Bug 16940: After update, load local change notes. --- Moved AboutTBUpdate.jsm to AboutTBUpdateChild.jsm, extending ActorChild (followed pattern used internally). This seems to be loading blank about:tor pages and about:tbupdate on startup. Perhaps some missing prefs? c6843493db5a Bug 21724: Make Firefox and Tor Browser distinct macOS apps 85334a982084 Bug 18912: add automated tests for updater cert pinning cc8dc68c600d Bug 11641: change TBB directory structure to be more like Firefox's 055498dc6885 Bug 9173: Change the default Firefox profile directory to be TBB-relative. 1a096da1e527 Bug 19890: Disable installation of system addons 2828385c7e2a Bug 19273: Avoid JavaScript patching of the external app helper dialog. 1f1780afa6c9 Bug 18923: Add a script to run all Tor Browser specific tests e0bf4a7a6e8c Regression tests for #2874: Block Components.interfaces from content 86bab99e16bf Bug 18821: Disable libmdns for Android and Desktop 742d0c9459c8 Bug 18800: Remove localhost DNS lookup in nsProfileLock.cpp 6a3afe512b84 Bug 18799: disable Network Tickler 1e91414fac09 Bug 16620: Clear window.name when no referrer sent 190e9e67f67e Bug 16441: Suppress "Reset Tor Browser" prompt. 04c01de92591 Bug 14392: Make about:tor behave like other initial pages. 548e81f875d1 Bug 2176: Rebrand Firefox to TorBrowser 5d5e7469b211 Bug 18995: Regression test to ensure CacheStorage is disabled in private brows.. d5b09650a06f Regression tests for "Omnibox: Add DDG, Startpage, Disconnect, Youtube, Twitte.. 2a7fa9ec659c Regression tests for TB4: Tor Browser's Firefox preference overrides. 1f9d6925f3d3 Regression tests for Bug #2950: Make Permissions Manager memory-only 61d4f80ab517 Bug 12620: TorBrowser regression tests folder 5f4a2625e699 Bug 14631: Improve profile access error msgs (strings). 568ef8aa5967 Bug 14631: Improve profile access error messages. b6da0f584d7b Bug 13028: Prevent potential proxy bypass cases. 2eb11a17e593 Bug 16439: remove screencasting code. --- [Investigate, is 'secondscreen/RokuApp.jsm' and 'secondscreen/SimpleServiceDiscovery.jsm' only used in Android now? or is there a different mechanism for desktop?] d243f5affabc Bug 2874: Block Components.interfaces from content --- [Now done via pref.] 38dc17f97c8a Bug 12974: Disable NTLM and Negotiate HTTP Auth --- [Uplifted, but we might still want this.] da77d1390e7b Bug 10280: Don't load any plugins into the address space. 1380ffc86035 Bug 8312: Remove "This plugin is disabled" barrier. 07fe87a26867 Bug 3547: Block all plugins except flash. 3bdb29524b80 Bug 26321: New Circuit and New Identity menu items 05c7fdec8e51 Bug 28640 - Try showing the homepage after the Distribution loads e6f469366c70 Bug 28640 - Reload distribution preferences on update aa6704c5c5a7 Bug 28640 - Uninstall torbutton in the user profile on Android d9446943c73a Bug 25013: Add torbutton as a tor-browser submodule 8c146581a741 TB4: Tor Browser's Firefox preference overrides. 47aefe747950 TB3: Tor Browser's official .mozconfigs. --- [commented out --disable-maintenance-service in .mozconfig, only for Windows?]== [TODO updater -> Pearl Crescent?]
f3f4fcf25d43 Bug 13379: Sign our MAR files. 657ffb296498 Bug 4234: Use the Firefox Update Process for Tor Browser. 477911c58292 Bug 19121: reinstate the update.xml hash check b5be160897af Bug 19121: reinstate the update.xml hash check 29951f9c779c Bug 18900: updater doesn't work on Linux (cannot find libraries) 60e7b5b78e87 Bug 13252: Do not store data in the app bundle== [TODO onboarding -> to be ported in 28822 - Note: 25702 also touches some onboarding file]
d92980f74016 Bug 29768: Introduce new features to users 523a66a1affa Bug 27486 Avoid about:blank tabs when opening onboarding pages. ee4f40d85a8b Bug 26962 - implement new features onboarding (part 1). 98b1707f1930 Bug 26961: New user onboarding.== [TODO onion security expectations]
988d41acfaca Bug 26456: HTTP .onion sites inherit previous page's certificate information 651e4ef7de3e Bug 23247: Communicating security expectations for .onion== [TODO waiting for 1330467 to reland]
f47cd2fb5288 Bug 21569: Add first-party domain to Permissions key ---[https://bugzilla.mozilla.org/show_bug.cgi?id=1330467] 3298251467df Bug 26670: Make canvas permission respect FPI ---[https://bugzilla.mozilla.org/show_bug.cgi?id=1330467]== [TODO not landed firefox patches from #28711 (moved) - update and do 'try' run?]
7afe16fd6d27 Bug 1474659 Part 2 - Add dedicated AllocKinds just for ArrayBufferObjects. r?s.. ---[GC code changed, would be good to update patch and do 'try' to check breakage] 2beafe9bd417 Bug 1474659 Part 1 - Add support to EnumSet for more than 32 values. r?sfink ---[Changes for this part seem not to be needed anymore]== [DROP not needed]
e9b8de8c8d6a Updating Torbutton to 2.1.9 9e2fca791f62 Pull in latest Torbutton code 19963a38e431 Pulling in new Torbutton release 2170968a8e1e Pulling in latest Torbutton code (2.1.6) 76c0ea886161 Pull in latest Torbutton code 1b10f5a8d841 Picking up latest Torbutton changes e20342fe5080 Update Torbutton to 2.1.4 67bbd0703b2d Import latest Torbutton commits 44d088130f66 Picking up latest Torbutton commits 67eb148067ba Update Torbutton submodule 40212baaea92 Revert "Bug 29445: Enable support for enterprise policies" 6643872049b8 Bug 29445: Enable support for enterprise policies 934d12f1de17 Include <cstring> for memcmp in certverifier/Buffer.cpp== [DROP? might not be needed -> check]
8ee45ee52355 Bug 27411: Security Slider breaks on Windows --- [Message passing to noscript webext was changed, might not be an issue now] 65bbebea18a8 Bug 14716: HTTP Basic Authentication prompt only displayed once --- [Could not reproduce, seems to be working now without the patch] 1711b3160e3b Bug 26381: about:tor page does not load on first start on Windows and browser.. 09f0faa4dea6 Bug 26381: about:tor page does not load on first start on Windows ---[In case this is still needed, squash?] 7d0bb93e5c4b Bug 24398: Plugin-container process exhausts memory ---[This was needed because of 24052, but we now have mozilla 1412081 -> check] 603397b01823 Bug 27597: Package dom_bindings_test only with tests enabled 0ce33b09b33f Bug 27597: Package layout debugger interface only if tests are enabled 1d2a9f3f1e8e Bug 29180: MAR download stalls when about dialog is opened ---[Previous logic of pause/restart download is not there or changed, should check if we are good without this patch.]== [DROP uplifted]
41f620e4a0a3 Bug 13398: at startup, browser gleans user FULL NAME (real name, given name) f.. 091b41ec2465 Bug 21787: Spoof en-US for date picker 34063061f825 Bug 26540: Enabling pdfjs disableRange option prevents pdfs from loading c894688d4924 Bug 25909: disable updater telemetry== [DROP included in 68]
d3072ff444c2 Bug 1548634 - Update the default letterboxing behavior to use stepped ranges r.. 120eaaa6cff8 Bug 1407366 - Part 5: Reset the Zoom in browser_bug1369357_site_specific_zoom_.. 8efd394db5e7 Bug 1407366 - Part 4: Adding a test case for testing letterboxing. r=johannh 8aea5fe0df54 Bug 1407366 - Part 3: Implementing the window letterboxing. r=johannh 9d0e645be52e Bug 1447592 Do not reset the Spoof English pref after disabling Resist Fingerp.. dcaabab69579 Bug 1407366 - Part 2: Rearrange RFPHelper for expansion r=johannh 32eb8a433e88 Bug 1407366 - Part 1: Rename the LanguagePrompt.jsm to RFPHelper.jsm and chang.. 6d6441e5a79b Bug 1441327 - Allow for seccomp filtering of socket(AF_INET/AF_INET_6) calls o.. 78c2ae1d57f7 Bug 1463509 - SOCKS support for Alternative Services r=valentin fa1eb8174ad6 Bug 859782 - Firefox cannot start without /proc (chroot). r=sfink,evilpie,jld 084a76168f4d Bug 1470156 - Part 2: Fixing the crashing problem when using an invalid charac.. fa942f8b1bb6 Bug 1470156 - Part 1: Adding a test case for reassuring mozilla::OriginAttribu.. 6e1290cfa0fc Bug 1473247 - Part 2: Add a test case for making sure that IP addresses can wo.. 754ba4bf02fb Bug 1473247 - Part 1: Fixing the issue that the IP addresses won't be set for .. ec18e05896a2 Bug 1459089 - Don't use OS Locale when resistFingerprinting is enabled. When t.. 9fc79a537c6a Bug 1455165 - Filter external apps out if needed. r=mcomella 68220bda3a25 Bug 1474306 - Fix typo in the extension optionsType handler. r=Mossop 7992cd827ac0 Bug 1459420 - HLS Player doesn't use the centralized Proxy Selector r=mcomella.. 7a794f5f8f1d Bug 1500906 - Suppress FileUriExposedExceptions when launching helper apps. r=.. fa49a114636c Bug 1484472 - Avoid FileUriExposedException in ExternalIntentDuringPrivateBrow.. 4eb162e6193a Bug 1450449 - Part 5: Disable file:// URI checks for downloaded files and laun.. 3aa1d282f2df Bug 1450449 - Part 4: Starting from Nougat, install updates via content:// URI.. 14a82cb003c0 Bug 1450449 - Part 3: Starting from Nougat, share images via content:// URIs. .. aac358bdcb7f Bug 1450449 - Part 2: Use content:// URI for capturing images from FilePicker... 85d06450d978 Bug 1450449 - Part 1: Add FileProvider. r=jchen e068c7bad5c4 Bug 1480079 - Add REQUEST_INSTALL_PACKAGES permission for all builds; r=jchen 27b6ce1f2833 Bug 1504159 - add test to verify we can save a mixed content image from the co.. f2e586f93c2f Bug 1504159 - use TYPE_SAVEAS_DOWNLOAD for data saved through nsIWebBrowserPer.. e806657ea618 Bug 1487263 - set requesting principal for macOS drags, r=mstange ea930c8e6643 Bug 1473507 - fix crash in nsILoadInfo::GetOriginAttributes when passing no pr.. bb22e0dc14f0 Bug 1473509 - store principal information with the URIs to avoid having to loc.. b2fc4a61addb Bug 1469916, r=ckerschb,jkt bff72e9e644a Bug 1479311 - Don't attempt finding and highlighting a tab's base domain withi.. 17698434a0b4 Bug 1448305 - Fall back to the memory cache when a shortcut is created. r=JanH 9540e5fdef8e Bug 1448305 - Avoid disk cache for icons of TwoLinePageRow in private tabs. r=.. 15b3db497834 Bug 1448305 - Avoid disk cache for icons of private tabs in the TabsLayoutItem.. 9dfbafa2b516 Bug 1448305 - Avoid disk cache for icons of private tab's session history. r=J.. 0b1088652aaf Bug 1448305 - Avoid disk cache for icons of private tabs. r=JanH f6b82e518ce3 Bug 1441345 - Force the use of the Linux phishing list on Android. r=dimi, a=p.. 976a4cbc2b1b Bug 1458905 - Update to FreeType 2.9.1. r=jfkthame 861997889d84 Bug 1448014 - avoid needless flattening in AndroidDecoderModule; r=jesup 74855fb47cf5 Bug 680300 - Part 3: Make the client.navigate() not to reference the baseURL i.. 3cd98a889ec7 Bug 680300 - Part 2: Add a test case for ensuring no error reporting when load.. 8d4eb7e0ef61 Bug 680300 - Part 1: Stopping reporting errors when loading an unknown externa.. 676b2190104d Bug 1472618 - Make navigator.platform return "Win32", even on Win64 OS. r=peterv eec5a116ea35 Bug 1463936 - Set default security.pki.name_matching_mode to enforce (3) for a.. 062853dd3527 Bug 1503354 - Disable background HTTP response throttling for causing visible .. 118d51a33bdf Bug 1474626 - fix timestamp test and values, r=rpl defb067c6cb0 Bug 1471959 - leave keaGroupName and signatureSchemeName undefined if value is.. a0d6857913c4 Bug 1470516 - remove or fix localized values in securityInfo, r=rpl e274fb6e4a68 Bug 1464481 - fix and test crash when getting registered channelwrapper, r=kmag 3c0773d98af7 Bug 1322748 add securityInfo to webRequest listeners, r=keeler,rpl a91cf4cd69fd Bug 1322748 add ability to get registered channelwrappers, r=kmag cf3c165c7d22 Bug 1516642 - Add a function declaration for arc4random_buf in expat. r=peterv fa7548c07486 Bug 1542309 - Set firstPartyDomain to public suffix if getBaseDomain fails. r=..== [TODO android]
43582f5809cf Bug 29859: Disable HLS support for now 34e52459b150 Bug 29982 - Force single-pane UI on Tor Preferences 6cba968b5a7e Bug 30136: Use 'Tor Browser' as brand name on mobile, too 321bba05d203 Bug 30239 - Render Fragments after crash 22e72c1e7716 Bug 30214 - Kill background thread when Activity is null 75dcaffedcee Bug 30086 - Prevent Sync-related crashes on Android 545db499b0c7 Bug 28622: Update Tor Browser icon for mobile b563b207c38c Bug 29238 - Prevent crash on Android after update 323ebe2ca2d4 Bug 28329 - Part 4. Add new Tor Bootstrapping and configuration screens 79c9b8acba0b Bug 28329 - Part 3. Remove OrbotActivity dependency dd3f22090e52 Bug 28329 - Part 2. Implement checking if the Tor service is running fe784a8284e5 Bug 28329 - Part 1. Add new Tor resources a195c7180350 Bug 28507 - Don't call Push and Sync services during Sanitize f6f8859b3891 Bug 26690: Port padlock states for .onion services to mobile 722afbb6c5c6 Bug 28051 - Use Orbot's notification-builder wrapper class 8436a9443fde Bug 28051 - Stop the background service when we're quitting 464c03696814 Bug 28051 - Use our Orbot for proxying our connections a0b886f177d8 Bug 28051 - Launch Orbot if it isn't running in the background a5e24cd02c8e Bug 28051 - Integrate Orbot and add dependencies 39316ccaedbb Bug 28507: Implement fallback to delete private data in the browser startup b0b54ec7c7a0 Bug 28507: Add prefs that allow the browser to delete browsing history by defa.. 894c64799812 Bug 28507: Parse a set of strings in Android Set Preferences e860dd77665d Bug 27125 - Move localized Tor Browser for Android strings into separate file 06f9c75d7725 Bug 27111: Configure tor browser for mobile to load about:tor 28a2ea997ba0 Bug 28125 - Prevent non-Necko network connections 878c9536c888 Bug 27472 - Export MOZILLA_OFFICIAL during Android build 826e0934adb4 Bug 27400 - Target Android API 26 cb63b9e84081 Bug 27473 - Change app name on Android for Alpha version 6177eef07093 Bug 25696 - Design of alpha onboarding for Tor Browser for Android 8e272a5204e2 Bug 25696 - Implement alpha onboarding for Tor Browser for Android 2c08bd6d5df5 Bug 24796 - Comment out excess permissions from GeckoView 1bd41f99acd1 Bug 26825 - Delete RECEIVE_BOOT_COMPLETED permission 9023dea11376 Bug 26826 - Disable tab queue and delete SYSTEM_ALERT_WINDOW permission 350766eecb63 Bug 25906 - Imply false both Adjust and Leanplum configure options 9312fdf1ac53 Bug 27016 - Create proxy connection during image download bff13bd887c3 Bug 26528 - Don't allow Fennec to use UpdateService when installed through the.. 9b218c05cecc Orfox: hook up default panic trigger to "quit and clear" 80b18b10f092 Orfox: quit button added f1c13fc12685 Orfox: disable screenshots and prevent page from being in "recent apps" e96b86f7a6f6 Bug 25741 - TBA: Disable GeckoNetworkManager ffcc8cefd762 Bug 25741 - TBA: Adjust the User Agent String so it doesn't leak Android version 0e4ae4cbd11a Bug 25741 - TBA: top sites changed, used bookmarks icon temporarily. 6e806c3cb9ed Orfox: Centralized proxy applied to AbstractCommunicator and BaseResources. 433766efba25 Orfox: add BroadcastReceiver to receive Tor status from Orbot 41b333e3a9d3 Orfox: NetCipher enabled, checks if orbot is installed d073b92b20ba Bug 25741 - TBA: Neuter Firefox Accounts f96dc4c9ae4d Bug 25741 - TBA: Only include GCM permissions if we want them e8f22d0b795e Bug 25741 - TBA: Only include Firefox Account permissions if we want them 6299c89034d3 Bug 25741 - TBA: Always Quit, do not restore the last session a66bd47fd601 Bug 25741 - TBA: Disable all data reporting by default eebbebcb7010 Bug 25741 - TBA: Clear state when the app exits, by default 0820ddf07093 Bug 25741 - TBA: Do not import bookmarks and history from native browser by de.. 8839bac75801 Bug 25741 - TBA: Do not save browsing history by default 3531fb88ab73 Bug 25741 - TBA: Move CAMERA permission within MOZ_WEBRTC c910edef7833 Bug 25741 - TBA: Conditionally require *_LOCATION permissions d2fa5a6928bf Bug 25741 - TBA: Conditionally require WIFI and NETWORK permissions f90df9c52d2d Bug 25741 - TBA: Disable QR Code reader by default b6584da608bf Bug 25741 - TBA: Disable the microphone by default 92959f9de719 Bug 25741 - TBA: Disable telemetry and experiments e4ea4bf5fbfd Bug 25741 - TBA: Remove sync option from preferences 7122131c88d9 Bug 25741 - TBA: Add mobile-override of 000-tor-browser prefs 39979b254f2c Bug 25741 - TBA: Add default configure options in dedicated file d9b6b1116a67 Bug 25741 - TBA: Do not register Stumbler listener at start up 765b0fb6cdee Bug 25741 - TBA: Add an AppConstant for TOR_BROWSER_VERSION de5d9f3c4f83 Bug 25741 - TBA: Disable features at compile-time 2e75b845a93f Bug 25741 - TBA: Add mozconfig for Android and pertinent branding files. b4aef2103fef Bug 25741 - TBA: Move GCM Push prefs within preprocessor guard db59d0ef5a5b Bug 25741 - TBA: Exclude unwanted Stumbler tests
Replying to acat:
[snip]
Okay, let me start by replying to the ToDos:
Some TODOs:
[snip]
- Integrate in tor-browser-build: is the toolchain ready? would it "just work" to change the firefox repo to point to this one?
No, it is not yet. The current branch is
linux_esr68_v3in my repo (with probably a bunch oflinux_esr68_v$to follow) but there are still build requirements missing (nodejsis the next one to add)[snip]
- Updater: will mcs/brade work on these?
Yes.
- Onboarding: as described in #28822 (moved), this needs to be ported, since onboarding (bootstrapped) extension is not there anymore. I could take a look at this one.
Sounds good.
- Onion security expectations: a couple of patches, depending on availability, perhaps pospeselr could work on these?
What are the issues here? Could you file a new bug to track that work?
- Decide what to do with patches from #28711 (moved).
Ideally we could backport them and have some knowledgeable Mozilla person looking over the result.
[snip]
- Backport https://bugzilla.mozilla.org/show_bug.cgi?id=1330467 or wait if it's included in 68.
It seems we need to backport them, alas. :( They got a minus for beta (that is esr68) inclusion.
- Android: should we do this after desktop patches? here or in a separate ticket?
We should do it in parallel or better: not blocking the rebasing work on desktop patches. sysrqb will pick this up and probably decide whether to use a child bug or have the rebase in this ticket.
[snip]
Onion security expectations: a couple of patches, depending on availability, perhaps pospeselr could work on these? What are the issues here? Could you file a new bug to track that work?
I left them for the end after a quick look, but I just tried again and it seems not so much work. So I can work on these after working on onboarding.
I added some more patches in https://github.com/acatarineu/tor-browser/commits/30429+1. It's onboarding and onion security expectations ones. I think there is one from the "onion security expectations" that is not needed anymore. Changes in the list of commits:
== [rebased]
+ d92980f74016 Bug 29768: Introduce new features to users + 523a66a1affa Bug 27486 Avoid about:blank tabs when opening onboarding pages. + ee4f40d85a8b Bug 26962 - implement new features onboarding (part 1). + 98b1707f1930 Bug 26961: New user onboarding. + 651e4ef7de3e Bug 23247: Communicating security expectations for .onion== [onboarding]
- d92980f74016 Bug 29768: Introduce new features to users - 523a66a1affa Bug 27486 Avoid about:blank tabs when opening onboarding pages. - ee4f40d85a8b Bug 26962 - implement new features onboarding (part 1). - 98b1707f1930 Bug 26961: New user onboarding.== [onion security expectations]
- 988d41acfaca Bug 26456: HTTP .onion sites inherit previous page's certificate information - 651e4ef7de3e Bug 23247: Communicating security expectations for .onion== [DROP? might not be needed -> check]
+ 988d41acfaca Bug 26456: HTTP .onion sites inherit previous page's certificate informationReplying to gk:
Replying to acat:
- Android: should we do this after desktop patches? here or in a separate ticket?
We should do it in parallel or better: not blocking the rebasing work on desktop patches. sysrqb will pick this up and probably decide whether to use a child bug or have the rebase in this ticket.
I opened #31010 (moved) for Android. I'll update that ticket and let this one remain focused on the desktop and shared code patches.
-
Replying to acat:
== [rebased]
Here are my comments, the hashes are from your
30249branch:9510c9416ddd35a016ee2074dd58f927d97246c7 - not okay
no need to comment out the maintenance related option, just remove it (and while we are at it, please remove the other unused options as well)
please add
--enable-proxy-bypass-protectionto all mozconfigs4aec090126cd79289628b4403366c176714a4c77 - okay 23b8e34d8fa64affdb265911ff586d8babf1a119 - should we not point to the latest Torbutton commit (especially as we have removed all the other Torbutton commit updates (rightly so))? otherwise okay
22088a63f01c5526aedcafb3232f211a78ec9106 - okay (mobile) 912ed4b07281ceebb67726b1785d12b37ab95b12 - okay (mobile) 493b664f0fd1867559e51d6015c784e7c21a3259 - okay (mobile) 34126c910efab560ff0d6923437c50758f8dc03f - we should merge that with the patches for #10760 (moved) I think (otherwise okay) bdf970dcdeeb276eccb9538b0d86dfee07ec5776 - okay 5d3e47ad112820208dd894aaeb51497ab37caf65 - okay 27fa31d4350e4248b0bfb35c51918955629a112a - okay b7c8b9e0b641cdfeef8ac2adc5188c3411110374 - okay f9957d4fd3f164be68adcd32206c09cb6f59a16d - could you do a
git commit --squashhere with commit 4aec090126cd79289628b4403366c176714a4c77; please add the Trac bug number to this pref flip so we get easily the context and move the pref flip to the fingerprinting section of000-tor-browser.js55367b7e2edbaf55f1142140b2cb9ec9b9247bec - okay 9909eeb95cea7fa84bcd45bcdddd0c4c22d83e17 - okay 45072c2fea6a535a712ac0888f12f881393cccbd - (mcs/brade should have a second look at this patch) I wondered how we ever settled at aconst char16_t*first given the signature ofFormatStringFromName()but I agree that usingconst char*seems more intuitive.+ ProfileStatus profileStatus = PROFILE_STATUS_OK;<- do we need to assignPROFILE_STATUS_OKhere, wouldn't it be enough to just doProfileStatus profileStatus;given that you do+ aProfileStatus = PROFILE_STATUS_OK;and don't assign that inSelectUpdateProfileeither?There is in
nsToolkitProfileService:GetProfileByDir(lf, localDir, getter_AddRefs(profile)); if (profile && mIsFirstRun && mUseDedicatedProfile) {Should we have the usual
CheckProfileWriteAccess()call here as well or did you think this is a non-issue as we are there "generally" from an app initiated restart, as the comment says.a8c48df07cc505bd45c764d223f6ff01de738f31 -- indentation (keep original):
+ nsresult rv = GetOverrideStringBundleForLocale( + aSBS, uriString.get(), userAgentLocale.get(), aResult);// Build Torbutton file URI string by starting from the profiles directory.<- It's not the profiles directory anymoreMaking
dirProvideraRefPtrseems okay to me.Should
general.useragent.localebeintl.locale.requested(I guess this should have been the case for Tor Browser 8 already, but well...)2d2a55296e255f9b502d0aa9eb70d4822a1bdd0e - okay 9ec12a9075a87f1a1d446200ca4f64f88ef8466f - okay [we should upstream that one, bug 967812 has code parts] 5b585b0633d5359504542b0fb05b599cb879a883 - okay 630b395081dc68828d8f55ea41c71297b123bb87 - okay 73e8dc78ddb3caf7b7dde8df2ea15dd9898cccac - not needed, see bug 1434772 8295e48bb7d948fd8e85cbc9f5ffe7e9e0d9ea6b - okay d9cc80636e3dfcdf28bb368b416508402e9b9f9a - okay 237124a540877734cc15a60de613f29b064f3799 - okay a4282bea59a32dba0a17f3d31e6f7f6094a98eac - not okay
There is no docshell/test/mochitest.ini in esr68 and we should not add one.
+const kTestPath = "/tests/docshell/test/";needs adaptation to new pathWhy
GetComputedReferrer()and notGetOriginalReferrer()? Could you add an explaining comment here?a3acbf09d562e14f9e67f91db7a12bd185058b18 - not needed anymore with
--enable-proxy-bypass-protectionset 4bd0f7037b7a89a9ec95e540f58ebcb5cd74bd07 - okay 5011133b08cfdfbb5e6ad3ff03b1d230bb206608 - not okayThe
AndroidCastProviderpart needs to get ripped out in components.conf as well, as things might break otherwise if we don't include it in moz.build (which we should not and your patch makes sure)You are not including
components.confin libmdnsmoz.buildinstead of patching it as you did in the provider case; we should have the same approach inboth cases (I am fine with just themoz.buildone if we think that's enough)4c93f2c748ba5f6f00a5d5b197716a899798aea2 - okay 6684321fb901ef1546391e786fca8d2e3ef8b5fc - okay a2ade001bdcffa4469baa3ac88a5db19ca8c6e52 - okay fd0570fb485cdb5d0327e51745b1ad59ef284240 - okay ce0db56b09a3b448508cc619418d84220f8e9acd - not okay
What is the reason to patch GetUser$Directory now while we did not have done so in the esr60 patch? (I guess we should follow Mozilla here and if not, please add some comment/explanation for that deviation)
18a6e88f2c28c4439858476334388de9208ad447 - okay d9b20bc60c8167c0ab32b93623beb8968f4b5f07 - test is probably not working anymore as we don't have static pins for #29811 (moved); we should either drop it or fix #29811 (moved) and test that the test is still working then (or working then again) (I think we should go the fixing route here :) )
2943fd7440cf90f613ff3a96180cd7d71a3ca483 - okay 9d8ca4380e947c2097d0fd3554b1f3dad20de634 - not okay
+ removeMessageListener("AboutTBUpdate:Update", this); + removeEventListener("pagehide", this, true);The respective
add$Listenersgot lost? Or do we get them now via theLEGACY_ACTORSobject? (Does not seem to be the case for me as nothing changed in that regard if you look at the restructuring of theAboutReaderpage) Maybe those listeners were not needed in the first place?Does the restructured code not need
isAboutTBUpdate()anymore?We put a lot of the code behind
TOR_BROWSER_UPDATE, should the inclusion of the .jsm file then be behind that as well?I guess compilation breaks if we keep
BrowserContentHandler.jsmwhere it is? (mcs/brade should have a second look here)468ccd77da35cb0ea0a3419619d42ce810d00238 - not okay
Where are all the search engines coming from in that patch which we did not have in esr60? I think we don't need those.
To keep the patch small we should just patch the search engines entries of the locales we ship. This holds for mobile as well and we probably should address #30017 (moved) (and maybe #30606 (moved)).
If you look at https://hg.mozilla.org/integration/autoland/rev/111b88dd28d6 you see that the search plugins are converted to WebExtensions. I wonder
a) what is the issue with not following Mozilla here? I.e. I am a bit wary of deviating from their approach as we want to keep the differences small if possible.
b) that if a) is indeed a thing we should probably make sure to comment that in the code and we might want to just not add the search extensions in the first place to minimize possible weird interactions.
4cad2e391d96b7e1c197de5c37408ecd371d6aff - okay c1ff0d730d048e115b7d6c551b95a58a50ae8827 - okay 294445810e60e08f00b8fdf74937664eff5a925d - okay da608e51ef73beb1f77fd6852c60b7db270ea31a - okay 8908dc936bd0ae6f497533d5a69835b81eb242e2 - okay c55aea4b373ac4500d1539a8a4c79009fc9c0076 - okay b0a1cddc170a51d2b97a7b9c52b4294927c599b9 - okay [upstreaming??] 1ee3bc541b96729ecb3c41df2fdf2c3bdeb278a4 - okay 3490265e4331eb77a20eeb169776ddf55d891ecd - okay db3a5b7d339bad673a01804ff83fd2f8a79f46d3 - okay 371f840f3096a9f1cf03d448df0d62569b7a52b9 - okay [upstreaming??] 3bd36833ce14d3a9ad9d70ac4e6d4f10d5cebdfa - okay [upstreaming??] 3b6785c5de66cb0f78faf945859e090633e09525 - okay df6140a3563f78252721cc3b53bc07ac4f05ca0e - okay 829a448ca35a6323ce5d7982738c54424292d502 - [probably upstreamed, see 1561636] 73569f04f387efd7d7b9e06e5dfadbdfba0a3ee5 - okay 9d9ca4e994b7a9713153f56a5f93ebc9fa2ed939 - okay 83459e103450f80b471ecda459a2017857e5d26c - okay 04d72c21af73359698ccd9f5c6808adb36be816c - not okay
I think we should squash it with the general
000-tor-browser.jscommit (4aec090126cd79289628b4403366c176714a4c77)ba383936028e955cef2ced0f98d2f90ca39564de - not okay
We should fold that into 9510c9416ddd35a016ee2074dd58f927d97246c7; additionally it's worth solving #27493 (moved) while we are at it.
9f05eedab888b33f83e97bb3cd870ecb5174ea41 - not okay
> .../fxmonitor/content/img/tor-watermark.svg | 6 +++It seems you added the .svg to the wrong dir?
content/branding/horizontal-lockup.svgis missing inbrowser/branding/alpha/content/jar.mnor maybe we should just remove it in the other series? Or replace it with an own icon fornewInstallPage.html? Glancing at bug 1518632 I am tempted to just remove that .svg file.We should remove the dead app.update.download.backgroundInterval.
4a8236665a3250705dbab46f6b74a6c0eac1af2b - not okay
Let's squash it with 4aec090126cd79289628b4403366c176714a4c77
5269df12586ac7e4e45803a0f1b8c15da9ef529a - looks okay; I guess you need to add the EXTRA_PP_JS_MODULES because the build would break otherwise? If so, do you know why? mcs/brade should have a second look here.
d945c68acc68a834f26a89973ab5f728fdbd3e38 - okay d84a2fb95136a1728b621a9d4cbe979389db48a0 - not okay
please include the fixup for the manual page as specified in eb5d5dfaae93805baee9e84039e95fca74f9cce2
c08b58e1d7062b732e39caca8a37b2a52af47249 - not okay, not needed due to 1520177 65ec28479828c9bf80f73c0d3d1d5817177c646e - okay 939c662e69f3cae14e8f5e31b5a75eb6b20fb214 - okay (we should have this early on on our final branch so it is easier to bisect problems and still have a working Tor Browser experience; that probably includes moving the patch exempting our signed extensions to an early place in the branch as well)
== [TODO waiting for 1330467 to reland] {{{ f47cd2fb5288 Bug 21569: Add first-party domain to Permissions key ---[https://bugzilla.mozilla.org/show_bug.cgi?id=1330467] 3298251467df Bug 26670: Make canvas permission respect FPI ---[https://bugzilla.mozilla.org/show_bug.cgi?id=1330467] }}}
We need to backport 1330467 and look closely at the fallout. Might need some fix-ups to take this into account.
== [TODO not landed firefox patches from #28711 (moved) - update and do 'try' run?] {{{ 7afe16fd6d27 Bug 1474659 Part 2 - Add dedicated AllocKinds just for ArrayBufferObjects. r?s.. ---[GC code changed, would be good to update patch and do 'try' to check breakage] 2beafe9bd417 Bug 1474659 Part 1 - Add support to EnumSet for more than 32 values. r?sfink ---[Changes for this part seem not to be needed anymore] }}}
Sounds like a good plan I think.
== [DROP not needed]
Looks good.
== [DROP uplifted] {{{ 41f620e4a0a3 Bug 13398: at startup, browser gleans user FULL NAME (real name, given name) f.. 091b41ec2465 Bug 21787: Spoof en-US for date picker 34063061f825 Bug 26540: Enabling pdfjs disableRange option prevents pdfs from loading c894688d4924 Bug 25909: disable updater telemetry }}}
Looks good. 41f620e4a0a3 is actually not needed anymore as the respective code got removed in Firefox 68.
== [DROP included in 68]
Looks good.
Trac:
Status: needs_review to needs_revision
Keywords: TorBrowserTeam201907R deleted, TorBrowserTeam201907 added 
Replying to gk:
mcs/brade: to not lose this in the previous wall of text: I thought it could be helpful if you'd had a second look at commits
45072c2fea6a535a712ac0888f12f881393cccbd
R.e. the
char16_tquestion: a while ago, Mozilla changedFormatStringFromName()to take aconst char *so the new code is good.R.e. gk's second comment: Kathy and I would prefer to always initialize
profileStatus(seems safer).R.e. gk's last comment about possibly adding an another call to
CheckProfileWriteAccess(): we think it is OK to skip this case.In the
ProfileErrorDialog()function, please reinstate thervcheck for the call tosb->FormatStringFromName()(I think it was lost during a previous rebase).In
nsToolkitProfileService::SelectStartupProfile()the first call toGetProfileByName()is wrong. We should not be passing a path to that function (it looks like the ESR60 patch is wrong too). Instead, once we obtainlf, we should callCheckProfileWriteAccess(lf). In other words, we can remove the entire chunk that contains the first call toGetProfileByName()as well as a call toCheckProfileWriteAccess(profile)and instead make the second call earlier.Also, Mozilla has made several changes to these files since Alex started the ESR68 TB rebasing work. Kathy and I would like to take another look at the patch after those changes have been accounted for.
9d8ca4380e947c2097d0fd3554b1f3dad20de634
It looks like event listeners are handled by via the
LEGACY_ACTORSobject. See the large comment near the beginning oftoolkit/modules/ActorManagerParent.jsm. If that is correct, we should remove theremoveMessageListener()andremoveEventListener()calls frombrowser/actors/AboutTBUpdateChild.jsm.R.e.
isAboutTBUpdate(), that is replaced by thematchesproperty within theLEGACY_ACTORSobject.R.e.
BrowserContentHandler.jsmbeing moved toEXTRA_PP_COMPONENTS: that was part of the #4234 (moved) patch.Otherwise, this looks good. We will need to test it once the other updater patches have been rebased.
5269df12586ac7e4e45803a0f1b8c15da9ef529a
This looks good to us, although testing is required. The reason the
EXTRA_PP_JS_MODULESchange appears in this patch is because a similar change is included in the #4234 (moved) patch (which was applied before this patch).-
Alex, we have a couple of questions about the rebasing process:
-
Do you want to rebase your rebased patches onto the tip of Mozilla's esr68 branch (or a recent tag) before Kathy and I do our rebasing work? I think that makes sense unless it turns out to be very time consuming.
-
Previous attempts to reorder the patches while rebasing have not gone well; it would be better if we preserved the order so for example the #4234 (moved) patch comes before the other updater-related patches such as the #16940 (moved) one. Is it OK if Kathy and I create a new branch with the rebased patches for the following tickets inserted in a similar place within the commit stream as for the ESR60 Tor Browser branches: #18900 (moved), #13252 (moved), #19121 (moved), #4234 (moved), #13379 (moved)? The only downside that I see is that if you are working in parallel with us someone will need to cherry pick some patches from our branch along with all of your patches to create a consolidated branch (or do something else magical with git).
-
Replying to mcs:
Alex, we have a couple of questions about the rebasing process:
- Do you want to rebase your rebased patches onto the tip of Mozilla's esr68 branch (or a recent tag) before Kathy and I do our rebasing work? I think that makes sense unless it turns out to be very time consuming.
I will do that, after I address the requested changes here. Since I assume this is blocking for you, I will try to do it soon.
- Previous attempts to reorder the patches while rebasing have not gone well; it would be better if we preserved the order so for example the #4234 (moved) patch comes before the other updater-related patches such as the #16940 (moved) one. Is it OK if Kathy and I create a new branch with the rebased patches for the following tickets inserted in a similar place within the commit stream as for the ESR60 Tor Browser branches: #18900 (moved), #13252 (moved), #19121 (moved), #4234 (moved), #13379 (moved)? The only downside that I see is that if you are working in parallel with us someone will need to cherry pick some patches from our branch along with all of your patches to create a consolidated branch (or do something else magical with git).
I think this is fine, I don't see any issue with cherry-picking the updater patches to a more up to date consolidated branch later, if needed.
Thanks for the reviews, GeKo, mcs/brade.
I took branch https://github.com/acatarineu/tor-browser/commits/30429+2 (which is the one resulting of #10760 (moved) changes) and addressed the review comments there, resulting on https://github.com/acatarineu/tor-browser/commits/30429+3. Then I rebased those commits on top of latest esr68 branch, that's https://github.com/acatarineu/tor-browser/commits/30429+4. And finally, reordered some commits to address some review remaining comments and also fixed the search engines patch (Omnibox: Add DDG, Startpage, Disconnect, Youtube, Twitter...), that's branch https://github.com/acatarineu/tor-browser/commits/30429+5. I left the search engine fix for the end, and then I realized that there were some conflicts with esr68 branch, so that's why I only fixed it in the last branch (30429+5).
While rebasing to esr68 branch (30429+3 -> 30429+4) there were several conflicts, although most of them were due to style changes. But I think it would be good to review them, for example: - Bug 21849: Don't allow SSL key logging (
gyp_vars['enable_sslkeylogfile']was added)I also realized that in
Bug 8312: Remove "This plugin is disabled" barrier.there is adoc.getAnonymousElementByAttributebut I do not see any defineddocvariable. It's the same in 52, but I think in the previous version there was. So I added thedocdefinition, although I would have to test this to make sure if it works as intended.A question before comments: would it also make sense to try to upstream Bug #5741 (closed), with a different commit name and behind the --proxy-bypass-protection flag?
Some comments:
please add --enable-proxy-bypass-protection to all mozconfigs Do you mean
.mozconfig-mac,.mozconfig-asan,.mozconfigand.mozconfig-mingw? If so, isn't it already there?23b8e34d8fa64affdb265911ff586d8babf1a119 - should we not point to the latest Torbutton commit (especially as we have removed all the other Torbutton commit updates (rightly so))? otherwise okay I think so, but I kept the submodule change in a separate commit since it's currently it's in my personal branch. Would it be ok to make the last
WIP torbutton submodule changecommit a fixup when we have an "official" torbutton "esr68" branch?a8c48df07cc505bd45c764d223f6ff01de738f31 -- indentation (keep original):
- nsresult rv = GetOverrideStringBundleForLocale(
-
aSBS, uriString.get(), userAgentLocale.get(), aResult);
mach clang-formatis changing this one, should we keep the original indentation anyway?There is no docshell/test/mochitest.ini in esr68 and we should not add one. Thanks. I also realized I removed I accidentally removed a test
[test_bug1507702.html], fixed that too.Why GetComputedReferrer() and not GetOriginalReferrer()? Could you add an explaining comment here? I think
GetComputedReferrer()is the one that preserves the current patch behaviour (e.g. takes into consideration referrer policy). Actually, when you mentioned I was not sure whether this was something intended. For example, with a noreferrer policy navigating through pages of the same origin will always clearwindow.name. But I saw #3414 (closed), so this actually seems to be the intended behaviour. I added a comment in the code.You are not including components.conf in libmdns moz.build instead of patching it as you did in the provider case; we should have the same approach inboth cases (I am fine with just the moz.build one if we think that's enough) Ok, I reincluded
components.confinmoz.build, and made the corresponding components.conf empty. Not sure if you meant this, or that we actually don't need to touchlibmdnscomponents.conf.What is the reason to patch GetUser$Directory now while we did not have done so in the esr60 patch? (I guess we should follow Mozilla here and if not, please add some comment/explanation for that deviation) Not sure if you mean changing the static calls by the non-static ones. If so, this is needed because the patch needs to make several
GetUser$Directorynon-static, and apparently in esr60 the static functions were called via object (I would expect C++ to at least warn if you call a static method via an object?).9d8ca4380e947c2097d0fd3554b1f3dad20de634 - not okay I guess compilation breaks if we keep BrowserContentHandler.jsm where it is? (mcs/brade should have a second look here) Yes, I think the PP is needed when it needs to go through the preprocessor (same as 5269df12586ac7e4e45803a0f1b8c15da9ef529a, we added a macro to check for a build pref).
It looks like event listeners are handled by via the LEGACY_ACTORS object. See the large comment near the beginning of toolkit/modules/ActorManagerParent.jsm. If that is correct, we should remove the removeMessageListener() and ?>removeEventListener() calls from browser/actors/AboutTBUpdateChild.jsm. True, did it. But I realized that now
onPageHidedoes nothing, which seems wrong. Can we also remove it? I'm not sure if the new patch changes some behaviour here, since I guess before onpagehide would stop updating the document?Where are all the search engines coming from in that patch which we did not have in esr60? I think we don't need those. To keep the patch small we should just patch the search engines entries of the locales we ship. This holds for mobile as well and we probably should address #30017 (moved) (and maybe #30606 (moved)). If you look at https://hg.mozilla.org/integration/autoland/rev/111b88dd28d6 you see that the search plugins are converted to WebExtensions...
If you mean the *.xml, I added those to reuse the old ones without converting them to WebExtensions. But perhaps you also mean that the list.json changes are a bit dirty, that we're changing too many engines. In any case, I also think moving to webextensions is the right thing to do, so I used the script in https://github.com/daleharvey/ffx-opensearch-to-webext to convert them (it's the script that the mozilla dev used). I had to modify the yahoo one, since it contained a system param which seems not to be supported:
{ "name": "hsimp", "condition": "purpose", "purpose": "system", "value": "yhs-007" }I also included 'Google' in the searchEngineOrder field, for some reason it was not respecting the order we currently have in esr60. We could perhaps also include other search engines there, and also not sure if the order prefs are needed anymore.
This is working fine the first time I open the browser, but unfortunately for some reason after restarting it the search engine icons disappear (although they still work). I'll try to find out why, but I think this should not be a blocker for a nightly. It might also be possible that it's some issue with my local build, so we can see if it reproduces.
I also removed the changes for mobile
list.json, as I think they were only addressing #29798 (moved), which was fixed in 65. The mobile search engines are currently set inBug 25741 - TBA: top sites changed, used bookmarks icon temporarily.. Should we change that and do all browser search engines in this patch, or is it fine to do it later for Android separately?As I said before, These changes were done only on 30429+5 branch.
ba383936028e955cef2ced0f98d2f90ca39564de - not okay We should fold that into 9510c9416ddd35a016ee2074dd58f927d97246c7; additionally it's worth solving #27493 (moved) while we are at it. Done, removed
mk_add_options MOZILLA_OFFICIAL=1and alsomk_add_options BUILD_OFFICIAL=1, since it's not used anymore.9f05eedab888b33f83e97bb3cd870ecb5174ea41 - not okay .../fxmonitor/content/img/tor-watermark.svg | 6 +++ It seems you added the .svg to the wrong dir? content/branding/horizontal-lockup.svg is missing in browser/branding/alpha/content/jar.mn or maybe we should just remove it in the other series? Or replace it with an own icon for newInstallPage.html? Glancing at bug 1518632 I am tempted to just remove that .svg file. We should remove the dead app.update.download.backgroundInterval. I reviewed the patch again and changed it a bit. First, is there any problem with just removing the
tor-watermark.svg? It's not used (I did that in the revision). Then, I realized that this patch actually was not changing the*/pref/firefox-branding.js, but just copying the release one over the alpha and nightly ones. But the release one is modified in one of the updater patches (#4234 (moved)), which has not been applied yet. So I left the */pref/firefox/branding.js untouched in this patch, and I plan to modify it when #4234 (moved) is applied. Or we could apply #4234 (moved) after this one, and do all the */pref/firefox/branding.js changes there, not sure. I also updated some locale files. For example, brand.ftl is now used extensively in the code, and has more strings. That means that I also needed to modify patch #2176 (closed) to replace brand.ftl -> tor-browser-brand.ftl, and to add the missing strings.