Fix torbutton for ESR45

domain-isolator.js has a couple of minor breakages in Firefox 45.

1. A new ReferenceError is thrown by things like let mozilla = mozilla || {};. We can change it to let mozilla = {};.

2. The method newSOCKSProxyInfo was available in our SOCKS proxy patch. Since we landed that patch with some minor changes, we should now make an equivalent call to built-in Firefox method newProxyInfoWithAuth.

added GeorgKoppen201604 TorBrowserTeam201604R component::applications/tor browser ff45-esr owner::gk priority::high resolution::fixed severity::critical status::closed tbb-6.0a5 type::defect labels

Trac:
Description: domain-isolator.js has a couple of minor breakages.

1. A new ReferenceError is thrown by things like let mozilla = mozilla || {};. We can change it to let mozilla = {};.

2. The method newSOCKSProxyInfo was available in our SOCKS proxy patch. Since we landed that patch with some minor changes, we can now make an equivalent call to built-in Firefox method newProxyInfoWithAuth.

to

domain-isolator.js has a couple of minor breakages in Firefox 45.

1. A new ReferenceError is thrown by things like let mozilla = mozilla || {};. We can change it to let mozilla = {};.

2. The method newSOCKSProxyInfo was available in our SOCKS proxy patch. Since we landed that patch with some minor changes, we should now make an equivalent call to built-in Firefox method newProxyInfoWithAuth.

Here's my patch, for review:

https://github.com/arthuredelstein/torbutton/commit/18466

(This patch will break torbutton for TBB/FF38, so we will likely need a separate branch.)

Trac:
Status: new to needs_review
Keywords: TorBrowserTeam201603 deleted, TorBrowserTeam201603R added

Trac:
Description: domain-isolator.js has a couple of minor breakages in Firefox 45.

1. A new ReferenceError is thrown by things like let mozilla = mozilla || {};. We can change it to let mozilla = {};.

2. The method newSOCKSProxyInfo was available in our SOCKS proxy patch. Since we landed that patch with some minor changes, we should now make an equivalent call to built-in Firefox method newProxyInfoWithAuth.

to

domain-isolator.js has a couple of minor breakages in Firefox 45.

1. A new ReferenceError is thrown by things like let mozilla = mozilla || {};. We can change it to let mozilla = {};.

2. The method newSOCKSProxyInfo was available in our SOCKS proxy patch. Since we landed that patch with some minor changes, we should now make an equivalent call to built-in Firefox method newProxyInfoWithAuth.

Summary: Fix domain-isolator for ESR45 to Fix torbutton for ESR45

Trac:
Keywords: N/A deleted, ff45-esr added
Reviewer: N/A to N/A

Kathy and I looked at c55c1ef497f7b2a97ff7c9b0af351a065df3b477 and 594b2bbc55a0bbc2f548356f44ea191d4e2466e2. Your changes look good. We have two questions: Can you give us some background on why statements such as let mozilla = mozilla || {}; were needed previously? Or maybe they were not needed?

I assume that the newProxyInfoWithAuth() change is what breaks in ESR38. Can we avoid the need for a branch by using a runtime check such as: if ("newProxyInfoWithAuth" in mozilla.protocolProxyService) ... ?

Trac:
Cc: N/A to brade, mcs

Trac:
Status: needs_review to needs_information

No easy way to do reviews in March anymore.

Trac:
Keywords: TorBrowserTeam201603R deleted, TorBrowserTeam201604R added

There is more we need to change for fixing Torbutton. I am working on an additional patch

Trac:
Keywords: TorBrowserTeam201604R deleted, TorBrowserTeam201604, GeorgKoppen201604 added
Priority: Medium to High
Parent: #15197 (moved) to N/A
Status: needs_information to assigned
Severity: Normal to Critical
Owner: tbb-team to gk

Trac:
Keywords: GeorgKoppen201604 deleted, GeorgKoppen201604 tbb-6.0a5 added

Part of the pain here is due to https://bugzilla.mozilla.org/show_bug.cgi?id=589199 (see: https://blog.mozilla.org/addons/2015/10/14/breaking-changes-let-const-firefox-nightly-44/).

See bug_18466_v2 (https://gitweb.torproject.org/user/gk/torbutton.git/log/?h=bug_18466_v2) in my public torbutton repo for review. I added an additional commit to Arthur's two which looked good to me fwiw. mcs: I don't think we need to do something special to avoid a branch here. master will be used for the next alpha and that one is going to be ESR45 based. So, everything is fine. Regarding your other question I defer to Arthur.

Trac:
Keywords: TorBrowserTeam201604 deleted, TorBrowserTeam201604R added
Cc: brade, mcs to brade, mcs, arthuredelstein

Trac:
Status: assigned to needs_review

Replying to gk:

Part of the pain here is due to https://bugzilla.mozilla.org/show_bug.cgi?id=589199 (see: https://blog.mozilla.org/addons/2015/10/14/breaking-changes-let-const-firefox-nightly-44/).

Yes, these incompatible changes seem to be causing a lot of pain across the Firefox extension universe. The patch that you point to in comment:11 looks fine. I wonder if we missed any other things like this in Tor Launcher or Torbutton. How did you find these ones? I assume you noticed something was not functioning correctly?

Replying to mcs:

Replying to gk:

Part of the pain here is due to https://bugzilla.mozilla.org/show_bug.cgi?id=589199 (see: https://blog.mozilla.org/addons/2015/10/14/breaking-changes-let-const-firefox-nightly-44/).

Yes, these incompatible changes seem to be causing a lot of pain across the Firefox extension universe. The patch that you point to in comment:11 looks fine. I wonder if we missed any other things like this in Tor Launcher or Torbutton. How did you find these ones? I assume you noticed something was not functioning correctly?

Yes, I kicked Tor Browser until the issues went away and I had all the functionality back we currently have with the ESR38. I can try to test Tor Launcher a bit more but am pretty confident that it is fine.

I tested Tor Launcher and it works fine for me. Thus, I think only Torbutton was affected. I've applied bug_18466_v2 to master (e6939a0043b8fc944ecfa4559efbd503d1d758c1, d454bd4ac3eb71b3945ff64704bb87fdb211d711 and e4f7079dd5d4ae44b8d10b48889d165eba20a660).

Trac:
Resolution: N/A to fixed
Status: needs_review to closed

Replying to mcs:

Kathy and I looked at c55c1ef497f7b2a97ff7c9b0af351a065df3b477 and 594b2bbc55a0bbc2f548356f44ea191d4e2466e2. Your changes look good. We have two questions: Can you give us some background on why statements such as let mozilla = mozilla || {}; were needed previously? Or maybe they were not needed?

These were not needed. I used this to avoid accidentally destroying a mozilla object (for example) if it had already been defined. But these JS modules are isolated from all other code, so there is no risk of that.

closed

moved to tpo/applications/tor-browser#18466 (closed)