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?

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.

(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.

I guess it is fairly easy to port the FF patch from bug 231048.

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...

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.

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/

Oops! I attached the patch to the original FX bug; here is my patch to Seamonkey's code.

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.

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

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

(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>

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?