The default bug view has changed. See this FAQ.

[SeaMonkey] FTP file upload not working

RESOLVED FIXED in seamonkey2.1a1

Status

SeaMonkey
Download & File Handling
--
major
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: Stefan A. Möller, Assigned: neil@parkwaycc.co.uk)

Tracking

({fixed-seamonkey2.0.4})

Trunk
seamonkey2.1a1
fixed-seamonkey2.0.4
Dependency tree / graph
Bug Flags:
blocking1.9.2 -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [See comment 2 and comment 5])

Attachments

(2 attachments, 4 obsolete attachments)

(Reporter)

Description

8 years ago
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.
(Reporter)

Updated

8 years ago
Keywords: regression
Version: unspecified → Trunk
[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.)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: blocking-seamonkey2?
Keywords: regressionwindow-wanted
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...
Created attachment 383841 [details] [diff] [review]
fix for seamonkey repository
[See bug 495291]

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.
Attachment #383841 - Attachment description: fix for seamonkey repository → fix for seamonkey repository [See bug 495291]
Attachment #383841 - Attachment is obsolete: true
(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.
Blocks: 495583
Depends on: 495291
Flags: blocking-seamonkey2.0b1?
(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.

Comment 7

8 years ago
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.
Flags: blocking-seamonkey2.0b1? → blocking-seamonkey2.0b1-
(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.
status1.9.1: --- → ?
Component: Download & File Handling → Download Manager
Flags: blocking1.9.2?
Flags: blocking-seamonkey2?
Flags: blocking-seamonkey2.0b1-
Keywords: regression, regressionwindow-wanted
OS: Windows 2000 → All
Product: SeaMonkey → Toolkit
QA Contact: download → download.manager
Hardware: x86 → All
Summary: FTP file upload not working → FTP file upload not working (Toolkit part)
Whiteboard: [See comment 2 and comment 5]
Summary: FTP file upload not working (Toolkit part) → [SeaMonkey] FTP file upload not working (Toolkit part)
Not blocking as per Kairo's comment 7
Flags: blocking1.9.2? → blocking1.9.2-
(Assignee)

Comment 10

7 years ago
I don't think we need toolkit help for this.
Component: Download Manager → Download & File Handling
Product: Toolkit → SeaMonkey
QA Contact: download.manager → download
Summary: [SeaMonkey] FTP file upload not working (Toolkit part) → [SeaMonkey] FTP file upload not working
(Assignee)

Comment 11

7 years ago
Created attachment 429343 [details] [diff] [review]
Branch patch
[Checkin: Comment 15]

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.
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #429343 - Flags: review?
Attachment #429343 - Flags: approval-seamonkey2.0.4?
(Assignee)

Comment 12

7 years ago
Comment on attachment 429343 [details] [diff] [review]
Branch patch
[Checkin: Comment 15]

D'oh, KaiRo isn't unique :-(
Attachment #429343 - Flags: review? → review?(kairo)

Comment 13

7 years ago
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.
Attachment #429343 - Flags: review?(kairo)
Attachment #429343 - Flags: review+
Attachment #429343 - Flags: approval-seamonkey2.0.4?
Attachment #429343 - Flags: approval-seamonkey2.0.4+

Comment 14

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

kairo@ is unique actually :)
(Assignee)

Comment 15

7 years ago
Pushed changeset c0d669d37469 to releases/comm-1.9.1
Keywords: fixed-seamonkey2.0.4
(Assignee)

Comment 16

7 years ago
Created attachment 429439 [details] [diff] [review]
WIP#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.
Attachment #429439 - Flags: review?(kairo)

Comment 17

7 years ago
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 18

7 years ago
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?
(Assignee)

Comment 19

7 years ago
Created attachment 429540 [details] [diff] [review]
WIP#2

Moving it into common/downloads made the l10n much easier.
Attachment #429439 - Attachment is obsolete: true
Attachment #429540 - Flags: ui-review?(kairo)
Attachment #429439 - Flags: review?(kairo)

Comment 20

7 years ago
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.
(Assignee)

Comment 21

7 years ago
Created attachment 431010 [details] [diff] [review]
Proposed patch
Attachment #429540 - Attachment is obsolete: true
Attachment #431010 - Flags: ui-review?(kairo)
Attachment #429540 - Flags: ui-review?(kairo)

Comment 22

7 years ago
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.
Attachment #431010 - Flags: ui-review?(kairo) → ui-review+
(Assignee)

Updated

7 years ago
Attachment #431010 - Attachment description: WIP#3 → Proposed patch
Attachment #431010 - Flags: review?(iann_bugzilla)

Comment 23

7 years ago
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?
(Assignee)

Comment 24

7 years ago
(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.
(Assignee)

Comment 25

7 years ago
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.
(Assignee)

Comment 26

7 years ago
(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?

Comment 27

7 years ago
(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.
(Assignee)

Comment 28

7 years ago
(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.
(Assignee)

Comment 29

7 years ago
Created attachment 436485 [details] [diff] [review]
With extra goodness
[Checkin: Comment 30]

Not only with extra variables, but uses Services.jsm too :-)
Attachment #431010 - Attachment is obsolete: true
Attachment #436485 - Flags: review?(iann_bugzilla)
Attachment #431010 - Flags: review?(iann_bugzilla)

Updated

7 years ago
Attachment #436485 - Flags: review?(iann_bugzilla) → review+
(Assignee)

Comment 30

7 years ago
Pushed changeset 2668ddc4da4b to comm-central.
Status: ASSIGNED → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → FIXED
Attachment #429343 - Attachment description: Branch patch → Branch patch [Checkin: Comment 15]
Attachment #436485 - Attachment description: With extra goodness → With extra goodness [Checkin: Comment 30]
status1.9.1: ? → ---
Target Milestone: --- → seamonkey2.1a1
You need to log in before you can comment on or make changes to this bug.