1281959 - Introduce pref to disable "open with" option in download dialog (Tor 17502)

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[Arthur Edelstein [:arthur]]

A patch was introduced in Tor Browser to disable the "open with" option in the download dialog. From the patch summary:

"""
If browser.download.forbid_open_with is set to true (default: false)
the download dialog will not ever show the "Open with" option for
opening the file with an external application; only the "Save" and
"Cancel" options will be available.

This is very useful to enable when the browser is sandboxed (e.g. via
AppArmor) in such a way that it is forbidden to run external
applications, since users then are not presented with this choice
which will not work and only cause confusion.
"""

See https://trac.torproject.org/17502 for more information

I'd like to propose upstreaming this patch

Comment 1

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1281959-Add-a-pref-hiding-the-Open-with-option.patch

Here's the Tor Browser patch rebased to mozilla-central.

Try result: https://treeherder.mozilla.org/#/jobs?repo=try&revision=ec385071736a

Comment 2

[Dave Huseby [:huseby]]

Justin, this patch is part of the Tor Patch uplift.  See here: https://wiki.mozilla.org/Security/Tor_Uplift/Tracking  Will you please review this patch ASAP?  Thanks!

Comment 3

[Justin Dolske [:Dolske]]

Comment on attachment 8764806 [details] [diff] [review]
0001-Bug-1281959-Add-a-pref-hiding-the-Open-with-option.patch

Review of attachment 8764806 [details] [diff] [review]:
-----------------------------------------------------------------

Yay for more Tor patches!

Since this is toolkit code, you should add the default pref to /modules/libpref/init/all.js. Otherwise a non-Firefox app (like Thunderbird) will fail, since the getBoolPref() call throws when there's no default pref. Prefs in firefox.js should generally be for prefs that are different than the all.js default, or for things that are in /browser-only code.

[Alternatively you could do something like wrapping the getBoolPref in a try/catch with a local default, blah blah blah, but I think it would be best to move the pref to all.js]

Nit: Fix the "See bug ??", or better yet just remove it. Hg history is fine here.

r+ with those obvious fixes (no need for re-review).

Also, if there are other things that break due to AppArmor restrictions, it would worth considering if this kind of stuff can be done as a run-time check instead of as a static pref. Then Firefox will just do the right thing automagically, without someone needing to remember to an obscure pref.

Comment 4

[Arthur Edelstein [:arthur]]

Attachment: 0001-Bug-1281959-Add-a-pref-hiding-the-Open-with-option.patch

(In reply to Justin Dolske [:Dolske] from comment #3)
> Comment on attachment 8764806 [details] [diff] [review]
> 0001-Bug-1281959-Add-a-pref-hiding-the-Open-with-option.patch
> 
> Review of attachment 8764806 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Yay for more Tor patches!

:) Thank you for the review!

> Since this is toolkit code, you should add the default pref to
> /modules/libpref/init/all.js. Otherwise a non-Firefox app (like Thunderbird)
> will fail, since the getBoolPref() call throws when there's no default pref.
> Prefs in firefox.js should generally be for prefs that are different than
> the all.js default, or for things that are in /browser-only code.
> 
> [Alternatively you could do something like wrapping the getBoolPref in a
> try/catch with a local default, blah blah blah, but I think it would be best
> to move the pref to all.js]
> 
> Nit: Fix the "See bug ??", or better yet just remove it. Hg history is fine
> here.
> 
> r+ with those obvious fixes (no need for re-review).

I made these fixes and I'm attaching the new version.

> Also, if there are other things that break due to AppArmor restrictions, it
> would worth considering if this kind of stuff can be done as a run-time
> check instead of as a static pref. Then Firefox will just do the right thing
> automagically, without someone needing to remember to an obscure pref.

That's a good idea.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/f5a3379de473
Add a pref hiding the "Open with" option. r=dolske

Comment 6

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/f5a3379de473

Comment 7

[Mohammad Maruf Islam]

I have reproduced this bug with Firefox Nightly 50.0a1 (Build ID:20160623030210) on 
Windows 8.1, 64-bit.

Verified as fixed with Firefox Nightly 50.0a1 (Build ID: 20160729030203)
Mozilla/5.0 (Windows NT 6.3; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0

Comment 8

[Subhrajyoti Sen]

Verified as FIXED on Firefox Developer Edition 50.0a2 (2016-09-09) on Linux Mint 64-bit

Comment 9

[Carmen]

I managed to reproduce the previous behavior on Nightly 50.0a1 (2016-06-23), using Windows 10 Pro x64.

The fix was verified on Firefox Beta 50.0b3 Build 1, across platforms, using Windows 10 Pro x64, Ubuntu 16.04 LTS x64 and Mac OS Yosemite 10.10.4

Comment 10

[Cristian Comorasu, QA [:ccomorasu], Releae Desktop QA - Please contact [e-mail]]

Thank you, Carmen!