User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20081202 SeaMonkey/2.0a2pre Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b3pre) Gecko/20081202 SeaMonkey/2.0a2pre With SM 2.0a I cannot upload files to a FTP server any more. Reproducible: Always Steps to Reproduce: 1. Open your FTP location and log in. 2. Menu "File -> Upload file" 3. Choose some file in the file picker. 4. Click the "Open" button. Actual Results: File picker closes, but otherwise nothing happens. There is no network activity. Expected Results: File should be uploaded to the server.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.2a1pre) Gecko/20090127 SeaMonkey/2.0a3pre] (experimental/_m-c_, home, optim default) (W2Ksp4) (http://hg.mozilla.org/mozilla-central/rev/11e029835944 +http://hg.mozilla.org/comm-central/rev/daa8b76471bf) Confirming. Nothing in the Error Console either. (Firefox does not have this feature.)

File upload is broken in comm-central repository because of missing interface nsIProgressDialog. In seamonkey repository and mozilla CVS it is located in xpfe/components/download-manager/public/nsIProgressDialog.idl. It seems that this file was never imported to mozilla-central or comm-central repository. There are still some references to this interface in mozilla-central http://mxr.mozilla.org/mozilla-central/search?string=nsIProgressDialog so I'm not sure if this file was omitted intentionally. If yes then we probably need to define the interface somewhere in the comm-central. Someone more familiar with seamonkey should decide...

Upload is broken in seamonkey repository as well, but for another reason. This patch fixes it.

Comment on attachment 383841 [details] [diff] [review] fix for seamonkey repository [See bug 495291] How did you create this patch? Neil already fixed this 3 weeks ago in bug 495291.

(In reply to comment #2) Indeed! Current CVS (still) has: http://mxr.mozilla.org/mozilla/source/xpfe/components/download-manager/public/ Makefile.in 1k Aug 18 2007 nsIDownload.idl 4k Aug 17 2007 nsIDownloadManager.idl 8k Jul 7 2005 nsIDownloadProgressListener.idl 3k Apr 6 2005 nsIProgressDialog.idl 3k Aug 14 2007 mozilla-central rev 2 had only: http://hg.mozilla.org/mozilla-central/file/10cab3c34c28/xpfe/components/download-manager/public/ 2007-03-22 16:01 -0700 1943 Makefile.in 2007-03-22 16:01 -0700 8274 nsIDownloadManager.idl 2007-03-22 16:01 -0700 3916 nsIDownloadProgressListener.idl The missing IDL was added by http://hg.mozilla.org/mozilla-central/rev/fd843d9a910b and recently removed by bug 495583. I don't think this can explain the current bug report, but I think the current (IDL) situation is odd (= additional regression) and should be looked at here or (rather) in bug 495291 or (probably) a dedicated bug.

(In reply to comment #4) > How did you create this patch? Ah! This is a cvs patch actually: if this is actually needed on 1.8.1 branch too (which I'm not sure of), you should just ask for approval in bug 495291.

xpfe/components/download-manager/ isn't built on current SeaMonkey any more, so it doesn't really matter if a file is in there or not. That said, we won't block beta for FTP upload.

(In reply to comment #7) > xpfe/components/download-manager/ isn't built on current SeaMonkey any more, so > it doesn't really matter if a file is in there or not. The point is not the location, but that a file seems to be missing in the tree! Moving this bug to Toolkit.

Not blocking as per Kairo's comment 7

I don't think we need toolkit help for this.

This is just a copy of the 1.8 branch XPFE nsIProgressDialog.idl which is enough to get the dialog to work. I couldn't find a better place to put it.

Comment on attachment 429343 [details] [diff] [review] Branch patch [Checkin: Comment 15] D'oh, KaiRo isn't unique :-(

Comment on attachment 429343 [details] [diff] [review] Branch patch [Checkin: Comment 15] r+a=me for 1.9.1 only. For trunk, we need to have a larger patch, no matter if we need the IDL or not, we should have everything reviewed at once.

(In reply to comment #12) > (From update of attachment 429343 [details] [diff] [review]) > D'oh, KaiRo isn't unique :-( kairo@ is unique actually :)

Pushed changeset c0d669d37469 to releases/comm-1.9.1

This is a straight port of the upload progress dialog, but with less cruft, so although it works, it's still ugly. At the very least I need to use the same time and size formatting as the download progress dialog (speed too but I can't see where that is handled right now). I could probably change the cancel button into a stop icon but I was too lazy (if you look carefully you might notice that I haven't even localised it yet). I'm not sure what to do with the source and target though, as the options available for downloads don't make sense here.

Comment on attachment 429439 [details] [diff] [review] WIP#1 Not sure if I'm the right person to review this patch, I don't really that deeply into code. Also, I'm not sure how much value it is to review unlocalizable code.

Comment on attachment 429439 [details] [diff] [review] WIP#1 hmm, any chance we can put it in common/downloads so it's found in one location with the similar download progress stuff? Also, can we name it uploadProgress.xul/.js to state that it's upload only?

Moving it into common/downloads made the l10n much easier.

Comment on attachment 429540 [details] [diff] [review] WIP#2 Hrm, I really would have liked UI that is more like the download progress windows we have on trunk, but I won't insist on that as I don't want to get into another flamewar about anything like this.

Comment on attachment 431010 [details] [diff] [review] Proposed patch IMHO, the "From" isn't really needed, as I'd colloquially say I'm "uploading foo.zip to example.org" and we could use exactly that wording in the dialog, but else it looks good to me.

Comment on attachment 431010 [details] [diff] [review] Proposed patch Could DownloadUtils getDownloadStatus be made use of? Is it worth having globals for the status and size elements?

(In reply to comment #23) > (From update of attachment 431010 [details] [diff] [review]) > Could DownloadUtils getDownloadStatus be made use of? I probably didn't bother because we don't use it for downloads either, but I can look into it, as updates.js seems to use it in a similar way. > Is it worth having globals for the status and size elements? Yeah, I could do that.

getDownloadStatus doesn't format in the way we want, it gives us [remaining] - [transferred] of [total] ([rate]) but we want [transferred] of [total] ([rate]) [elapsed] [remaining] as separate fields.

(In reply to comment #23) > (From update of attachment 431010 [details] [diff] [review]) > Is it worth having globals for the status and size elements? Also the elapsed, percent and progressmeter elements, no?

(In reply to comment #26) > (In reply to comment #23) > > (From update of attachment 431010 [details] [diff] [review]) > > Is it worth having globals for the status and size elements? > Also the elapsed, percent and progressmeter elements, no? I was looking at ones used in multiple locations but yes, those as well.

(In reply to comment #27) > (In reply to comment #26) > > (In reply to comment #23) > > > (From update of attachment 431010 [details] [diff] [review]) > > > Is it worth having globals for the status and size elements? > > Also the elapsed, percent and progressmeter elements, no? > I was looking at ones used in multiple locations but yes, those as well. Well, I don't see how centralising all the progressmeter updates in setPercent absolves it from using a global variable, but I guess I'm already inconsistent with my use of gBundle, so I might as well make them all into variables.

Not only with extra variables, but uses Services.jsm too :-)

Pushed changeset 2668ddc4da4b to comm-central.