Closed Bug 1740135 Opened 2 years ago Closed 2 years ago

Firefox automatically opens a PDF after downloading it

Categories

(Firefox :: Downloads Panel, defect, P2)

Desktop
All
defect
Points:
3

Tracking

()

VERIFIED FIXED
96 Branch
Tracking Status
firefox-esr91 --- unaffected
firefox94 --- unaffected
firefox95 --- disabled
firefox96 --- verified

People

(Reporter: marco, Assigned: sfoster, NeedInfo)

References

Details

(Keywords: blocked-ux, regression, Whiteboard: [fidefe-mr11-downloads])

Attachments

(1 file)

STR:

  1. Open a PDF
  2. Click on the PDF.js button to save a PDF

The PDF is saved and automatically opened. It shouldn't be automatically opened.

Keywords: regression
Summary: Downloading a PDF → Firefox automatically opens a PDF after downloading it
Severity: -- → S2
Keywords: blocked-ux
OS: Unspecified → All
Hardware: Unspecified → Desktop
See Also: → 1719897
Whiteboard: [fidefe-mr11-downloads]

Yes, we're aware that this regressed. We're waiting on UX feedback on what should happen instead, which is less obvious.

:gijs is there a workaround for this in the meantime?
having to deal with a lot of PDFs for hiring and they're cluttering my drive 😅

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Morgan Reschenberg [:morgan] from comment #2)

:gijs is there a workaround for this in the meantime?
having to deal with a lot of PDFs for hiring and they're cluttering my drive 😅

Change your PDF action to "always ask" in the Firefox settings, or change the about:config pref to turn off the downloads improvement work for now.

Flags: needinfo?(gijskruitbosch+bugs)

Downloading a PDF, and automatically it opens in the new tab, even after checking the box for "Save File"

(In reply to roy from comment #4)

Downloading a PDF, and automatically it opens in the new tab, even after checking the box for "Save File"

I'm not really sure exactly what you mean; can you file a separate bug and include details of what version of Nightly/Firefox you're using, what your PDF preference is set to in Firefox's settings, a link to a PDF with which this happens, and perhaps a screencast or more detailed steps? Unfortunately all of those impact the behaviour here (yes, even if you think this happens with all the PDFs you use).

Flags: needinfo?(royritesh102)
Priority: -- → P2
Assignee: nobody → sfoster
Blocks: 1738448
Status: NEW → ASSIGNED

So... I /think/ all we need here is effectively something like this:

diff --git a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm b/toolkit/components/pdfjs/content/PdfStreamConverter.jsm
--- a/toolkit/components/pdfjs/content/PdfStreamConverter.jsm
+++ b/toolkit/components/pdfjs/content/PdfStreamConverter.jsm
@@ -365,6 +365,15 @@ class ChromeActions {
       });
       return;
     }
+    if (data.sourceEventType == "download") {
+      let actor = getActor(this.domWindow);
+      actor.sendAsyncMessage("PDFJS:Parent:saveURL", {
+        blobUrl,
+        filename,
+        skipPrompt: true
+      });
+      return;
+    }
 
     // The download is from the fallback bar or the download button, so trigger
     // the open dialog to make it easier for users to save in the downloads
diff --git a/toolkit/components/pdfjs/content/PdfjsParent.jsm b/toolkit/components/pdfjs/content/PdfjsParent.jsm
--- a/toolkit/components/pdfjs/content/PdfjsParent.jsm
+++ b/toolkit/components/pdfjs/content/PdfjsParent.jsm
@@ -101,7 +101,7 @@ class PdfjsParent extends JSWindowActorP
       data.filename /* aFileName */,
       null /* aFilePickerTitleKey */,
       true /* aShouldBypassCache */,
-      false /* aSkipPrompt */,
+      !!data.skipPrompt /* aSkipPrompt */,
       null /* aReferrerInfo */,
       null /* aCookieJarSettings*/,
       null /* aSourceDocument */,

I.e. When there's a "download" sourceEventType - which we /should/ have when clicking the download button in the PDF.js viewer - we simply save the blob URL to the given filename, in whatever default or configured directory is used for Downloads. That gives us the result that when you click on the download button in the PDF.js viewer, the blobURL from the viewer is downloaded and the download panel opens showing this new entry. Which I believe is what we want - no "Save as.." prompts, or other how-do-you-want-to-handle this prompts. This seems to fix both this bug and bug 1738448.

And on the face of it means we might be able to remove a bunch of code from PDFStreamConverter. However there's a try/catch in viewer.js which could result in this download method being called with no sourceEventType. So I guess we still need the existing code which does all the redundant and hard work of getting a channel, hooking up the stream listeners etc. etc. And maybe there are other front-ends or entry points which might call download in PDFStreamConverter where we need this stuff.

Am I way off-base here? If this seems roughly on the right track I'll put together a patch, fix tests etc. And wish phabricator had f? again.

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Sam Foster [:sfoster] (he/him) from comment #6)

That gives us the result that when you click on the download button in the PDF.js viewer, the blobURL from the viewer is downloaded and the download panel opens showing this new entry. Which I believe is what we want - no "Save as.." prompts, or other how-do-you-want-to-handle this prompts. This seems to fix both this bug and bug 1738448.

I don't think this is right; the figma from UX shows that we want a "save as" prompt when the user clicks that button. I don't know off-hand how that impacts the other parts of the work here...

Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(sfoster)
Points: --- → 3

(In reply to :Gijs (he/him) from comment #7)

I don't think this is right; the figma from UX shows that we want a "save as" prompt when the user clicks that button. I don't know off-hand how that impacts the other parts of the work here...

Oh that's even simpler in that case - we'll treat sourceEventType == "save" and sourceEventType == "download" in the exact same way.

Flags: needinfo?(sfoster)
Pushed by sfoster@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e7b6f35b147a
Treat download button like save action to avoid opening on completion. r=Gijs
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 96 Branch
Flags: qe-verify+

Reproduced the issue using an affected Nightly build 96.0a1 (2021-11-08) and STR from Comment 0.

Verified as fixed on Firefox 96.0b4 (20211212185725) on Windows 10 x64, Ubuntu 20.04 and macOS 10.15. The PDF files are no longer automatically opened after they are downloaded.

Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1751061
Regressions: 1766030
Blocks: 1759083
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: