Closed Bug 1657796 Opened 4 years ago Closed 4 years ago

PDFs downloaded with "Open with Nightly" don't respect "Open in system viewer" context menu item

Categories

(Firefox :: Downloads Panel, defect)

defect

Tracking

()

VERIFIED FIXED
81 Branch
Tracking Status
firefox-esr68 --- unaffected
firefox-esr78 --- unaffected
firefox79 --- wontfix
firefox80 + verified
firefox81 --- verified

People

(Reporter: agashlin, Assigned: sfoster)

References

(Blocks 1 open bug, Regressed 1 open bug)

Details

Attachments

(1 file)

Given that a system PDF viewer besides Nightly is set:

  1. Download a PDF served with content-disposition: attachment, example
  2. Keep the default "Open with Nightly" option in the unknown content type dialog, the PDF will open from the filesystem in a new tab
  3. Right click on the download in the downloads list and select "Open in System Viewer". It will open another tab with the PDF in Firefox.

[Tracking Requested - why for this release]:
Menus should do what they say they do.

Sam, can you take a look?

Severity: -- → S2
Flags: needinfo?(sfoster)

(In reply to Adam Gashlin (he/him) [:agashlin] from comment #0)

  1. Right click on the download in the downloads list and select "Open in System Viewer". It will open another tab with the PDF in Firefox.

I can reproduce. Thanks.

Assignee: nobody → sfoster
Status: NEW → ASSIGNED
Flags: needinfo?(sfoster)

Sam, we're building the last beta for 80 tomorrow do you think you'll have an upliftable fix ready?

Flags: needinfo?(sfoster)

I'm putting a patch together. The problem is right here: https://searchfox.org/mozilla-central/source/toolkit/components/downloads/DownloadIntegration.jsm#799-800
aDownload.handleInternally is true in this case, so even though useSystemDefault is also true, it wins out and and open the download in a new tab. I think is true to say that useSystemDefault should always win in this case - the user has explicitly made that choice by clicking that context menu item . But that seems kinda obvious so I need to understand why it wasn't already that way!

Flags: needinfo?(sfoster)

There's a patch up for review, and try push at: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e33862474c1b851472b733231bf3172a86842b17

We don't have great automated test coverage in this area - as it is interacting with the external system and installed applications.

The change just re-orders the logic so even if a download can be handled internally (i.e. the user chose to open in the browser at the "What should nightly do with this file prompt", or its a known application/pdf type of file) we still open in system viewer as the user requested by clicking on the "Open in system viewer" context menu item.

Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1b027ec0b329 Ensure downloads requested to open with system viewer always do. r=jaws

Comment on attachment 9169888 [details]
Bug 1657796 - Ensure downloads requested to open with system viewer always do. r?Gijs

Beta/Release Uplift Approval Request

  • User impact if declined: Clicking the "open in system viewer" on a downloaded PDF will actually open the PDF in a new tab.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment #0.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a minor tweak to the logic for how we handle opening a download. There is test coverage for the scenarios in which we expect the download to open in a new tab, but not for those cases where we expect to open in an external application.
  • String changes made/needed: None
Attachment #9169888 - Flags: approval-mozilla-beta?
Flags: qe-verify+

I don't think the current patch works properly, useSystemDefault is now being ignored if any other option was selected from the dialog, or if the file was just saved. (cross-posted from phab, sorry for not getting to this sooner)

Comment on attachment 9169888 [details]
Bug 1657796 - Ensure downloads requested to open with system viewer always do. r?Gijs

I backed that patch out as it broke a different case and needs more work.

Attachment #9169888 - Flags: approval-mozilla-beta?
Pushed by sfoster@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/2420dc816657 Ensure downloads requested to open with system viewer always do. r=Gijs

Comment on attachment 9169888 [details]
Bug 1657796 - Ensure downloads requested to open with system viewer always do. r?Gijs

Beta/Release Uplift Approval Request

  • User impact if declined: Clicking the "open in system viewer" on a downloaded PDF will actually open the PDF in a new tab.
    Toggling off the "Always use system viewer" item won't actually revert to loading downloaded PDFs in a new pdf.js tab.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: No
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: 1. Download a PDF served with content-disposition: attachment
  1. Keep the default "Open with Nightly" option in the unknown content type dialog, the PDF will open from the filesystem in a new tab
  2. Right click on the download in the downloads list and select "Open in System Viewer". It will open another tab with the PDF in Firefox.

For the "Always use system viewer" behavior:
4. Right click on the download item in the downloads panel for the previously downloaded PDF
5. Click "Always open in system viewer". It should become checked, and open the PDF into your external PDF viewer.
6. Right click again on the PDF download item. Click "Always open in system viewer" to toggle it off. The PDF and any subsequent PDF downloads should open and load directly into a new tab via pdf.js

  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): This is a minor tweak to the logic for how we handle opening a download. There is test coverage for the scenarios in which we expect the download to open in a new tab, but not for those cases where we expect to open in an external application.
  • String changes made/needed: None
Attachment #9169888 - Flags: approval-mozilla-beta?
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 81 Branch

Comment on attachment 9169888 [details]
Bug 1657796 - Ensure downloads requested to open with system viewer always do. r?Gijs

Approved for 80.0rc1.

Attachment #9169888 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
QA Whiteboard: [qa-triaged]

Reproduced the initial issue with Firefox 81.0a1 (20200806215439) on Windows 10x64.
The issue is verified fixed with Firefox 81.0a1 (20200816214203) and Firefox 80.0b9 (20200814201339) from comment 16 on Windows 10x64, macOS 10.12 and Ubuntu 18.04. Checking Always Open in System Viewer in downloads panel will open the pdf with the external PDF viewer and unchecking Always Open in System Viewer will open the pdf in a new tab.

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

Attachment

General

Created:
Updated:
Size: