PDFs downloaded with "Open with Nightly" don't respect "Open in system viewer" context menu item
Categories
(Firefox :: Downloads Panel, defect)
Tracking
()
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)
(deleted),
text/x-phabricator-request
|
RyanVM
:
approval-mozilla-beta+
|
Details |
Given that a system PDF viewer besides Nightly is set:
- Download a PDF served with
content-disposition: attachment
, example - Keep the default "Open with Nightly" option in the unknown content type dialog, the PDF will open from the filesystem in a new tab
- 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.
Comment 1•4 years ago
|
||
[Tracking Requested - why for this release]:
Menus should do what they say they do.
Sam, can you take a look?
Assignee | ||
Comment 2•4 years ago
|
||
(In reply to Adam Gashlin (he/him) [:agashlin] from comment #0)
- 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.
Updated•4 years ago
|
Comment 3•4 years ago
|
||
Sam, we're building the last beta for 80 tomorrow do you think you'll have an upliftable fix ready?
Assignee | ||
Comment 4•4 years ago
|
||
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!
Assignee | ||
Comment 5•4 years ago
|
||
Assignee | ||
Comment 6•4 years ago
|
||
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.
Assignee | ||
Comment 8•4 years ago
|
||
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
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 9•4 years ago
|
||
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 10•4 years ago
|
||
Backed out as requested: https://hg.mozilla.org/integration/autoland/rev/2ef8b79d5af2d94e1ac94be7d6b33329c19cb112
Assignee | ||
Comment 11•4 years ago
|
||
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.
Comment 12•4 years ago
|
||
Assignee | ||
Comment 13•4 years ago
|
||
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
- Keep the default "Open with Nightly" option in the unknown content type dialog, the PDF will open from the filesystem in a new tab
- 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
Comment 14•4 years ago
|
||
bugherder |
Comment 15•4 years ago
|
||
Comment on attachment 9169888 [details]
Bug 1657796 - Ensure downloads requested to open with system viewer always do. r?Gijs
Approved for 80.0rc1.
Comment 16•4 years ago
|
||
bugherder uplift |
Updated•4 years ago
|
Comment 17•4 years ago
|
||
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.
Description
•