Rip Firefox Hello Beta / Loop extension in ESR45 based Tor Browser

We should think about getting rid of the Hello/Loop extension. As gacar mentioned (https://lists.torproject.org/pipermail/tor-qa/2016-April/000809.html) it is quite large (1.6 MB) and is probably not running anyway as we are disable WebRTC at compile time and Firefox Hello is not active either.

added TorBrowserTeam201605R component::applications/tor browser ff45-esr owner::arthuredelstein priority::high resolution::fixed severity::major status::closed tbb-6.0-must type::task labels

Trac:
Description: We should think about getting rid of the Hello/Loop extension. As gacar mentioned it is quite large (1.6 MB) and is probably not running anyway as we are disable WebRTC at compile time and Firefox Hello is not active either.

to

We should think about getting rid of the Hello/Loop extension. As gacar mentioned (https://lists.torproject.org/pipermail/tor-qa/2016-April/000809.html) it is quite large (1.6 MB) and is probably not running anyway as we are disable WebRTC at compile time and Firefox Hello is not active either.

Also see https://bugzilla.mozilla.org/show_bug.cgi?id=1266358 (which I happened to see today in a list of recent bugs found by the Firefox desktop QA team).

Trac:
Cc: gacar to gacar, brade, mcs

Yes, I think we should rip that out (+ the bits depending on it) to make sure it is off.

Trac:
Severity: Normal to Major
Priority: Medium to High

Good that all issues about it that were found in 6.0a5 were reported to Mozilla. So it's not worth mentioning them in #18937 (moved).

Trac:
Summary: Think about getting rid of the loop extension in ESR45 based Tor Browser to Rip Firefox Hello Beta / Loop extension in ESR45 based Tor Browser

Dragging into May to have it on our 6.0 radar.

Trac:
Keywords: N/A deleted, TorBrowserTeam201605 added

Trac:
Status: new to assigned
Owner: tbb-team to arthuredelstein

Trac:
Keywords: N/A deleted, tbb-6.0-must added

Here's a branch with two commits: https://github.com/arthuredelstein/tor-browser/commits/18884+6

• 941c9a27bc5dfa5ac8be9cf6d6c4d9d06c41fbbb introduces a build flag, --disable-loop, to prevent bundling the Loop extension.
• 3790ded147c3dcf5f12e65f5ae8a4cc7e987f204 adds --disable-loop to Tor Browser's mozconfig files.

Trac:
Keywords: TorBrowserTeam201605 deleted, TorBrowserTeam201605R added
Status: assigned to needs_review

When testing with a standalone (non-gitian) build on MacOS, we had to add this to the patch:

diff --git a/browser/locales/Makefile.in b/browser/locales/Makefile.in
index be9454e..ec54b63 100644
--- a/browser/locales/Makefile.in
+++ b/browser/locales/Makefile.in
@@ -133,7 +133,9 @@ ifdef MOZ_WEBAPP_RUNTIME
        @$(MAKE) -C ../../webapprt/locales AB_CD=$* XPI_NAME=locale-$*
 endif
        @$(MAKE) -C ../../extensions/spellcheck/locales AB_CD=$* XPI_NAME=locale-$*
+ifdef MOZ_LOOP
        @$(MAKE) -C ../extensions/loop/chrome/locale AB_CD=$* XPI_NAME=locale-$*
+endif
        @$(MAKE) -C ../../intl/locales AB_CD=$* XPI_NAME=locale-$*
        @$(MAKE) -C ../../devtools/client/locales AB_CD=$* XPI_NAME=locale-$* XPI_ROOT_APPID='$(XPI_ROOT_APPID)'
        @$(MAKE) -B searchplugins AB_CD=$* XPI_NAME=locale-$*

Also, our packaged build still includes the loop extension (but we haven't figured out why).

ls -l obj-macos/dist/firefox/TorBrowserDebug.app/Contents/Resources/browser/features/
total 3312
-rw-r--r--  1 brade  staff  1691779 May 24 11:39 loop@mozilla.org.xpi

Trac:
Status: needs_review to needs_revision

Okay, I am waiting with the 6.0 build for this one. I guess we can get it going tomorrow then.

I decided to postpone this one for 6.0.1 which seems to be okay to me given that there will only be about one week between it and 6.0 (the latter is planned for May 30 now). Apart from the need to get it reviewed again I think testing the fix in a nightly at least would be beneficial. But starting the build for 6.0 is kind of urgent as we want to have a somewhat meaningful QA before.

Replying to mcs:

When testing with a standalone (non-gitian) build on MacOS, we had to add this to the patch: {{{ diff --git a/browser/locales/Makefile.in b/browser/locales/Makefile.in index be9454e..ec54b63 100644 --- a/browser/locales/Makefile.in +++ b/browser/locales/Makefile.in @@ -133,7 +133,9 @@ ifdef MOZ_WEBAPP_RUNTIME @

(MAKE) -C ../../webapprt/locales AB_CD=

* XPI_NAME=locale-$* endif @

(MAKE) -C ../../extensions/spellcheck/locales AB_CD=

* XPI_NAME=locale-$* +ifdef MOZ_LOOP @

(MAKE) -C ../extensions/loop/chrome/locale AB_CD=

* XPI_NAME=locale-$* +endif @

(MAKE) -C ../../intl/locales AB_CD=

* XPI_NAME=locale-$* @

(MAKE) -C ../../devtools/client/locales AB_CD=

* XPI_NAME=locale-

* XPI_ROOT_APPID='

(XPI_ROOT_APPID)' @

(MAKE) -B searchplugins AB_CD=

* XPI_NAME=locale-$* }}}

Thanks for finding this issue. I've added your fix, as well as a second one that fixes a problem I ran into with make package.

​https://github.com/arthuredelstein/tor-browser/commits/18884+7 Hash f2c8fe1176794a453318e359b08fbf1d618f45d4 There are two patches as before. I haven't yet tested the new revised patches in a full gitian build for 3 platforms, but I'm posting what is working for me on OS X.

Also, our packaged build still includes the loop extension (but we haven't figured out why). {{{ ls -l obj-macos/dist/firefox/TorBrowserDebug.app/Contents/Resources/browser/features/ total 3312 -rw-r--r-- 1 brade staff 1691779 May 24 11:39 loop@mozilla.org.xpi }}}

I'm not seeing this issue. Did you try running ./mach clobber before running ./mach build?

Trac:
Status: needs_revision to needs_review

Replying to arthuredelstein:

... Thanks for finding this issue. I've added your fix, as well as a second one that fixes a problem I ran into with make package.

​https://github.com/arthuredelstein/tor-browser/commits/18884+7 Hash f2c8fe1176794a453318e359b08fbf1d618f45d4 There are two patches as before. I haven't yet tested the new revised patches in a full gitian build for 3 platforms, but I'm posting what is working for me on OS X.

Did you push that branch?

Also, our packaged build still includes the loop extension (but we haven't figured out why). {{{ ls -l obj-macos/dist/firefox/TorBrowserDebug.app/Contents/Resources/browser/features/ total 3312 -rw-r--r-- 1 brade staff 1691779 May 24 11:39 loop@mozilla.org.xpi }}}

I'm not seeing this issue. Did you try running ./mach clobber before running ./mach build?

I thought we had clobbered everything, but I will double check.

Replying to mcs:

... I thought we had clobbered everything, but I will double check.

You were right. After fully clobbering and putting in a fix for make package (probably the same one you added), things look OK. Sorry for the confusion.

Please push your 18884+7 branch and we will review it.

Replying to mcs:

Replying to mcs:

... I thought we had clobbered everything, but I will double check.

You were right. After fully clobbering and putting in a fix for make package (probably the same one you added), things look OK. Sorry for the confusion.

Please push your 18884+7 branch and we will review it.

Oops, I meant to do that before. It's pushed now. Thanks for having a look. I haven't managed to run the full gitian build still, so it's possible you may run into issues.

r=brade, r=mcs This looks good to us.

FWIW, I built all 3 platforms with gitian and there were no errors. And I manually ran the OS X version and observed no problems.

Looks good to me. And given that this feature is not enabled in Tor Browser anyway we can put it into 6.0.1 too. Commit 61a4d57b1cc573b85baef431b277149cbf740c74 and 9a9765a3cb73e7341b7c2494c167725b4f688c07 on tor-browser-45.1.1esr-6.0-1 have the fixes.

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

closed