1738601 - Downloading one .pdf file results in two files at the target location (with "always ask me where to save files" enabled)

Status: VERIFIED FIXED

(Firefox :: File Handling, defect)

Description

[adamtha386]

Attachment: These are the files on the target location after following the steps above.

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:95.0) Gecko/20100101 Firefox/95.0

Steps to reproduce:

Have the preference set to "Always ask me where I want to save files" ("Fråga alltid var jag vill spara filerna" in Swedish.) under Download preferences.

Under the preference "Program", choose to open in Nightly for PDF type documents.

Download a pdf-file.

Actual results:

The file gets downloaded to the chosen target location and the pdf opens in Nightly. But at the target location, there are two files with the same name.

One of the files has a size of 0 bytes (is unusable) and the other is the file that was supposed to be downloaded.

It does not seem to pose a security risk, but it is annoying since I only wanted to download one file but got two at the target location.

Expected results:

Only one file should be at the target location whether one choses to open with Nightly or not.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Firefox::File Handling' component, and is moving the bug to that component. Please revert this change in case you think the bot is wrong.

Comment 2

[Alice0775 White]

I can reproduce the issue in Nightly95.0a1 Windows10.

STR:

1. turn on "Always ask me where I want to save files"
2. choose to "Open in Nightly" for "Portable Document Format(PDF)"
3. Open http://www.shikisensha.com/pdfs.html
4. Right click on a pdf link and choose to "Save As..." and select a folder and save it.

Regression window:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=216de3cdebc9db25336b56380f23a20252d7e555&tochange=3de56e38f3c87f33a1e7849701edb3c62bc472a5

Comment 3

[Shane Hughes [:aminomancer]]

Attachment: A recording of the bug

Thanks for posting, also here's a video of the bug

Comment 4

[Fanolian]

Feels like bug 1719897.

Comment 5

[:Gijs (he/him)]

(In reply to Fanolian from comment #4)

Feels like bug 1719897.

Similar, but that bug is specific to file: PDFs, clicking the download button inside PDF.js (which shouldn't then open another tab with the same PDF in it), and doesn't involve "always ask me where to save files". This is a separate issue.

Comment 6

[:Gijs (he/him)]

Tracking regressions on the pref flip isn't really useful (ie that change didn't change any code so it doesn't tell us where to look for broken code, and because it was a nightly-only pref flip also doesn't accurately reflect what releases will be affected), so I'm removing those flags. We'll track all the opened bugs as blocking enabling this on release, at least initially.

It may be useful to determine a regression window with the pref flipped all the time, to see what actual behaviour change that happened behind the pref changed something here, but I'm not sure if that's the case for this bug. (I suspect this changed when we added the filepicker support in bug 1719901, and the behaviour prior to that would just be broken differently, though I could always be mistaken...)

Comment 7

[:Gijs (he/him)]

Mass moving affected flag for download improvements issues found on Nightly to 96 (nightly from later today / tomorrow), as the download improvements pref will be disabled for 95 beta.

Comment 8

[Micah [:mtigley] (she/her)]

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

Tracking regressions on the pref flip isn't really useful (ie that change didn't change any code so it doesn't tell us where to look for broken code, and because it was a nightly-only pref flip also doesn't accurately reflect what releases will be affected), so I'm removing those flags. We'll track all the opened bugs as blocking enabling this on release, at least initially.

It may be useful to determine a regression window with the pref flipped all the time, to see what actual behaviour change that happened behind the pref changed something here, but I'm not sure if that's the case for this bug. (I suspect this changed when we added the filepicker support in bug 1719901, and the behaviour prior to that would just be broken differently, though I could always be mistaken...)

I dug into this a bit and it looks like the regression is likely bug 1719901. In particular, it looks like this change might be the culprit. I'll see if I can get logs from mozregression for this.

Comment 9

[Micah [:mtigley] (she/her)]

Last good revision: 141832e40a4c643e9a68c21e5d109b42baed7736
First bad revision: 16afe3f983133f2df1242302cff2a5c0e35c3838
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=141832e40a4c643e9a68c21e5d109b42baed7736&tochange=16afe3f983133f2df1242302cff2a5c0e35c3838

Comment 10

[:Gijs (he/him)]

(In reply to Micah [:mtigley] (she/her) from comment #8)

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

Tracking regressions on the pref flip isn't really useful (ie that change didn't change any code so it doesn't tell us where to look for broken code, and because it was a nightly-only pref flip also doesn't accurately reflect what releases will be affected), so I'm removing those flags. We'll track all the opened bugs as blocking enabling this on release, at least initially.

It may be useful to determine a regression window with the pref flipped all the time, to see what actual behaviour change that happened behind the pref changed something here, but I'm not sure if that's the case for this bug. (I suspect this changed when we added the filepicker support in bug 1719901, and the behaviour prior to that would just be broken differently, though I could always be mistaken...)

I dug into this a bit and it looks like the regression is likely bug 1719901. In particular, it looks like this change might be the culprit. I'll see if I can get logs from mozregression for this.

Thanks! And this is interesting, that's the same bit of code as bug 1738129 - see bug 1738129 comment 2 for my investigation there. I wonder if fixing that will address both issues.

Comment 11

[Micah [:mtigley] (she/her)]

I narrowed down the initial file being created at https://searchfox.org/mozilla-central/rev/88cd13997fb0747cdcd78638fc762ff2d75e1fc5/toolkit/mozapps/downloads/HelperAppDlg.jsm#464, which is then passed to the saveDestinationAvailable callback where we proceed to use the file destination in ContinueSave

Now the issue starts when we begin checking whether or not the file should be handled internally. Because PDFs are set to open in Nightly by default, we kick off the call to LaunchWithApplication, which ends up creating the other file and we end up using that as the final save destination.

It seems we can check whether or not the "Alway ask you where to save files" is enabled (something similar to this) and avoid calling LaunchWithApplication here. Or ideally, we remove that check altogether since we already decide to do this here. I'm trying to remember if there was a reason why this path is needed when "Always ask you where to save files" is enabled, but it's lost on me atm.

Comment 12

[Micah [:mtigley] (she/her)]

Attachment: Bug 1738601 - Ensure internally handled files are created only once when save destination is prompted. r=mhowell!,gijs!

Comment 15

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/07410d76eca4
Ensure internally handled files are created only once when save destination is prompted. r=mhowell,Gijs

Comment 16

[Cristina Cozmuta (:CrissCozmuta)]

Backed out for causing failures on browser_shows_where_to_save_dialog.js. CLOSED TREE

Backout link : https://hg.mozilla.org/integration/autoland/rev/873fbf652ba633e16e15ffc98b781ca27dd1f72e

Push with failures : https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Crunnable&revision=07410d76eca461dc4c6ef6fee07ead4711706da8&selectedTaskRun=QIuO6EusTGGBjD4Brw1mMw.0

Link to failure log :
https://treeherder.mozilla.org/logviewer?job_id=357438370&repo=autoland&lineNumber=3991
https://treeherder.mozilla.org/logviewer?job_id=357438364&repo=autoland&lineNumber=2805

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a475462a2dce
Ensure internally handled files are created only once when save destination is prompted. r=mhowell,Gijs

Comment 18

[Cristina Cozmuta (:CrissCozmuta)]

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

Comment 19

[Giorgia Nichita, Release Desktop QA]

Reproduced the issue using an affected Nightly build 95.0a1 (2021-10-31) and STR from Comment 2.

Verified as fixed on Firefox 96.0b4 (20211212185725) on Windows 10 x64, Ubuntu 20.04 and macOS 10.15. There is only one file downloaded in the target location.