download option uniquify prematurely creates a 0 byte file

RESOLVED FIXED in Firefox 55

Status

()

Toolkit
WebExtensions: General
P3
normal
RESOLVED FIXED
5 months ago
5 months ago

People

(Reporter: none11given, Assigned: zombie)

Tracking

54 Branch
mozilla55
Points:
---

Firefox Tracking Flags

(firefox55 fixed)

Details

(Whiteboard: downloads[triaged])

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(3 attachments)

(Reporter)

Description

5 months ago
Created attachment 8871020 [details]
bug.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/58.0.3029.110 Safari/537.36

Steps to reproduce:

Create a WebExtension with download permissions. Set it up to download an image from a webpage with the saveAs:true option (causing a "save as" file dialog to open).

Save an image from a webpage with the extension (everything works fine). Then, attempt to save the same image again. Observe the contents of the "save as" dialog.

I'll attach a simple WebExtension that prompts to save an image when Shift+clicking it. It will demonstrate this issue if you Shift+click an image, save it, then Shift+click it again and look at the "save as dialog."


Actual results:

When the "save as" dialog opens, Firefox has already created a 0 byte file with the same "uniqufied" name you are now being prompted to download.

In detail...

I have "Example.png" in my downloads folder already. Whether I downloaded it using the WebExtension or not doesn't matter. Then I use the WebExtension to prompt a download of "Example.png" again. It will use the default name conflict action of "uniquify," and prompt me to download the file as "Example(1).png" instead.

The problem is that when it prompts me to download "Example(1).png", it has in fact already created "Example(1).png", a 0 byte file, at the download location.

Aftermath...

If you go ahead with the save action, then the 0 byte file is overwritten with the actual download, and everything is fine.

But if you close the "save as" dialog without saving, then the 0 byte file will just sit there on the disk forever. If you keep initiating the "save as" and then closing the dialog, Firefox will create "Example(2).png" then "Example(3).png" then "Example(4)".png...


Expected results:

No file should be created because we are at the "save as" dialog -- we haven't decided to download the image yet.
(Reporter)

Comment 1

5 months ago
Created attachment 8871021 [details]
webext.zip

WebExtension useful for recreating the problem. Shift+click an image to prompt "save as" for it.

Updated

5 months ago
Component: Untriaged → WebExtensions: Untriaged
Product: Firefox → Toolkit

Updated

5 months ago
Whiteboard: downloads

Updated

5 months ago
Component: WebExtensions: Untriaged → WebExtensions: General
Flags: needinfo?(tomica)
(Assignee)

Comment 2

5 months ago
This is not actually related to saveAs.  The problem is when uniquify option is used, we call DownloadPaths.createNiceUniqueFile(), which not only generates a unique name, but also creates the file, prematurely.  The problem is made evident by saveAs as it provides an easy way to cancel the download, but this happens when the download is canceled for any reason.
Assignee: nobody → tomica
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Flags: needinfo?(tomica)
Summary: "uniquify" w/ saveAs:true prematurely creates 0 byte file, and leaves it on disk forever → download option uniquify prematurely creates a 0 byte file

Updated

5 months ago
Priority: -- → P3
Whiteboard: downloads → downloads[triaged]
Comment hidden (mozreview-request)
(Assignee)

Updated

5 months ago
Attachment #8874618 - Flags: review?(aswan)

Comment 4

5 months ago
mozreview-review
Comment on attachment 8874618 [details]
Bug 1367572 - Remove uniquified file when used in combination with saveAs option

https://reviewboard.mozilla.org/r/145962/#review149894

Hm, this is annoying, I think it is intentional that we create the file right away when action is uniquify so that multiple simultaneous downloads don't collide on the same target file, but that isn't feasible when uniquify is combined with saveAs.  This seems just as good as any of the alternatives...

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html:26
(Diff revision 1)
> +  SpecialPowers.setIntPref("browser.download.folderList", 2);
> +  SpecialPowers.setCharPref("browser.download.dir", directory.path);
> +
> +  SimpleTest.registerCleanupFunction(() => {
> +    SpecialPowers.clearUserPref("browser.download.folderList");
> +    SpecialPowers.clearUserPref("browser.download.dir");
> +    directory.remove(true);
> +  });

can you use `SpecialPowers.pushPrefEnv()` instead
(you still need the cleanup function to delete the directory of course)

::: toolkit/components/extensions/test/mochitest/test_chrome_ext_downloads_saveAs.html:110
(Diff revision 1)
> +  result = await extension.awaitMessage("done");
> +
> +  ok(!result.ok, "download rejected if the user cancels the dialog");
> +  is(result.message, "Download canceled by the user", "with the correct message");
> +
> +  ok(!unique.exists(), "unique file was not created.");

nit: its not really true that the file was not created...
Attachment #8874618 - Flags: review?(aswan) → review+
Comment hidden (mozreview-request)

Comment 6

5 months ago
Pushed by tomica@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/a2d49022f1b6
Remove uniquified file when used in combination with saveAs option r=aswan
https://hg.mozilla.org/mozilla-central/rev/a2d49022f1b6
Status: ASSIGNED → RESOLVED
Last Resolved: 5 months ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.