Closed
Bug 918466
Opened 11 years ago
Closed 11 years ago
Residual file left after cancelling download
Categories
(Core Graveyard :: File Handling, defect)
Core Graveyard
File Handling
Tracking
(firefox24 unaffected, firefox25 unaffected, firefox26 fixed, firefox27 verified)
RESOLVED
FIXED
mozilla27
Tracking | Status | |
---|---|---|
firefox24 | --- | unaffected |
firefox25 | --- | unaffected |
firefox26 | --- | fixed |
firefox27 | --- | verified |
People
(Reporter: repozlist, Assigned: Paolo)
References
Details
(Whiteboard: [bugday-20130918])
Attachments
(1 file, 2 obsolete files)
6.21 KB,
patch
|
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•11 years ago
|
Whiteboard: [bugday-20130918]
Reporter | ||
Updated•11 years ago
|
Version: 27 Branch → Trunk
Updated•11 years ago
|
OS: Linux → All
Hardware: x86_64 → All
Comment 1•11 years ago
|
||
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
Assignee | ||
Comment 2•11 years ago
|
||
Tracking this bug until we confirm whether this is a Downloads API regression.
Blocks: 907082
Assignee | ||
Comment 3•11 years ago
|
||
Assignee: nobody → paolo.mozmail
Status: NEW → ASSIGNED
Attachment #809843 -
Flags: review?(enndeakin)
Comment 4•11 years ago
|
||
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?
Assignee | ||
Comment 5•11 years ago
|
||
(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)
Updated•11 years ago
|
Attachment #810605 -
Flags: review?(enndeakin) → review+
Assignee | ||
Comment 7•11 years ago
|
||
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: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla27
Comment 9•11 years ago
|
||
I suggest landing this fix on the same milestone as bug 847863 since it is a nontrivial bug introduced by that.
Assignee | ||
Comment 10•11 years ago
|
||
(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?
Assignee | ||
Updated•11 years ago
|
status-firefox24:
--- → unaffected
status-firefox25:
--- → unaffected
status-firefox26:
--- → affected
status-firefox27:
--- → fixed
Updated•11 years ago
|
Attachment #816214 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Comment 12•11 years ago
|
||
I confirm the fix is verified using Latest Aurora 27 on Windows 7 x64, Mac OS 10.8 and Ubuntu 13.10: BuildID: 20131031004003
Updated•8 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•