251625 - .part files not removed when cancelling or removing downloads

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P2)

Description

[Michael Stather]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040711 Firefox/0.9.0+
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8a2) Gecko/20040711 Firefox/0.9.0+

The title says it all *g.
When cancelling and then removing a download the .part file stays in my download
folder.

Reproducible: Always
Steps to Reproduce:
1.
2.
3.

Comment 1

[Ali Ebrahim]

Weird, this doesn't happen for me. I'm using branch builds, though.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Reporter please try with a current branch build. Your UA shows a trunk build. I
can't reproduce it with Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.7.2) Gecko/20040814 Firefox/0.9.1+

Comment 3

[PikeUK]

I can reproduce this on:

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.8b) Gecko/20050206 Firefox/1.0+

Looks like the suite has the same problem, see Bug 256753.

Comment 4

[vilmanis]

Confimed using:
Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.7.5) Gecko/20041107 Firefox/1.0
This is using the download in big bold letters on the front page :)

Also, removes 'completed' file if present - I suspect that the 'cancel' action
attempts to remove "filename.ext" instead of "filename.ext.part"

Steps to reproduce:
1) Start a download, then exit firefox before its done.
2) Download the file to the same directory using something else, eg wget.
   There should now be a "filename.ext" and a "filename.ext.part" file
3) Start firefox again, bring up the download manager and click cancell on the file
4) Result: "filename.ext" (the one which was downloaded from another program) is
deleted, "filename.ext.part" (the incomplete firefox download) remains

This can be exceedingly annoying - I had 25 ~20MB files downloading, when
windoze packed up... I went to nix, downloaded everything again, then back to
windows.. clean up my download manager, and all the files disappear again :(
there goes another few hours....

Comment 5

[jack linkowski]

I believe I have problem similar to "Additional Comment #4".
1. open existing file on your PC - in my case it is excel spreadsheet opened
with Excel
2. intiate download of a file - for a name to sore select the same file as
opened in step 1.  Confirm that you want to replace the file
3. "Downloads" firefox window displays Excel icon, filename, and "Starting ..."
4. Nothing happens probably can wait forever (I tried few minutes)
5. close the file within excel
6. "Downloads" firefox window stays the same as in step 3 above.
7. Click "cancel" within "Downloads" firefox window
8. The file opened in step 1 is gone from the Windows folder

Win2K build 2195,  Firefox 1.0.2 (was also happening with 1.0.1),

Comment 6

[Peter van der Woude [:Peter6]]

*** This bug has been marked as a duplicate of 255773 ***

Comment 7

[PikeUK]

Peter, why is this a dupe of Bug 255773, neither the titles nor the descriptions
seem to match, or are you saying they are the same cause?

Comment 8

[Peter van der Woude [:Peter6]]

(In reply to comment #7)
> Peter, why is this a dupe of Bug 255773, neither the titles nor the descriptions
> seem to match, or are you saying they are the same cause?
my mistake.

Comment 9

[Michael]

Actually blame me, he trusted the comment I made in that bug. There were a
number of bugs filed that were related to this issue and I misread that bug's
description even though the summary is pretty clear. This bug's cause appears to
be with the core not Firefox's download manager.

Comment 10

[Ali Ebrahim]

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050527
Firefox/1.0+

I see this bug. Since I haven't been able to find anything to dupe this against
in Core, Toolkit or Firefox, I'm marking this one as NEW.

RE comment 9, on what basis are you assuming that the problem lies with Core and
not with Firefox? I'm not saying you're wrong, I'm just not sure that its
entirely obvious (to me, anyway). Anyway, if it needs to go in Core or Toolkit
it can always be moved.

Comment 11

[Michael]

Not from looking at the code but just observation of when the bug first
appeared. It first appeared after they merged the branch back into the trunk
after the Fx 1.0 release. I don't think it matters too much what its been filed
under, whoever decides to work on this bug can sort that out.

Comment 12

[Aaron Slunt]

IMO, .part files should only be cleaned up when called so by the download
manager (when the user hits clean-up for the download).

Comment 13

[Mike Connor [:mconnor]]

Attachment: patch

Comment 14

[Benjamin Smedberg]

Comment on attachment 186980 [details] [diff] [review]
patch

Please add a comment that this uses an implementation-specific hack of
nsExternalHelperAppService.

Comment 15

[chris hofmann]

Comment on attachment 186980 [details] [diff] [review]
patch

a=chofmann

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

erm, exthandler notifies you of the exact file it creates. see
http://lxr.mozilla.org/seamonkey/source/uriloader/exthandler/nsExternalHelperAppService.cpp#2092
and/or http://lxr.mozilla.org/seamonkey/source/uriloader/base/nsITransfer.idl#90

(sorry for not mentioning that before...)

Comment 17

[Mike Connor [:mconnor]]

checked in yesterday, marking fixed.

Comment 18

[Christian :Biesinger (don't email me, ping me on IRC)]

filed Bug 298701 on comment 16

Comment 19

[William Bumgarner [:zsinj]]

Could bug 292692 be related to this one?

Comment 20

[Mike Connor [:mconnor]]

*** Bug 292692 has been marked as a duplicate of this bug. ***

Comment 21

[William Bumgarner [:zsinj]]

*** Bug 298427 has been marked as a duplicate of this bug. ***

Comment 22

[Bill Gianopoulos [:WG9s]]

I am reopening because the fix that was checked in does not work for WIN32.  I'm
not sure if it would be better to open a new bug.

After doing some digging I suspect that the issue might be that this code
appears to remove the file before the stream is closed.  That might not be
allowed by the OS.  Perhaps a better approach would be to make the user clicked
on cancel case call Cancel with a specific reason code indicating the user
cancelled the download, which could be checked for in
nsExternalAppHandler:Cancel upon entry, and in that case set
mReceivedDispostionInfo to PR_FALSE and this would permit the normal processing
to properly remove the file after the stream is closed.

Comment 23

[Mike Connor [:mconnor]]

Attachment: replace original kludge with the new nsITransfer bits

tested fairly well on Windows and Linux

Comment 24

[Benjamin Smedberg]

Comment on attachment 188513 [details] [diff] [review]
replace original kludge with the new nsITransfer bits

Does this solve the reported problem of windows locking the file?

Comment 25

[Bill Gianopoulos [:WG9s]]

(In reply to comment #24)
> (From update of attachment 188513 [details] [diff] [review] [edit])
> Does this solve the reported problem of windows locking the file?
> 

Not sure that was the problem.  It was just a guess on my part.

Comment 26

[Mike Connor [:mconnor]]

Yeah, I think we were attempting to remove the file before the download was
actually cancelled, which is bogus, and I'm surprised it worked on Linux.  In
any case, I went through the cases where this failed on Windows and it fixed all
of those cases.

Comment 27

[Adam Guthrie]

*** Bug 300645 has been marked as a duplicate of this bug. ***

Comment 28

[Serge Gautherie (:sgautherie)]

Attachment: (Cv1) <nsDownloadManager.cpp> comment fix [Checkin: Comment 30]

Copy/Port comment fixup from bug 311098.

'approval-1.8.1=?': (Toolkit only)
Trivial, no risk.
(To stay in sync'.)

Comment 29

[Mike Connor [:mconnor]]

Comment on attachment 232399 [details] [diff] [review]
(Cv1) <nsDownloadManager.cpp> comment fix
[Checkin: Comment 30]

grammar correction in comments isn't something worth syncing at this stage

Comment 30

[:Gavin Sharp [email: [e-mail]]]

mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp 	1.69