PDFs, WebP and avif were downloaded automatically to /tmp (which might be okays for Linux, but it was %temp% on Windows, and in neither the files weren't even cleaned up as expected!).
XML had an additional radio button to be opened in Mullvad Browser, and if it was chosen, it had the same behavior.
I'd say this behavior (that the file remains in the temporary files) is unexpected to say the least.
We probably have other issues about this.
Please notice that only Content-disposition: attachment triggered this.
But I wonder if there's a maximum file size Firefox will open in memory, and above that threshold will download the file also when attachment isn't specified.
I explicitly mentioned these files because they're the ones shown in settings.
Firefox has a lot of additional files, but I wonder if they are collected when you visit them.
For example, many supported files aren't shown there (HTML, images such as PNG and JPEG, audio files including ogg, flac and wave).
Okay, I think I found a way to fix this behavior also for content that isn't PDF and is set for automatic download (e.g., avif and webp images).
Patch v1
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yamlindex 8b44be0ef39c..1fdb6673f560 100644--- a/modules/libpref/init/StaticPrefList.yaml+++ b/modules/libpref/init/StaticPrefList.yaml@@ -1446,6 +1446,12 @@ value: false mirror: always+# tor-browser#42220+- name: browser.download.ignore_content_disposition+ type: bool+ value: true+ mirror: always+ # See bug 1811830 - name: browser.download.force_save_internally_handled_attachments type: booldiff --git a/uriloader/base/nsURILoader.cpp b/uriloader/base/nsURILoader.cppindex ef138f05d2b8..a7b607b085c9 100644--- a/uriloader/base/nsURILoader.cpp+++ b/uriloader/base/nsURILoader.cpp@@ -292,10 +292,12 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { LOG((" forceExternalHandling: %s", forceExternalHandling ? "yes" : "no")); if (forceExternalHandling &&- mozilla::StaticPrefs::browser_download_open_pdf_attachments_inline()) {+ (mozilla::StaticPrefs::browser_download_open_pdf_attachments_inline() ||+ mozilla::StaticPrefs::browser_download_ignore_content_disposition())) { // Check if this is a PDF which should be opened internally. We also handle // octet-streams that look like they might be PDFs based on their extension. bool isPDF = mContentType.LowerCaseEqualsASCII(APPLICATION_PDF);+ nsAutoCString ext; if (!isPDF && (mContentType.LowerCaseEqualsASCII(APPLICATION_OCTET_STREAM) || mContentType.IsEmpty())) {@@ -307,7 +309,6 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { aChannel->GetURI(getter_AddRefs(uri)); nsCOMPtr<nsIURL> url(do_QueryInterface(uri)); if (url) {- nsAutoCString ext; url->GetFileExtension(ext); isPDF = ext.EqualsLiteral("pdf"); }@@ -319,7 +320,8 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { // 'forceExternalHandling' again. This allows it open a PDF directly // instead of downloading it first. It may still end up being handled by // a helper app depending anyway on the later checks.- if (isPDF) {+ if (isPDF ||+ mozilla::StaticPrefs::browser_download_ignore_content_disposition()) { nsCOMPtr<nsILoadInfo> loadInfo; aChannel->GetLoadInfo(getter_AddRefs(loadInfo));@@ -328,8 +330,13 @@ nsresult nsDocumentOpenInfo::DispatchContent(nsIRequest* request) { nsCOMPtr<nsIMIMEService> mimeSvc( do_GetService(NS_MIMESERVICE_CONTRACTID)); NS_ENSURE_TRUE(mimeSvc, NS_ERROR_FAILURE);- mimeSvc->GetFromTypeAndExtension(nsLiteralCString(APPLICATION_PDF), ""_ns,- getter_AddRefs(mimeInfo));+ if (isPDF) {+ mimeSvc->GetFromTypeAndExtension(nsLiteralCString(APPLICATION_PDF),+ ""_ns, getter_AddRefs(mimeInfo));+ } else {+ mimeSvc->GetFromTypeAndExtension(mContentType, ext,+ getter_AddRefs(mimeInfo));+ } if (mimeInfo) { int32_t action = nsIMIMEInfo::saveToDisk;
I found the UX of this is... strange.
Even when you open a link like this from about:blank, you will go to a new tab.
And you won't be able to refresh this new tab. I found a known Bug, in which they say the tab doesn't have history.
I might try to fix this, but at least the disk leak that was caused without confirmation will be fixed by this patch.
Now, there's also another case: when you choose "Open with { -brand-short-name }" in the download (i.e., content the browser will offer to open, but not automatically).
That's the case for example of XML (at least in my dev build, but I think I haven't set it, so it should be a default).
A trivial version of the patch (one that just prevent forceExternalHandling from being set to true) forces all of these contents to be opened in-browser, even when the user has "ask me" in the settings.
I prefer the current version, that follows the user settings.
However, I need to investigate on that other path.
So far, I found that it's partially implemented in toolkit/mozapps/downloads/HelperAppDlg.sys.mjs, and that I might have some luck with setDownloadToLaunch.
1. Some downloads happen automatically (PDF, WebP, avif)
Some files are set to be opened automatically by the browser automatically by default.
They currently result in disk leaks because they are saved first.
Upstream has a preference to open PDFs directly (browser.download.open_pdf_attachments_inline), but it doesn't have a similar thing for other files.
I can extend it in three ways:
Add another pref, and keep the special cases for PDF files so that one open only PDFs inline (my current approach)
Keep the PDF pref, and use it for all files that are set to be opened automatically (the pref won't be descriptive anymore...), we can probably delete upstream's special cases for PDFs
Mix of 1+2: rename the PDF pref and remove the various PDF special cases
UX problems
Files opened in this way can't be reloaded (clicking reload won't do anything).
I think this has a lower priority, and I opened Bug 1917085 (and there's also Bug 1851968).
Second problem is that if you try to open a file in this way from new tab/new window, a new tab will be opened anyway.
I've filed upstream Bug 1917088 for this, but I see it mostly as an annoyance and lowest priority of the problems I found so far.
I think both these UX problems can be deferred to 14.5.
2. Some files will have "Open with Tor/Mullvad Browser" in the prompt, but it will result in a download
This is very hard to solve for me.
Basically, we need to tell the download to go back to be a normal load.
It's above my knowledge (and current understanding) of Firefox's networking code. I will need some help from upstream (unless @jwilde also knows something about this).
(Also, a wrong patch here might make Firefox very unhappy about memory leaks, you can notice that with debug builds).
As a workaround, we could change the UI to be more explicit about it, and say: "Download and open with Tor/Mullvad Browser", instead of saying only "Open with".
(Notice that also the other "open with" will perform a download first, but it should be enough explicit that people will go out of the "safe" Tor Browser environment... Maybe we should add the download warning also in that window).
Opening a bug was a great idea, and it led to the creation of #43211.
This patch should be good enough, but Gijs mentioned excluding HTML, SVG and other files that can run scripts from it because they will run in the same origin as the serving website (instead of being from the filesystem).
I think in our case we should not offer "Open with Tor Browser" in that case, but that seems harder to uplift, as upstream will be able to download the files to the filesystem, and get away with it in this way (which will be a disk leak for us).
Unless we decide to keep the feature even though it's a known disk leak.