Closed Bug 302184 Opened 17 years ago Closed 13 years ago

download manager renames existing files by incrementing number suffix if you use auto-download

Categories

(SeaMonkey :: Download & File Handling, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED DUPLICATE of bug 381157

People

(Reporter: hidenosuke, Unassigned)

References

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050725 SeaMonkey/1.0a
Build Identifier: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8b4) Gecko/20050725 SeaMonkey/1.0a

Seamonkey has the same bug as bug 231048 for firefox.


Reproducible: Always
Really? It's new to me that this was patched for Mozilla/SM trunk. HAve I missed
that?
Version: unspecified → Trunk
Try this file:
http://ftp.mozilla.org/pub/mozilla.org/mozilla/source/wintools-19990429.zip

SeaMonkey/2005072605-trunk/WinXP
wintools-19990429.zip (ok)
wintools-19990430.zip
wintools-19990431.zip
(bug 231048 comment 66)

Firefoz/2005072506-trunk/WinXP
wintools-19990429.zip
wintools-19990429(2).zip
wintools-19990429(3).zip

Confirming. I guess this didn't fix SeaMonkey after all.
Status: UNCONFIRMED → NEW
Ever confirmed: true
(In reply to comment #1)
> Really? It's new to me that this was patched for Mozilla/SM trunk. HAve I missed
> that?

Yes :-/ I found it. This happens only with the auto-download feature. Only as
note: This was implemented with Bug 7840.
Summary: download manager renames existing files by incrementing number suffix → download manager renames existing files by incrementing number suffix if you use auto-download
I guess it is fairly easy to port the FF patch from bug 231048.
Flags: blocking-seamonkey1.0a?
Keywords: polish
Flags: blocking-seamonkey1.0a? → blocking-seamonkey1.0a-
Whiteboard: [good first bug]
Attached patch port of fix from Firefox's code (obsolete) — Splinter Review
My very first patch!  Hopefully the first of many.  I simply ported over the new renaming code from the Firefox bug.  I tested that it works as expected with that wintools example file given above.  Now I can just figure out who to request review from...
Assignee: download-manager → napolj2
Status: NEW → ASSIGNED
Attachment #202651 - Flags: review?
Comment on attachment 202651 [details] [diff] [review]
port of fix from Firefox's code

Found someone to request review from from Bonsai.  This is a low risk change so it probably can go into the SeaMonkey 1.0 branch as well as the Trunk.
Attachment #202651 - Flags: review? → review?(cst)
Comment on attachment 202651 [details] [diff] [review]
port of fix from Firefox's code

I'm not qualified to review this.   Also, you patched code in toolkit/, which isn't used by SeaMonkey.  The relevant SeaMonkey code is in xpfe/
Attachment #202651 - Flags: review?(cst)
Attached patch correct patchSplinter Review
Oops!  I attached the patch to the original FX bug; here is my patch to Seamonkey's code.
Attachment #202651 - Attachment is obsolete: true
Attachment #202693 - Flags: review?
Chris, can you review the xpfe code, or should I find someone else?
(In reply to comment #10)
> Chris, can you review the xpfe code, or should I find someone else?
> 

In general I can't review any FF/TB/SM code, so you'll have to find someone else.
Comment on attachment 202693 [details] [diff] [review]
correct patch

Requesting review from cbiesinger for this simple patch, ported from a fix for Firefox.  Chris, thanks for the quick response.
Attachment #202693 - Flags: review? → review?(cbiesinger)
Comment on attachment 202693 [details] [diff] [review]
correct patch

I think I don't want to own this autodownload code, changing request to the original reviewer of this function
Attachment #202693 - Flags: review?(cbiesinger) → review?(neil.parkwaycc.co.uk)
Comment on attachment 202693 [details] [diff] [review]
correct patch

a) I'm not convinced that we want to change from foo-1.ext to foo (2).ext
b) you only need to match once, and concatenate the leaf name from the count
Attachment #202693 - Flags: review?(neil.parkwaycc.co.uk) → review-
(In reply to comment #14)
> (From update of attachment 202693 [details] [diff] [review] [edit])
> a) I'm not convinced that we want to change from foo-1.ext to foo (2).ext

<nit>Actually my patch would rename 'foo.ext' to 'foo(2).ext', w/o any space</nit>, but I see your point.

Having numbers separated by dashes/hyphens is common enough in file names on the web (see comment 2) that we would want to use some other suffix for renaming duplicate files.  There was some discussion in bug 231048 for FX about what renaming scheme to use.  We could try some other scheme like 'foo[2].ext', 'foo{2}.ext', 'foo.2.ext', or 'foo.copy1.ext'.  There rarely are spaces in the file names on the web (since people use _'s or -'s instead), so we could try 'foo (2).ext', or, since % is used an escape character in URLs, 'foo%2.ext'.  Or we could use a prefix like 'Copy(1) of foo.ext'.

Any preferences?

> b) you only need to match once, and concatenate the leaf name from the count
> 
Good catch.  I just copied over the new code from the FX patch, since I thought it would be nice to use the same code in FX and SeaMonkey, but since we're changing the code anyway, I'll rewrite it to be optimized.  Just want to get my grad school applications done first...

BTW, since there is a maximum file name length (at least with Windows), I wonder  what would happen if we automatically downloaded a second version of file with a name that already was at max length, and then tried to append '(2)' or '-1'.  Maybe we could consider that case and add a JS alert when it happens.
(In reply to comment #14)
> (From update of attachment 202693 [details] [diff] [review] [edit])
> a) I'm not convinced that we want to change from foo-1.ext to foo (2).ext

When creating multiple copies of the same file, Windows uses the "foo (2).ext" naming format.
(In reply to comment #16)
>When creating multiple copies of the same file, Windows uses the "foo (2).ext"
>naming format.
That doesn't make it right :-P
How about foo.2.ext?

Now that I look at it again, I'm not convinced by the .tar.gz detection - people will complain that foo.tar.bz3 (or whatever) isn't detected.
(In reply to comment #17)
> (In reply to comment #16)
> >When creating multiple copies of the same file, Windows uses the "foo (2).ext"
> >naming format.
> That doesn't make it right :-P
I would think it does, at least on Windows.
See

<http://mxr.mozilla.org/mozilla/find?text=&kind=text&string=%2FcontentAreaUtils.js>
Both are currently shipped. (Should try and merge them, if possible.)

<http://mxr.mozilla.org/seamonkey/search?string=uniqueFile&case=on&find=%5C.js%24&findi=%5C.h%24&tree=seamonkey>
Should look into merging them, if possible. (See FF bug 296267.)

<http://mxr.mozilla.org/seamonkey/search?string=makeFileUnique&case=on&tree=seamonkey>
Assignee: mozilla → nobody
Status: ASSIGNED → NEW
Depends on: 7840, 231048
QA Contact: download-manager
This got fixed by bug 381157, see http://hg.mozilla.org/comm-central/rev/178d054e3163#l5.151

We probably should file a bug on the issue bug 296267 was for though. Serge, interested in that?
Status: NEW → RESOLVED
Closed: 13 years ago
Keywords: polish
Resolution: --- → DUPLICATE
Whiteboard: [good first bug]
Duplicate of bug: 381157
You need to log in before you can comment on or make changes to this bug.