Opened 4 years ago

Closed 4 years ago

#16200 closed defect (fixed)

Torbutton changes for ESR 38

Reported by: mcs Owned by: tbb-team
Priority: Medium Milestone:
Component: Applications/Tor Browser Version:
Severity: Keywords: tbb-torbutton, ff38-esr, TorBrowserTeam201506R, tbb-5.0a3-essential
Cc: arthuredelstein, gk, brade, mikeperry Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Kathy and I have some Torbutton ESR 38 compatibility fixes. We will make them available on the user/brade/torbutton repo. Note that we have not yet run Tor Launcher and Torbutton with a fully-patched Tor Browser tree.

Child Tickets

Change History (15)

comment:1 Changed 4 years ago by gk

Component: TorbuttonTor Browser
Keywords: tbb-torbutton added
Owner: set to tbb-team

comment:3 Changed 4 years ago by mcs

Keywords: TorBrowserTeam201505R added
Status: newneeds_review

comment:4 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201506 added

comment:5 Changed 4 years ago by mikeperry

Keywords: TorBrowserTeam201506R added; TorBrowserTeam201505R TorBrowserTeam201506 removed

comment:6 Changed 4 years ago by arthuredelstein

In Firefox 38, we're no longer allowed to call nsICacheService.evictEntries(...); from JavaScript. This breaks torbutton's "New Identity" function. Instead we're supposed to call Services.cache2.clear().

Here's a patch that fixes this problem, and also fixes two more minor issues with clearing the application cache (aka offline cache).

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

comment:7 in reply to:  6 Changed 4 years ago by gk

Replying to arthuredelstein:

In Firefox 38, we're no longer allowed to call nsICacheService.evictEntries(...); from JavaScript. This breaks torbutton's "New Identity" function. Instead we're supposed to call Services.cache2.clear().

We should respond to https://bugzilla.mozilla.org/show_bug.cgi?id=962334 while we are at it and review asyncEvictStorage() properly given that the new cache is working asynchronously (given comment 2 in the Mozilla bug mentioned above).

comment:8 in reply to:  6 Changed 4 years ago by mcs

Replying to arthuredelstein:

In Firefox 38, we're no longer allowed to call nsICacheService.evictEntries(...); from JavaScript. This breaks torbutton's "New Identity" function. Instead we're supposed to call Services.cache2.clear().

Here's a patch that fixes this problem, and also fixes two more minor issues with clearing the application cache (aka offline cache).

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

r=mcs for this patch (Kathy also looked at it).

comment:9 Changed 4 years ago by mikeperry

Keywords: tbb-5.0a3-essential added

Tag the set of things we should aim to understand/fix for the fist FF38-based TBB (5.0a3, on June 30th).

comment:10 in reply to:  2 ; Changed 4 years ago by arthuredelstein

Replying to mcs:

There are two fixes on the bug16200-01 branch within the user/brade/torbutton repo:

https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug16200-01&id=6e8c92aead9f62b65a018e48297cadbd4ec4d3db

r=arthuredelstein

https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug16200-01&id=840dcad96d55d40d7f3a35a1b485f5c545b3a572

Do we want to be providing a version number here at all? I wonder if a user agent string that we can keep the same forever would be ideal to avoid splitting the pool of users.

comment:11 in reply to:  10 ; Changed 4 years ago by gk

Replying to arthuredelstein:

Replying to mcs:

There are two fixes on the bug16200-01 branch within the user/brade/torbutton repo:

https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug16200-01&id=6e8c92aead9f62b65a018e48297cadbd4ec4d3db

r=arthuredelstein

https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug16200-01&id=840dcad96d55d40d7f3a35a1b485f5c545b3a572

Do we want to be providing a version number here at all? I wonder if a user agent string that we can keep the same forever would be ideal to avoid splitting the pool of users.

Generally, providing the UA of the ESR we ship is the right thing to do I think. This is mainly for usability issues as there are a bunch of sites out there that are using the UA as a means for site access control:

"You have an unsupported user agent, come back if you are using the up-to-date-version of _______."

And using the (most popular) one of the ESR we ship does not split the Tor Browser users group (and there is no chance to blend in with other browser versions properly anyway, so no damage done).

That said, this pref and related Torbutton UA ones are a relic of the toggle era and are doing nothing. We should remove them instead.

Last edited 4 years ago by gk (previous) (diff)

comment:12 in reply to:  11 Changed 4 years ago by mcs

Replying to gk:

That said, this pref and related Torbutton UA ones are a relic of the toggle era and are doing nothing. We should remove them instead.

Here is a patch that removes them:

https://gitweb.torproject.org/user/brade/torbutton.git/commit/?h=bug16200-02&id=699617776da868b4eabf982d23e7c901498905ef

There are other "leftovers" that could be removed too, but this is a start (e.g., I do not think entities such as torbutton.prefs.refererspoofing are used anywhere).

comment:13 Changed 4 years ago by mcs

Cc: mikeperry added

Something else to think about: on #tor-dev today, Mike asked whether we could replace our https://127.0.0.1/ updateURL hacks with data:text/plain, but Georg said we have to use an https: URL. And he is right; see the providesUpdatesSecurely() function here http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/internal/XPIProvider.jsm#6495

But looking at that function led me to try using a data: URL with a useless updateKey. And it seems to work, e.g.,

<em:updateURL>data:text/plain,</em:updateURL>
<em:updateKey>-</em:updateKey>

(the updateKey is not used unless some data is returned by the URL fetch).

Is this an improvement over the https://127.0.0.1/ hack? I think so.

comment:14 in reply to:  13 Changed 4 years ago by gk

Replying to mcs:

Is this an improvement over the https://127.0.0.1/ hack? I think so.

YES!11! I've opened #16427 and #16428 for Torbutton and TorLauncher (as these changes are strictly speaking unrelated to ESR 38 ones and deserve their own tickets).

comment:15 Changed 4 years ago by mikeperry

Resolution: fixed
Status: needs_reviewclosed

Ok, I pushed arthur/16200 and brade/bug16200-02 to origin/master. I also updated the Firefox useragent in tor-browser.git for FF38-esr.

Note: See TracTickets for help on using tickets.