1367572 - download option uniquify prematurely creates a 0 byte file

Status: RESOLVED FIXED

(WebExtensions :: General, defect, P3)

Description

[none11given]

Attachment: 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.

Comment 1

[none11given]

Attachment: webext.zip

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

Comment 2

[Tomislav Jovanovic :zombie]

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.

Comment 3

[Tomislav Jovanovic :zombie]

Attachment: Bug 1367572 - Remove uniquified file when used in combination with saveAs option

Review commit: https://reviewboard.mozilla.org/r/145962/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/145962/

Comment 4

[Andrew Swan [:aswan]]

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...

Comment 5

[Tomislav Jovanovic :zombie]

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

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/145962/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a2d49022f1b6
Remove uniquified file when used in combination with saveAs option r=aswan

Comment 7

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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