341771 - download manager does not overwrite existing files after confirmation

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect)

Description

[Christian Ascheberg]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060616 BonEcho/2.0a3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1a3) Gecko/20060616 BonEcho/2.0a3

The download manager does not overwrite existing even if you cofirm to do so

Reproducible: Always

Steps to Reproduce:
1. Finish downloading any file
2. Start downloading the same file into the same folder
3. Say yes to overwrite existing file

Actual Results:  
A second file with an index-number is created
(for example Firefox Setup 1.5.0.4(2).exe)

Expected Results:  
Existing file should be overwritten

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

I have been seeing the same in trunks.  I suspect fallout from : 
https://bugzilla.mozilla.org/show_bug.cgi?id=255942

WinXP SP2
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060616 Minefield/3.0a1,Firefox ID:2006061604 [cairo]

Comment 2

[Brian Polidoro]

How could it be fallout from bug 255942 if the reporter sees this on the 1.8.1 branch and bug 255942 hasn't landed there?

Comment 3

[Jim Jeffery not reading bug-mail 1/2/11]

This apparently regressed between: 
2006060804 build and
2006060904 build 

The download file is overwritten as expected in the 06-08 build and with the 06-09 build - the file is numbered/indexed  -  Fileinstaller(2).exe 

sorry for placing blame on the wrong bug in my comment above.

Comment 4

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to comment #3)
> This apparently regressed between: 
> 2006060804 build and
> 2006060904 build 
> 
> The download file is overwritten as expected in the 06-08 build and with the
> 06-09 build - the file is numbered/indexed  -  Fileinstaller(2).exe 
> 
> sorry for placing blame on the wrong bug in my comment above.
> 

Perhaps this is the bugfix that caused the regress:
https://bugzilla.mozilla.org/show_bug.cgi?id=236514

Comment 5

[Jeff Walden [:Waldo]]

Yeah, this looks like me.  :-\  Look for a fix soon, but not too soon -- I've got other things that are more important at the moment.

Comment 6

[Christian Ascheberg]

This does not happen when you do a right-click on the link and choose "Save Link As...", but it happens when you just left-click.

Comment 7

[Christian Ascheberg]

(In reply to comment #6)
> This does not happen when you do a right-click on the link and choose "Save
> Link As...", but it happens when you just left-click.
> 

I think i just realized that this does not really matter ;-)

But shouldn't this be fixed for beta1 or what do you think?

Comment 8

[Jeff Walden [:Waldo]]

Attachment: Remove the target file before uniquifying target

If we can somehow edit or remove the chosen file if it exists but not create it if it doesn't, the user's file will be deled and he'll get a pseudo-bogus error.  (That's what I think should happen; testing on my system suggests that some sort of caching may occur such that the save actually happens correctly in some cases.)  If this is possible, it's probably worth caring about, but it's enough of an edge case that I think it's a negligible concern for now which can be picked up after b1 if desired.

Comment 9

[Ryan Jones-Ward [:sciguyryan]]

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

Comment 10

[(account disabled for personal attacks)]

Still happening.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20060627 Minefield/3.0a1 - Build ID: 2006062723

Comment 11

[Adam Guthrie]

Fabio, considering that this bug is not resolved as FIXED, the attached patch has not been checked in or even reviewed, and there is no fixed1.8.1 keyword in this bug then I would assume the assertion you just made to be completely valid.

Comment 12

[(account disabled for personal attacks)]

Adam, I presumed (wrongly) that you guys wanted some feedback on the last hourly...
My comment was out of order. Sorry about that...   

Comment 13

[Jeff Walden [:Waldo]]

(In reply to comment #10)
> Still happening.

Generally, bugs don't just fix themselves, so the default assumption for a non-UNCONFIRMED bug is that the bug still exists and needs no confirmation.  Consequently, comments which confirm this are usually unuseful unless preceded by a comment from a developer requesting independent confirmation that the bug does or does not exist.

On the other hand, if you suddenly notice a still-open bug no longer exists, saying so is usually a good idea.  :-)  Just be sure to read through the bug anyway to see if there are any unusual circumstances (e.g., steps to reproduce have changed, the summary is inaccurate, there are comments specifically saying that the bug still exists even tho it may appear that it doesn't, the bug's filed against a different product from the one against which you tested, etc.).

Comment 14

[Mike Connor [:mconnor]]

Comment on attachment 226762 [details] [diff] [review]
Remove the target file before uniquifying target

I'm curious about whether this regresses the original bug, but assuming that it does not (haven't heard a definitive answer from Jeff yet) r=me

Comment 15

[Jeff Walden [:Waldo]]

(In reply to comment #14)
> I'm curious about whether this regresses the original bug, but assuming that
> it does not (haven't heard a definitive answer from Jeff yet) r=me

No, it doesn't.  The original bug was that you could use Save As to save a file to a location, and then while that file was downloading you could save another file to the same location and silently override the first download.  By the time the file removal code is reached in this patch, the user has explicitly chosen to download the file to a specific location.  If a download is already happening to that location, then the user had to say yes to an "overwrite this file?" dialog, explicitly giving permission to download something else to that location.

Patch checked in on trunk, marking FIXED, will ask for branch approval in a sec...

Comment 16

[Jeff Walden [:Waldo]]

Comment on attachment 226762 [details] [diff] [review]
Remove the target file before uniquifying target

This patch should be fairly low-risk; its effects are confined to the case where the user is always asked where to save files, saves a file with one name to a location, and then attempts to save another file with the same name to that location as well.  The patch only does anything if the user, on the second time through, explicitly chooses to overwrite the existing file during download initialization, and in that case it's therefore safe to remove the file.

Comment 17

[Darin Fisher]

Comment on attachment 226762 [details] [diff] [review]
Remove the target file before uniquifying target

a=darin on behalf of drivers

Comment 18

[Jeff Walden [:Waldo]]

Patch checked in on branch.

Also, FTR, I tweaked the comment in the patch a little before checkin to make it more informative -- no code changes, tho.  :-)