Residual file left after cancelling download

RESOLVED FIXED in Firefox 26

Status

defect
RESOLVED FIXED
6 years ago
3 years ago

People

(Reporter: repozlist, Assigned: Paolo)

Tracking

Trunk
mozilla27
Dependency tree / graph

Firefox Tracking Flags

(firefox24 unaffected, firefox25 unaffected, firefox26 fixed, firefox27 verified)

Details

(Whiteboard: [bugday-20130918])

Attachments

(1 attachment, 2 obsolete attachments)

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0 (Beta/Release)
Build ID: 20130919030202

Steps to reproduce:

1. Click on a link to download
2. Download started, and both original and .part file created
3. Cancel said download


Actual results:

The .part file was removed, but the original blank file remains in the download directory.


Expected results:

No temporary file should remain if a download is cancelled.
Whiteboard: [bugday-20130918]
Version: 27 Branch → Trunk
OS: Linux → All
Hardware: x86_64 → All
This bug seems to be related to bug 847863. Disabling prefs "browser.download.useJSTransfer" which is enabled by the 8th patch of that bug could solve this problem.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Tracking this bug until we confirm whether this is a Downloads API regression.
Blocks: 907082
Posted patch The patch (obsolete) — Splinter Review
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #809843 - Flags: review?(enndeakin)
Comment on attachment 809843 [details] [diff] [review]
The patch

>+add_task(function test_cancel_midway_tryToKeepPartialData()
>+{
>+  let download = yield promiseStartDownload_tryToKeepPartialData();
>+
>+  do_check_true(yield OS.File.exists(download.target.path));
>+  do_check_true(yield OS.File.exists(download.target.partFilePath));
>+
>+  yield download.cancel();
>+  yield download.removePartialData();
>+

What happens if you don't call removePartialData? Should the empty file still be around?

Should we also have a test that handles the download failing?
Posted patch Updated patch (obsolete) — Splinter Review
(In reply to Neil Deakin from comment #4)
> What happens if you don't call removePartialData? Should the empty file
> still be around?

Good question. We didn't consistently handle placeholders previously, and
currently we delete the placeholder while the download is stopped.

However, we might also decide to keep it, to reserve the file name. The
downside is that having a zero-sized file with the final extension kept in
place for a long time might be confusing.

I think we should investigate this further, so I filed bug 921052 and didn't
add a test preferring one case over the other. This case mostly concerns what
we show as "paused" downloads, that are a minority, so I don't think we need
to rush.

> Should we also have a test that handles the download failing?

I added a check to the existing source failure test, but I also realized that
we don't have failure tests for tryToKeepPartialData. I've filed the follow-up
bug 921054.
Attachment #809843 - Attachment is obsolete: true
Attachment #809843 - Flags: review?(enndeakin)
Attachment #810605 - Flags: review?(enndeakin)
Attachment #810605 - Flags: review?(enndeakin) → review+
Duplicate of this bug: 921904
Landed with a change to error reporting because of failing tests:

https://hg.mozilla.org/integration/fx-team/rev/4888e018458d
https://hg.mozilla.org/mozilla-central/rev/4888e018458d
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
I suggest landing this fix on the same milestone as bug 847863 since it is a nontrivial bug introduced by that.
Posted patch Landed patchSplinter Review
(In reply to Xidorn Quan from comment #9)
> I suggest landing this fix on the same milestone as bug 847863 since it is a
> nontrivial bug introduced by that.

Agreed.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 847863
User impact if declined: Regression described in comment 0
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): Low, has tests
String or IDL/UUID changes made by this patch: None
Attachment #810605 - Attachment is obsolete: true
Attachment #816214 - Flags: approval-mozilla-aurora?
Attachment #816214 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I confirm the fix is verified using Latest Aurora 27 on Windows 7 x64, Mac OS 10.8 and Ubuntu 13.10:
BuildID: 20131031004003
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.