24867 - basic UI for FTP upload (menu) not implemented

Status: RESOLVED FIXED

(Core Graveyard :: Networking: FTP, enhancement, P1)

Description

[Judson Valeski]

I just checked in the FTP upload code for the protocol (bug 18977). You can now 
call AsyncWrite() to upload data. However, we need UI for this. 4.x allowed the 
user to drag and drop a file into a directory. To do this in our new tree widget 
world, we need some notion of directory focus (via mousover) in the widget (not 
sure if this exists, but I suspect it does).

Comment 1

[Chris McAfee]

sounds like post-beta, m15

Comment 2

[leger]

Moving all UE/UI bugs to new component: User Interface: Design Feedback
UE/UI component will be deleted.

Comment 3

[Asa Dotzler [:asa]]

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

Comment 4

[don]

Move to M16 for now ...

Comment 5

[Eli Goldberg]

QA Assigning non-confidential New/Assigned User Interface: Design Feedback bugs 
to Matthew Thomas ([e-mail]).

Matthew Thomas is now the QA owner for the User Interface: Design Feedback 
component. (Bugs that involve UI issues in the Netscape-branded Mozilla browser 
should continue be QA assigned to [e-mail].)

Comment 6

[leger]

Putting on [nsbeta2-] radar. Not on the Seamonkey "in" list

Comment 7

[Gagan]

this should be an easy one for someone to take up as a weekend project-- adding 
helpwanted and to [e-mail]. 

Comment 8

[Gagan]

this should be in. 

Comment 9

[Asa Dotzler [:asa]]

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

Comment 10

[Asa Dotzler [:asa]]

adding dogfood keyword by way of bug 48548

------ Additional Comments From Frank Tang 2000-08-10 23:57 -------
mark this as dogfood since jar ask me to put it there for all the reason I don't 
use SeaMonkey.

Comment 11

[Jaime Rodriguez, Jr.]

Adding JohnG, and Michael to cc: list. PDT needs input from Marketing

a. This is a feature.
b. Wasn't this deliberately cut?

Comment 12

[leger]

Putting on [dogfood-] radar since already [nsbeta3+].  I will bring this but to 
PDT today.

Comment 13

[leger]

This feature is cut!  Putting on [nsbeta3-] radar.

Comment 14

[Alan S. Jones]

adding helpwanted keyword

Comment 15

[Alan S. Jones]

Question:  I heard that the back end for this was already done and all was 
needed is the UI?  Bug 18977

I was sorry to see this cut after the publishing feature was also pushed back.

maybe an outside developer will step up.

Comment 16

[Judson Valeski]

backend is there, just need the UI.

Comment 17

[Kathleen :Brade]

hmmmm... Composer was going to rely on this as an alternative for Publish...

Comment 18

[Frank Tang]

4.x not only allow user to drag & drop a file to a ftp directory for ftp upload, 
it also show a "File:Upload File..." if you open a ftp directory listing window. 
I know the drag&drop is hard to do, but at least can we do the "File:Upload 
File..."  ?
How can this be dogfood- ever? How can release team use SeaMonkey to upload the 
bits to the ftp staging server?
remove [dogfood-]. add rtm to keyword.

Comment 19

[Jim Roskind]

I don't think that the browser is the ftp upload tool-of-choice at this point.
Dogfood relates to day-to-day usage, and ftp upload does not show as a dogfood
issue.  My bet (wild guess) is that the release team isn't using a browser to do
their push operations anyway.
marking dogfood-minus.
This is a futured feature request, so I'm also adding beta3 and rtm minus.

Comment 20

[John Dobbins]

I'm starting work on TranZilla, a project to bring 2 levels of FTP support to 
Mozilla. Level 1 support is basic FTP support. I'll be submitting this to 
Mozilla.org to become part of Navigator.
Level 2 support will be TranZilla, a full featured Cross platform FTP client 
that will intergrate with Mozilla and be available as a seprate download. The 
homepage for TranZilla is http://tranzilla.mozdev.org

Comment 21

[(not reading, please use [e-mail] instead)]

nhotta:  this is the feature I was looking for.

Comment 22

[(not reading, please use [e-mail] instead)]

adding shaver to the cc list.

Comment 23

[Mike Shaver (:shaver emeritus)]

I'm starting to work on this, but I can't get an output stream from the FTP
channel object:

getting output stream for channel: ###!!! ASSERTION:
nsFTPChannel::OpenOutputStream: 'Not Reached', file
/s/1/mozilla/netwerk/protocol/ftp/src/nsFTPChannel.cpp, line 300
###!!! Break: at file /s/1/mozilla/netwerk/protocol/ftp/src/nsFTPChannel.cpp,
line 300
FAILED: [Exception... "Component returned failure code: 0x80004001
(NS_ERROR_NOT_IMPLEMENTED) [nsIChannel.openOutputStream]"  nsresult: "0x80004001
(NS_ERROR_NOT_IMPLEMENTED)"  location: "JS frame ::
/s/1/mozilla/netwerk/test/TestFTPUpload.js :: upload :: line 23"  data: no]

Help me, Obi-wan!  I need to get my hands on an nsIScriptableInputStream so that
I can write to the FTP channel from JS.

(Alternatively, I'd like to know how to wire things up so that an FTP
channel/URL/connection/socket is the sink for a file
channel/URL/connection/transport's source, perhaps just by matching up stream
interfaces.)

Comment 24

[Mike Shaver (:shaver emeritus)]

woo.  bryner showed me the light, and I'm now shipping data from files to FTP.

I think I have JS code that will upload correctly, but the lack of an event
queue in xpcshell is cramping my style.  I'll take another swing this weekend
and see if I can't get it wedged into the browser.

It's not very hard at all, now that jud has done the heavy AsyncWrite lifting.
rjc, I'll take this, unless you object, and add it to my collection. (If someone
else wants to take my upload("file:///local/file.txt",
"ftp://host/dir/remote-file.txt") code and do the wedging, just mail me.)

Comment 25

[Mike Shaver (:shaver emeritus)]

Attachment: chrome goop to test (currently crashing M18)

Comment 26

[Mike Shaver (:shaver emeritus)]

I've got it to the point where, with 9 statements of JS (plus error handling and
debug noise), I can kick off an FTP upload.  And then it crashes my M18 in
libnecko somewhere.

I'll try more tonight from home, where there is a debug build and the like, but
in case anyone else wants to take a swing at it, you should:

- unpack that tarball in $MOZILLA_FIVE_HOME/chrome
- add
      content,install,url,resource:/chrome/ftpupload/content/
  to installed-chrome.txt
- run regchrome
- visit chrome://ftpupload/content/ (the world's ugliest XUL!)
- enter a local file URL[*] in the left box
- enter an FTP URL, like ftp://localhost/pub/incoming/remote-name.txt, in the
  right box
- click the huge freaking button
- (crash)

[*] I think any URL will actually work, which is kinda neat, but let's start
with these baby steps.

Comment 27

[Mike Shaver (:shaver emeritus)]

I know where it's crashing now.  But I'm not smart enough to decipher netwerk
and figure out what it means:

#0  nsOnStartRequestEvent::HandleEvent (this=0x8725d08) at
/s/1/mozilla/netwerk/base/src/nsAsyncStreamListener.cpp:210

That's this line:

rv = receiver->OnStartRequest(mChannel, mContext);

and receiver is null (we get it from mListener->GetReciever()).

It looks like the nsAsyncStreamListener is never having its init() method
called, which means that it never gets a receiver set.

Should I be calling init() myself?  Where do I get a handle to the
AsyncStreamListener?  What should be the receiver?  (Perhaps the fileObserver?)

For the busy, here's the pared down version of what I'm doing:

- get the IO service
- create channels for local file: and remote ftp: URIs
- get a stream listener (``streamListener'') for the remote ftp: channel
- get an input stream (``fileInputStream'') for the local file: channel
- set the transfer count on the ftp: channel
- get a stream observer for the file: channel (``fileObserver'')
- wire them all together:
  ftpChannel.asyncWrite(fileInputStream, fileObserver, null);

valeski? gagan? can you help me figure this out?

Comment 28

[Judson Valeski]

if you set NSPR_LOG_MODULES=nsFTPProtocol:5 in your env then try running your
upload, you'll get FTP command debug output which will help determine which
codepath you're going down. FTP shouldn't prompt asyncobserver usage (it has
it's own homegrown events for this) unless there was a problem negotiating the
protocol. If the log indicates protocol error, then something has changed since
I put the STOR code in (It used to work for me) and rjc or I will need to hammer
it out. If I have a chance today, I'll debug myself.

Comment 29

[Mike Shaver (:shaver emeritus)]

Doesn't look like it's even attempting to store:
3076[85ef128]: nsFtpConnectionThread::Process() started for 854c320 (spec
=ftp://localhost/pub/incoming/remote-foo.txt)
OK
3076[85ef128]: 854c320 Process() - READ_BUF - read "220 loonie FTP server
(Version wu-2.6.0(1) Mon Feb 28 10:30:36 EST 2000) ready.
" (81 bytes)
3076[85ef128]: 854c320 Process() - S_USER -
3076[85ef128]: 854c320 Writing "USER anonymous
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "331 Guest login ok, send
your complete e-mail address as password.
" (68 bytes)
3076[85ef128]: 854c320 Process() - S_PASS -
3076[85ef128]: 854c320 Writing "PASS mozilla@
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "230 Guest login ok, access
restrictions apply.
" (48 bytes)
3076[85ef128]: 854c320 Process() - S_SYST -
3076[85ef128]: 854c320 Writing "SYST

3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "215 UNIX Type: L8
" (19 bytes)
3076[85ef128]: 854c320 Process() - S_PWD -
3076[85ef128]: 854c320 Writing "PWD
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "257 "/" is current directory.
" (31 bytes)
3076[85ef128]: 854c320 Process() - S_PASV -
3076[85ef128]: 854c320 Writing "PASV
" (addr is v4)
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "227 Entering Passive Mode
(127,0,0,1,16,123)
" (46 bytes)
3076[85ef128]: nsFTPChannel::GetContentType() returned text/plain
3076[85ef128]: 854c320 Process() - S_MODE -
3076[85ef128]: 854c320 Writing "TYPE A
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "200 Type set to A.
" (20 bytes)
3076[85ef128]: 854c320 Process() - S_CWD -
3076[85ef128]: 854c320 Writing "CWD /pub/incoming/remote-foo.txt
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "550
/pub/incoming/remote-foo.txt: No such file or directory.
" (62 bytes)
3076[85ef128]: 854c320 Process() - S_RETR -
3076[85ef128]: 854c320 Writing "RETR /pub/incoming/remote-foo.txt
"
3076[85ef128]: SUCCEEDED
3076[85ef128]: 854c320 Process() - READ_BUF - read "550
/pub/incoming/remote-foo.txt: No such file or directory.
" (62 bytes)
3076[85ef128]: 854c320 Process() - ERROR
3076[85ef128]: nsFtpConnectionThread::Process() ended for 854c320 (spec
=ftp://localhost/pub/incoming/remote-foo.txt)

(I then descend into assertion hell.)

Is the asyncWrite call not enough to make it go into STOR mode?

Comment 30

[Judson Valeski]

I feared this... AsyncWrite() has bit-rotted. you're right, we never get to a
STOR, and yes, AsyncWrite() should be enough. we're trying to download the
_uploaded_ file name :-/.

rjc, can you debug this? I can't be counted on this week for code :-/.

Comment 31

[Mike Shaver (:shaver emeritus)]

I've seen the enemy, and it is R_cwd.
http://lxr.mozilla.org/seamonkey/source/netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp#1135

If CWD fails -- which it always will if we're specifying the complete file path
for upload, not just the directory into which to put the file -- then we
unconditionally go into S_RETR.

What I propose:

If we're doing a PUT, and CWD _succeeds_, then the user has just specified the
directory, and we need to inherit the leafName from the originating URI.  Do we
have access to that at this point?

If it fails, then we have all the information we need, and we should just
proceed to STOR /pub/incoming/remote-foo.txt.

Make sense?

Comment 32

[Mike Shaver (:shaver emeritus)]

Jud and I decided that we don't even need to call CWD, because we're not
interested in guessing what to do when someone tries to upload to a directory.

So, we just go straight to STOR for the PUT case, and I have a patch for that
momentarily.  With _that_ patch, though, I start to blow assertions in
nsFileChannel::OnStartRequest:

###!!! ASSERTION: wrong thread calling this routine: 'mInitiator ==
PR_CurrentThread()', file
/s/1/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp, line 630
###!!! Break: at file /s/1/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp,
line 630
###!!! ASSERTION: No listener...: 'mRealListener', file
/s/1/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp, line 632
###!!! Break: at file /s/1/mozilla/netwerk/protocol/file/src/nsFileChannel.cpp,
line 632

Thoughts?

Comment 33

[Mike Shaver (:shaver emeritus)]

Attachment: Patch to fix FTP upload path.

Comment 34

[Mike Shaver (:shaver emeritus)]

Turns out you don't want the fileChannel to observe the asyncWrite -- it gets
very sad when its OnRequestStart is called from the ``wrong'' thread.

Looking for r= and sr= on my FTP-protocol patch.

Comment 35

[Mike Shaver (:shaver emeritus)]

Attachment: Better patch: FTP fixes and fileChannel assertion removal

Comment 36

[Judson Valeski]

ftp changes look good to me, file changes look scary, check w/ warren.

Comment 37

[Mike Shaver (:shaver emeritus)]

(Didn't you tell me to make those fileChannel changes? =) )

Adding warren for review.

Comment 38

[Judson Valeski]

no, I suggested removing the assertion in the FTP thread that was moaning about
someone's probably not receiving data.

Comment 39

[Mike Shaver (:shaver emeritus)]

Ah, so.  I'll make _those_ changes instead, and report back.

Sorry about that.

Comment 40

[Martin Horwath]

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

Comment 41

[Keyser Sose]

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

Comment 42

[Doug Turner (:dougt)]

Mike, please reassign back to you if you are going to fix this?

Comment 43

[Zach Lipton [:zach]]

Chaning the qa contact on these bugs to me. MPT will be moving to the 
owner of this component shortly. I would like to thank him for all his hard 
work as he moves roles in mozilla.org...Yada, Yada, Yada...

Comment 44

[Doug Turner (:dougt)]

Well, I wish I could get to this, but alas I am doomed.  Target Milesone = Future.

Help always wanted.

Comment 45

[rubydoo123]

Doug, the editor team was relying on this for publish, is there any way you 
could do this within the next milestone?

Comment 46

[Mike Shaver (:shaver emeritus)]

I got stuck in FTP assertion land, but I'll try again if I get some time soon. 
I have high hopes for dougt's networking changes.

(And it might as well languish on my buglist, eh?)

Comment 47

[Doug Turner (:dougt)]

beppe, FTP should allow you to upload programatically.  This bug is in regard to 
a drag and drop(?) ui.

shaver, if you have problems with uploading, let me know quickly as I want this 
feature in 0.9 really bad!  :-)

Comment 48

[rubydoo123]

right, our anticipation is to be able to drag to the destination location

Comment 49

[Peter Lubczynski]

Without fixing this bug, how is it possible for one to Upload a file to a FTP 
server? Also, when did the new HTML "Apache-style" FTP UI replace the tree 
widget?

Comment 50

[Håkan Waara]

Peter, my understanding is that Bradley Baetz is fixing the dir viewer to be
faster than before. In the meantime, they have switched over to the old-style
FTP listing. (cc bbaetz)

Comment 51

[Bradley Baetz]

err, no I'm not :) I'm playing with the backend (see bug 78148), but the only
changes will be API level changes. I may end up getting the directoryviewer to
use <outliner>, but since ftp uses the html interface by default now, that won't
help.

The html interface is real html, which is produced by nsIndexedToHTML. (I have
plans to make it a bit nicer, and add a stylesheet in, but I suspect that since
it looks like an ftp uri, it won't have access to anything in chrome. Thats just
something that would be Nice - I don't really have plans to do it ATM)

Comment 52

[Kathleen :Brade]

the patches here are pretty outdated but may still be relevant to dougt

Comment 53

[Doug Turner (:dougt)]

Attachment: inital cut browser patch for ftp DD.

Comment 54

[Doug Turner (:dougt)]

That last patch need 92837 to be fixed.  The patches are there.

What is left is (a) confirmation dialog, (b) a upload progress dialog, and (c) a
force refresh of the content area after uploader.  

Anyone care to take it from here?

Comment 55

[Christopher Aillon (sabbatical, not receiving bugmail)]

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

Comment 56

[Matthias Versen [:Matti]]

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

Comment 57

[lohphat]

Will drang-n-drop[tm, us reg. pat. off.] be included as part of the UI?

Comment 58

[Matthias Versen [:Matti]]

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

Comment 59

[Mike Schuette]

I've picked this up in my spare time and have a new DD patch (the one from
7/31/01 no longer works.)

This is my first major mozilla work, plus I'm only doing this in my spare time,
so I'm not entirely sure how long it'll take me to implement the confirmation
dialog, download dialog, etc.

In any case, if anyone is interested in seeing what I have so far, let me know.
 Otherwise, I'll just plug away at this quietly until I have something I like.

Comment 60

[Kathleen :Brade]

Thanks for the note here.  I'd love to see your progress on this.  I'd especially 
like to see us get a barebones version checked in (if it's not too late for 
that).

Mike Schuette--can you attach a patch when you have a chance?

Also, I presume that the above patches are now old and should be obsoleted.  If 
anyone has any objections to obsoleting the above patches, please speak now!

Comment 61

[Mike Schuette]

Attachment: Slightly revised DD patch

This patch is pretty barebones, but it works.  Log into an ftp site, DD a file
over the content area.	Answer OK to a confirm() and the file sends, but no
upload progress dialog (yet.)  After the file is done, it refreshes the content
area.

Incomplete:
- Like I said, there's no progress dialog, hence no way to cancel an upload. 
The progress dialog will take quite a bit longer than this simple revision.
- The File menu should have an "Upload File ..." menu when we're on an ftp
site.  I'm not sure of how to make the File menu update when we reach an FTP
site, so any hints there would be helpful.
- We're still unaware of dragging a file unto a directory in the ftp view. 
Files are always uploaded to the current dir.  I've heard murmurs that the ftp
view is to be rewritten, so the drop location detection can be done after that.

- Unknown if this works with a multiple-file drop ...

Obviously much work to be done, but here's barebones for ya :)

Comment 62

[Bradley Baetz (:bbaetz)]

I'm not going to be able to play with this until the weekend, but you can get
the current location by getting the URL attribute form teh dropped on node. Your
curent patch will accept ftp:// files as well as directories. See the drag code
in there for how that works.

This probably isn't the correct place for it, too - it should go into directory.js

Comment 63

[Doug Turner (:dougt)]

Attachment: better patch with uses the progress dialog

You will need my patch in bug
http://bugzilla.mozilla.org/show_bug.cgi?id=128147

Comment 64

[Doug Turner (:dougt)]

The last patch has dialog problems.  but it is a step in the right direction. :-)

Comment 65

[Kathleen :Brade]

Comment on attachment 71763 [details] [diff] [review]
better patch with uses the progress dialog

* contentAreaDD.js has strings which should be localizable:
+	     if (! confirm("Upload " + url + " to " + base + " ?")) return;

* uploadURI seems to have some tabs or different spacing?

* do we need an error dialog if nothing happens in uploadURI (in or after
catch)?

remove the dumps and r=brade  :-)

Comment 66

[Doug Turner (:dougt)]

this patch isn't really ready to go in.  see the depend bug.  plus the dialog is
almost functionless right now.  this is still a work in progress.  Bill has alot
of work to do to make the progress dialog work correctly for both uploading and
downloading.

Comment 67

[Mike Shaver (:shaver emeritus)]

Comment on attachment 71763 [details] [diff] [review]
better patch with uses the progress dialog

brade's comment said "r=brade if...", but adding localization requires the
patch to be reviewed again, in my book.  (Don't r= if there are questions
outstanding the answer to which will affect whether the patch is acceptable or
not, please!)  Marking needs-work, to amplify dougt's assessment (with which I
heartily agree).

Comment 68

[rubydoo123]

removing myself from the cc list

Comment 69

[Mike Shaver (:shaver emeritus)]

This bug will be much happier in the hands of dougt, who will actually love it.

Comment 70

[Doug Turner (:dougt)]

sure, hand it to me two weeks before the deadline! :-)

Comment 71

[lohphat]

Why is the status still "New" vs. "Assigned"?  It's only 2+ years old.

Comment 72

[WD]

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

Comment 73

[Flo Ledermann]

it's crazy and sad that this is now the _only_ remaining reason i have to fire
up ie from time to time... any current status on this?

Comment 74

[Doug Turner (:dougt)]

It is quite sad indeed.  The backend code is complete.  We just need a download
progress dialog.  Any XUL hackers?

Comment 75

[griffon4]

Adding myself to the CC:.  I'm behind a socks proxy firewall, so I would LOVE to
be able to FTP upload files; assuming it will use Moz's proxy logic to do the
lifting . . . . Just wanted to chime in with why a body might need this feature.

Comment 76

[Peter Trudelle]

nominating for Buffy.  Doug, would you like someone on Navigator to take this?

Comment 77

[Doug Turner (:dougt)]

To peter for considered for the next release.  

Comment 78

[Peter Trudelle]

->varga

Comment 79

[Samir Gehani]

Nav triage team: nsbeta1+/adt2

Comment 80

[Jan Varga [:janv]]

I talked to pinkerton, he says it should put into nsContentAreadDargDrop.cpp
instead of contentAreaDD.js
I'll try to port it

Comment 81

[Jan Varga [:janv]]

Attachment: patch v0.1

I have problems with uploading. File is created on FTP server, but it has zero
size.
dougt, could you take a look ?

Comment 82

[Jan Varga [:janv]]

I got this assertion in debug build:
###!!! ASSERTION: ReadSegments: 'Not Reached', file
../../../../mozilla/netwerk/base/src/nsFileStreams.cpp, line 664
Break: at file ../../../../mozilla/netwerk/base/src/nsFileStreams.cpp, line 664

Comment 83

[Jan Varga [:janv]]

ok, someone wrote a nice comment in that method, so I did what he/she suggested
I wrapped input stream with buffered input stream and now it works!

Comment 84

[R.K.Aa.]

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

Comment 85

[Julien Pierre]

So how exactly is one supposed to upload files using mozilla ? Is this not
planned ? Is any other type of upload supported, eg. HTTP upload ?

Comment 86

[Kathleen :Brade]

Julien Pierre (comment 85)--as the summary of this bug suggests, the UI to
permit users to upload in the browser is not done.  I am unaware of any specs on
how this might work.  The 'backend' support is all there; Composer's upload UI
supports ftp, http, https, and local files.

Jan Varga--how goes your patch?  Is it ready for reviews?

Comment 87

[Jan Varga [:janv]]

It basically works, but progress dialog needs some fixes, see bug 128147 and bug
128119

Comment 88

[Sam Katz]

This should at least be in the nightly build. We call it nightly because some
things (read: experimental features) are in them which may not work correctly.

Then maybe some skinner who is disgusted can write an XUL skin on the progress
dialog.. or I can open a seperate bug called "FTP progress dialog needs
skinnning support". skinners will love that one.

But when it has to do with form over function.. I think we should do function
first. Netscape doesn't have to pick it up..but Mozilla's constantly in
development.. and we need full FTP support in the GUI. (heck I would like the
browser to popup a *username* and *password* dialog *before* connecting to an
FTP site.) That would be better support than NS4 had. Of course, this would be
only from the address bar. Links (for downloading) would be assumed to be anon
ftp. But at least have basic functionality in there for now. People should be
able to assume that if browser-->then FTP. (unfortunately, IE is part of the
problem. They make FTP optional.

Comment 89

[Sam Katz]

I forgot to add the reason for my suggestions:

there is an improved patch.

Somebody should review it.. by testing it. It should be tested on a wide basis.

Also: historical question-did NS4 have a progress dialog?

Comment 90

[Doug Turner (:dougt)]

jan, can you write up exactly what it would take to fix the depend bugs?  Maybe
someone can donate their time?

Comment 91

[Charlie (web developer; new to bugzilla)]

No NS4 did not show upload progress afaik.
IE5 did, in the status window.
Can I offer any help testing / light coding here?

Comment 92

[Jan Varga [:janv]]

We need a help with fixing bug 128119 and 128147.
The rest is easy.

Comment 93

[Steve Dagley]

To correct Charlie, NS4 did have upload progress UI

Comment 94

[Jan Varga [:janv]]

note for shuehan:
The input stream must be wrapped by an nsIBufferedInputStream to get it work.

Comment 95

[Jan Varga [:janv]]

Attachment: more recent patch

Comment 96

[Jan Varga [:janv]]

reassigning

Comment 97

[(not reading, please use [e-mail] instead)]

speaking as a driver, I don't think we want this for 1.4 beta or final.

1)  we won't have progress without #128119

2)  without progress (or even with it), are we just open up a can of worms here?
 like, what if I quit while uploading, or close the window, etc.

3)  how common is upload?  for those who really need it, there are *plenty* of
other ftp "upload" clients.  (we're already a download client).

to me, this seems like something perfect for 1.5 / firebird, as an optional add on.

comments?

Comment 98

[Jamie Zawinski]

"There are plenty of other upload clients?"  Well hell, there are plenty of
other download clients too -- why not remove support for FTP entirely?

MacOS X doesn't come with any convenient GUI way to upload files via FTP.  You
either have to use the shell, or you have to discover, download, and
periodically re-install nagware like Fetch.  The last time my girlfriend was
doing the "uninstall and reinstall Fetch so that it will work again" dance, I
said "just use Mozilla to do it".  Oops, this feature that Netscape had since
1.0 has been removed.  Then it turned out that she didn't have the "BSD
Subsystem" installed, so no command-line FTP either.

This is a common situation.  Most people publish to their web sites via FTP. 
Mozilla should make this easy, like it used to in the past.

Comment 99

[(not reading, please use [e-mail] instead)]

I'm not saying don't add the feature, I'm saying add it once it works.

if there is no progress, how will I know when the upload is done?

also, is it hooked up properly to stop?

Comment 100

[Doug Turner (:dougt)]

I thought when i handed it over to peter this feature was going go be worked on. :-/

i think the more important point is that no one is working on this right now. 
If someone had a patch ready to go, of course we would try to get it in for 1.4.  

we totally dropped the ball on the uploading functionality in mozilla.  

shliang, if you are not planning to fix this soon, please reassign it to me.   

Comment 101

[Julien Pierre]

I was in the middle of adding my comment but Jamie was quicker.

1) If 128119 can't be resolved on time for 1.4 , could a simpler progress dialog
be implemented, ie. one which just goes from 0 to 100 when completed, and allows
stopping ?

2) - if you quit while uploading, the behavior should be similar to quitting
when downloading
- if the user closes the window, mozilla should probably prompt about whether
the user intends to stop the upload

3) Yes, there are plenty of other upload clients, however Mozilla integrates
Composer and it is extremely inconvenient to have to invoke another client to
upload the file. Even if there is no progress status and there are some quirks,
I still think the feature should be implemented. I don't know what the timeframe
is for 1.5, but maybe 1.4 could implement it to the point of being barely
usable, and 1.5 can fix the quirks.

Comment 102

[Christopher Blizzard (:blizzard)]

I don't want to see this go in until the UI is done.  It just seems busted
without any kind of feedback, and I'm sure that we would get a lot of bogus bug
reports as a result.  If someone finishes the progress bits, I would take this
in 1.4.  It's important functionality.

Comment 103

[Mike Shaver (:shaver emeritus)]

> 3) Yes, there are plenty of other upload clients, however Mozilla integrates
> Composer and it is extremely inconvenient to have to invoke another client to
> upload the file

Doesn't composer already do FTP-based publish?

(ashamed to show my face in this bug again)

Comment 104

[Mike Shaver (:shaver emeritus)]

bliz: what if we get it without the UI, and disable it with a hidden pref, so
that people can still bail themselves out with about:config -- and give us
feedback/bug reports, once they've leapt that barrier to entry -- but my mom
doesn't stumble into it?

Comment 105

[Akkana Peck]

> Doesn't composer already do FTP-based publish?

Yes.

In fact, if some budding XUL hacker wanted to take on the UI part of this
feature, they might find some useful code to copy in editor/ui/dialogs/.

> what if we get it without the UI, and disable it with a hidden pref

That would be awfully useful.  It would also make it easier to land the feature,
since the backend and the UI will probably need review by different sets of
people and will have different issues.

Comment 106

[Paul Wyskoczka]

ADT: Nominating topembed

Comment 107

[(not reading, please use [e-mail] instead)]

just to clarify, shuehan showed me what the current patch does:

1)  upload, and in the status text we get:  "uploading xxx..." (it doesn't clear
when done)
2)  the stop button isn't enabled, so clicking on it wouldn't stop
3)  the page doesn't refresh when the upload finishes (so you won't know it is done)
4)  the throbber doesn't throb.
5)  no progress updates to the progress meter / status text.

Comment 108

[(not reading, please use [e-mail] instead)]

note to who ever works on this.

what happens happen if you try upload a big file, and then closed the window
(since you didn't know it was done), or hit refresh or browsed somewhere else
before it was finished.

would we cancel the url (or load group)?

Comment 109

[Mike Shaver (:shaver emeritus)]

Can we just stick the upload-progress UI in the download manager?  I won't shed
a tear about the mildly-bogus naming, and it would make a lot of these issues
just go away.

Comment 110

[Samir Gehani]

adt: nsbeta1-

Comment 111

[chris hofmann]

topembed-

Comment 112

[Matthias Versen [:Matti]]

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

Comment 113

[Carsten Emde]

Just got an email from someone who recently switched from 4.x to Mozilla 1.4 (as
I recommended) and who was complaining that the File Upload menu entry is
missing.  I couldn't believe it, but yes, he was right! Is someone working on
it? Could someone let me know what the current status is? Thanks.

Comment 114

[Matthias Versen [:Matti]]

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

Comment 115

[beanladen]

We just moved the whole institute from NS4.8 to Mozilla, and a couple of people
started to complain about the missing ftp upload menu entry (for general files,
not the composer functionality). I saw that upload is ready in the networking
code, but the UI is still not ready after more than 3 and a half year...
Is there anything hidden which can be activated by a preference ? Any patch
or optional package to try out ? This bug should be raised in priority, since
when we loose composer we also loose the limited functionality for uploading
http files. Since it seems that everything is almost there, it couldn't be that
hard to upload the patches ? Is there a major obstacle why this bug can't be fixed
now ?

Comment 116

[Doug Turner (:dougt)]

the biggest hurdle is time.  :-)

the last patch seams to work for mozilla.  we are just missing a progress
dialog. However, for firebird client, it will not work - my front end changes
were in the mozilla specific xul code.  I think we need to move this kind of
functinality into the drag/drop code in gecko so that all clients can use it.  

Comment 117

[beanladen]

Since there are some people who just can't wait, I decided to write my own
rudimentary piece of code to have a simple workaround until this is bug is fixed.
The upload code is already running (it currently is a html/javascript page that
calls the respective xpfe functions), I only need to put an 'Upload File' entry 
in the menu that directs the current page's URL as an argument to a js script. Can
somebody show me how to do this (a simple bookmark entry that takes the current
page as an argument will also do) ? If it works, I will publish that small tool
for the unpatient.

Comment 118

[beanladen]

Attachment: Workaround to get Upload functionality

Here is a simple workaround without patching anything: Place the attached HTML
somewhere, bookmark it. Whenever you want to upload, login to the upload 
directory, then load this file via the bookmark. Please post corrections and
change what you like. I'm not a javascript or xpfe programmer, in fact, the
code
was created by copying a couple of lines from different broken scripts seen in
the newsgroups, just leaving out the broken parts...

Comment 119

[Jason Barnabe (np)]

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

Comment 120

[Valerio Messina]

It would be very wonderfull if after a ftp logon, we can see remote disk like a
local disk.
So other than drag&drop, I like using copy&paste, rename file/dirs, and upload
file and complete dirs tree.
Today is very common find a work PC (Win32 o Linux) or Sun workstation Solaris,
with Moz installed ufficially from administrator. So to have ftp complete client
integrated in Moz, maybe the only chanche to pass the company firewall and logon
on remote NON anonimous Ftp server to update Web site.
I propose this feature become blocking for 1.6
thanks, Valerio

I like to have listing of remote file property right and owner too (drwxrwxrwx)
RFE new bug?

Comment 121

[Pete Zha]

I have some idea to share for this feature.
I think for xpfe and firebird, upload when drag and drop is enough since most
end users only use browser to browse web page and download. We don't have to do
much for everyone. For those users who want to use full feature ftp in browser,
I think comment 120 is good. We can make it as an extension for both xpfe and
firebird.
I'm working on making the drag and drop ftp upload for Mozilla. I'm going to do
it based on existing patch and enhance it. I'm not sure a process dialog is
suitable. I think the status on status could be better. Just like when loading
page, we display the process on status bar, I think it's enough for end user to
know the progress. And the stop button should work when uploading. Means when
user click "Stop", the transfer should stop.

Comment 122

[Fernando Cassia]

I strongly disagree with comments #120 and #121. Why not keep things S-I-M-P-L-E
and offer the same functionality that was present in the old Netscape
Communicator 4.8 and previous versions?

That is, ONLY WHEN THE USER is already logged into a ftp:// url, display under
File->Upload File.

Once the user selects a file from the local disk the upload should begin but the
transfer status should be displayed on a detached PROGRESS DIALOG, just like
when the user downloads a file (and the Download Manager is not enabled).

Until the ftp transfer is done, the user shouldn't be able to cotinue moving
thru the ftp tree. Why? Because some FTP sites limit the number of simultaneous
connections from a single user/password or IP address. Letting the user continue
moving thru the ftp site while a ftp upload is in progress would make the
browser attempt to establish a secondary (or third, or fourth, depending on the
number of simultaneous uploads already in place) connection to the ftp site,
which more often than not will result in "you're already logged in" error
messages from the ftp server.

Of course this behaviour could be tweaked with a "max. number of simultaneous
connections for ftp uploads" preference.

Just my $0.02. I'd like to hear comments on these suggestions (just bring back
the old working Communicator 4.8 approach!).

Comment 123

[Fernando Cassia]

I strongly disagree with comments #120 and #121. Why not keep things S-I-M-P-L-E
and offer the same functionality that was present in the old Netscape
Communicator 4.8 and previous versions?

That is, ONLY WHEN THE USER is already logged into a ftp:// url, display under
File->Upload File.

Once the user selects a file from the local disk the upload should begin but the
transfer status should be displayed on a detached PROGRESS DIALOG, just like
when the user downloads a file (and the Download Manager is not enabled).

Until the ftp transfer is done, the user shouldn't be able to cotinue moving
thru the ftp tree. Why? Because some FTP sites limit the number of simultaneous
connections from a single user/password or IP address. Letting the user continue
moving thru the ftp site while a ftp upload is in progress would make the
browser attempt to establish a secondary (or third, or fourth, depending on the
number of simultaneous uploads already in place) connection to the ftp site,
which more often than not will result in "you're already logged in" error
messages from the ftp server.

Of course this behaviour could be tweaked with a "max. number of simultaneous
connections for ftp uploads" preference.

Just my $0.02. I'd like to hear comments on these suggestions (just bring back
the old working Communicator 4.8 approach!).

Comment 124

[dantifer]

I think comments #123 is reasonable. Before all browser is a browser. if user 
wants more complicated ftp functions, more powerful ftp clients are suggested.
So I think SIMPLE function is enough.
is uploading a directory necessary?

Comment 125

[Darin Fisher]

-> me

Comment 126

[chris hofmann]

nominating for 1.7b, but we would need to move quickly if it is going to make
the 1.7b string change and final deadline.. 

Comment 127

[Pete Zha]

darin,
Are you working on this? Actually, dantifer is also working on this and we will
get a patch very soon. Including a progress doalog(reuse the nsIProgressDialog
to show the progress and support multiple file selection.

Comment 128

[Mike Kaply [:mkaply]]

I'm going to put the File Upload UI into 1.7b so that we can get it for 1.7
final (need the DTD in for translation freeze)

Is there a strong opinion as to where on the Upload File menu in SeaMonkey it
should be?

I was thinking between edit page and page setup with separators around it.

Here's what 4.x looked like:

File

New
Open Page
----------
Save As
Save Frame As
----------
Send Page
Edit Page
Edit Frame
Upload File
Offline
----------
Page Setup
Print Preview
Print
----------
Close
Exit

Comment 129

[dantifer]

Now i am working on ftp UI. I have got some ideas. 

1. user can upload 
  (1). single file
  (2). multiple files. 
  directory is not allowed

2. user can use three methods to upload file(s).

  (1). Menu : File -> Upload Files ...
    ...
    ----------
    Edit Page
    ----------
    Upload Files ...
    ----------
    ...
    This menu entry is dynamically enabled or disabled.

  (2). Context menu: Upload Files ...

    Upload Files...
    ----------
    Back
    ...
    This menu entry is dynamically shown or hidden.

  (3). Drap & Drop
	While browsing ftp , multiple files can be droped into contentArea.


3. a progressdialog like dialog is shown while uploading.

  +-------------------------------------------------------------+
  |Uploading : [File name] ([Current number] of [Total number]) |
  +-------------------------------------------------------------+
  |                                                             |
  |   Upload From: [Local directory]/File name]                 |
  |          To  : [Network directory]/[File name]              |
  |        Status: [Uploaded size] of [Total size] (at [Speed]) |
  |     Time left: [hh:mm:ss]                                   |
  | Time Elsapsed: [hh:mm:ss]                                   |
  |      Progress: []]]]]]]]]]]]]]]]]]]]]]]]       ] [Percent]% |
  |                                                             |
  |   +--------+                                                |
  |   | Cancel |                                                |
  |   +--------+                                                |
  +-------------------------------------------------------------+

4. after uploading , refresh

Comment 130

[Ric Gates]

Would it be possible to add a DELE request to context menu as well?

Comment 131

[Darin Fisher]

The first version of this should be "as simple as possible."  I'm thinking "File
-> Upload File" a'la Nav4x, and that's it.

D&D could be nice, but I'm not sure I like the fact that it would change the
current D&D semantics.  I personally like being able to drag a file onto
Mozilla, and have it try to load the file into the browser window.  I'm not
saying that we shouldn't consider it, but for the first version of this patch I
think it should be excluded.

Comment 132

[Ric Gates]

(In reply to comment #131)
> D&D could be nice, but I'm not sure I like the fact that it would change the
> current D&D semantics.

... And end up with a confusing IE like UI where the behavior changes depending
on if you're logged in or not. But then it could be user pref.
Just having menu items for Upload, Download and Delete (if reasonably possible)
would provide basic file transfer functionality for not only web publishing, but
as an alternative to sending email attachments as well.

Comment 133

[dantifer]

> ... And end up with a confusing IE like UI where the behavior changes 
depending
> on if you're logged in or not. But then it could be user pref.

On the contrary, I think it is a step forward :)

Comment 134

[Ric Gates]

(In reply to comment #132)

> ... a confusing IE like UI where the behavior changes depending
> on if you're logged in or not.

should be
... a confusing IE like UI where its behavior changes depending on if IT THINKS
you're logged in or not.

If you think you are logged in and you're not, it's nice to have a clue.

Comment 135

[Pete Zha]

(In reply to comment #131)
>I'm not
> saying that we shouldn't consider it, but for the first version of this patch I
> think it should be excluded.
I agree, We can consider it later to add a user pref to change behavior for ftp
window.

Comment 136

[Damian Yerrick]

Why not add a box somewhere within the content area where the user can drag
a file to upload it?  Is that behavior good enough for waiting for this fix
and filing a new bug?

Comment 137

[Sam Katz]

I agree with plan A. On the contrary, with Set Access And Defaults charging us
with handling FTP, (even though we really shouldn't, and Firefox no longer
steals it), we should have basic upload functionality.  In terms of ease of use,
here are my personal priorities.
1. Emulate the IE behavior except do it better: when I type
ftp://ftp.someplace.com, ask for login information before contacting the server,
with a "I don't have one" button that logs people in anonymously.

IE will try connecting as anonymous automatically, which makes the server logs
deceptive and useless.

2. Implement wizards or "guides". see www.cnet.com's new video on the next
version of Internet Explorer. "guides" are security related information that
attaches to the navigation bar.. fully implementable in DOM. "Mozilla thinks you
need to login to this site." A guide could then be used to say "To send a file
to this server, Choose Upload File.. from the File menu."

3. Drag and Drop. I'll admit that every FTP client has it so it is weird that
Mozilla doesn't, and now that Netscape is not producing new versions, being 4xp
is irrelevant.

Once this feature is implemented, more user feedback can be gathered. But I
changed my mind--adding drag and drop first receives no objection. But from what
I understand, it isn't universally accepted by the community.

But I will point out, to a novice user, dragging and dropping to view while
browsing an ftp server, is very strange. A prompt certainly wouldn't be out of
the question.

Comment 138

[Sam Katz]

We missed the deadline for beta freeze. I don't think this is going in.

Comment 139

[chris hofmann]

let's shoot for 1.8a

Comment 140

[ZF.Dang]

Attachment: Implement FTP upload UI for mozilla

Comment 141

[ZF.Dang]

Comment on attachment 144098 [details] [diff] [review]
Implement FTP upload UI for mozilla

darin, please take a look at this patch. Thanks!

Comment 142

[ZF.Dang]

Comment on attachment 144098 [details] [diff] [review]
Implement FTP upload UI for mozilla

>     if (storStr.First() == '/')
>     {
>         // kill the first slash since we want to be relative to CWD.
>-        storStr.Cut(0,1);
>+//        storStr.Cut(0,1);
>     }
I made this modification for it causes problems while uploading.

we can use ftpChannel or nsIWebBrowserPersist to upload files.
the destination file is represented by a URI .
But after you login into the ftp server and you navigate to a certain 
directory, you still use the same URI will cause error.

the reason is ftpChannel or nsIWebBrowserPersist use browser connection cached,

for example:
destination file's URI is "ftp://ftp.mycom.com/incoming/test.txt"
you browser current location is "ftp://ftp.mycom.com/another/"
then you use that URI to upload file, ftpConnectionThread misunderstanding
the destination file as "ftp://ftp.mycom.com/another/incoming/test.txt"

I made this modification just for FTP uploading. But I afraid simply remove
a line will cause other problems.

Comment 143

[timeless]

i think you're supposed to use:
"ftp://ftp.mycom.com//incoming/test.txt"

Comment 144

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 144098 [details] [diff] [review]
Implement FTP upload UI for mozilla

hmm, I don't really like your nsIProgressDialog changes... Why are you
implementing nsIAuthPrompt etc on it? Why don't you set something else as the
notificationCallbacks?

bah, I had written a paragraph here how I'd like nsIProgressDialog to not care
whether its up- or downloading. then I realized it inherits from nsIDownload.

But can you rename uploadFlag to uploading?

+	     this.mUploadOperation == null;
this is a no-op. is it needed?

+	 if( this.mUploadFlag ){
+	     if( this.percent == 100 )
eww, this is such a hack. why is it needed, doesn't nsIWebProgressListener's
API offer a better way? onStateChange with STATE_STOP or something?

+    // ---------- nsIFTPEventSink methods ----------
again, why does this have to be implemented on this object?

+	 if( this.mUploadFlag )
+	     this.parent.DelayedBrowserReload(1000);
can you explain? This is in oncancel, right?

			 if ( this.target ) {
-			     this.enable( "launch" );
+			    if( ! this.mUploadFlag)

please use a single if here

+		 if ( this.mUploadFlag )
+		    this.parent.DelayedBrowserReload(1000);
why delayed? and why only if the dialog is set to auto-close?

-<!ENTITY defaultTitle	   "Saving">
+<!ENTITY defaultTitle	   "Transfer">
+<!ENTITY defaultTitleUpload	 "Uploading">

Can you use a single entity "Transferring"?

+<!ENTITY uploadingTitle      "#2&#037; of #1 Uploaded ( #3 of #4 )"> 

please remove the spaces before #3 and after #4

+<!ENTITY uploadingSource     "Upload To:"> 

"Uploading To"

+<!ENTITY uploadingTarget     "From:"> 

a _target_ has a label of "From"?

 <!ENTITY pausedMsg "Download Paused">
+<!ENTITY pausedUploadMsg "Upload Paused">

is either of these entities used?

 <!ENTITY completeMsg "Finished, #2 KB downloaded (elapsed time was #1)">
+<!ENTITY completeUploadMsg "Finished, #2 KB uploaded (elapsed time was #1)">
can you use a single entity here and use "transferred"?




(only looked at embedding/components/ui/progressDlg)

Comment 145

[ZF.Dang]

(In reply to comment #144)
> (From update of attachment 144098 [details] [diff] [review])
> hmm, I don't really like your nsIProgressDialog changes... Why are you
In fact, I agree with you. now nsIProgressDialog is really bad. I think 
it is better to create a new dialog. But what should we call the new dialog?
uploadProgressdialog? nsIProgressDialog can just be used for downloading ,he
called himself progressdialog...

> implementing nsIAuthPrompt etc on it? Why don't you set something else as the
> notificationCallbacks?
> 
> bah, I had written a paragraph here how I'd like nsIProgressDialog to not care
> whether its up- or downloading. then I realized it inherits from nsIDownload.
> 
> But can you rename uploadFlag to uploading?
> 
> +	     this.mUploadOperation == null;
> this is a no-op. is it needed?
yes, it is necessary to cancel upload.

> 
> +	 if( this.mUploadFlag ){
> +	     if( this.percent == 100 )
> eww, this is such a hack. why is it needed, doesn't nsIWebProgressListener's
> API offer a better way? onStateChange with STATE_STOP or something?
onStateChange is never used for uploading. but onStatus is used.
what make me to do such a stuid modification is: sometime, onStatus is not
alwayls called after transer is completed.(?)

> 
> +    // ---------- nsIFTPEventSink methods ----------
> again, why does this have to be implemented on this object?
> 
> +	 if( this.mUploadFlag )
> +	     this.parent.DelayedBrowserReload(1000);
> can you explain? This is in oncancel, right?
yes, this is in oncancel. you click cancel to cancel the rest uploading
jobs. but perhaps you have already uploaded some files....

> 
> 			 if ( this.target ) {
> -			     this.enable( "launch" );
> +			    if( ! this.mUploadFlag)
> 
> please use a single if here
ok, good suggest.

> 
> +		 if ( this.mUploadFlag )
> +		    this.parent.DelayedBrowserReload(1000);
> why delayed? and why only if the dialog is set to auto-close?
you can not understand this only if you tested the code yourself.
ftp connection needs time to clean up. if the ftp server limits its
connection number from one ip, then you got the error information:
"you can not login more than one connection." that's the reason.

> 
> -<!ENTITY defaultTitle	   "Saving">
> +<!ENTITY defaultTitle	   "Transfer">
> +<!ENTITY defaultTitleUpload	 "Uploading">
> 
> Can you use a single entity "Transferring"?
> 
when we don't know the exact direction , to use transfering is the only
choice. but after you know the direction, why not use 'saving'/'uploading' ?


> +<!ENTITY uploadingTitle      "#2&#037; of #1 Uploaded ( #3 of #4 )"> 
> 
> please remove the spaces before #3 and after #4
> 
ok.

> +<!ENTITY uploadingSource     "Upload To:"> 
> 
> "Uploading To"
ok.

> 
> +<!ENTITY uploadingTarget     "From:"> 
> 
> a _target_ has a label of "From"?
ok.

> 
>  <!ENTITY pausedMsg "Download Paused">
> +<!ENTITY pausedUploadMsg "Upload Paused">
> 
> is either of these entities used?
> 
i admit it is my mistake. i should remove pauseUploadMSg for it is never used.

>  <!ENTITY completeMsg "Finished, #2 KB downloaded (elapsed time was #1)">
> +<!ENTITY completeUploadMsg "Finished, #2 KB uploaded (elapsed time was #1)">
> can you use a single entity here and use "transferred"?
> 
same as "transfering"

> 
> 
> 
> (only looked at embedding/components/ui/progressDlg)
> 

now I feel it is really bad to try to reuse nsIProgressDialog. the same feeling
with you :)

Comment 146

[ZF.Dang]

> 
> +<!ENTITY uploadingTarget     "From:"> 
> 
> a _target_ has a label of "From"?

I have not explained this clearly.

target for uploading should be the URI, source for uploading should be URI or 
nsIlocalFile.

but nsIProgressDialog is quite amazing. the target is nsILocalFile, if you use
URI instead, crash ...

So I exchange them. I pass source(nsIlocalFIle) as target(pass) and target(URI)
as source(pass), labels are also changed.

aha, what a funny job.

Comment 147

[ZF.Dang]

darin, have you seen the patch? should I use a new progressdialog instead of
reusing the old one? 
waitting to get your suggestion ...

Comment 148

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #145)
> > +	     this.mUploadOperation == null;
> > this is a no-op. is it needed?
> yes, it is necessary to cancel upload.

I find that hard to believe. As written, that line does _nothing_ _at_ _all_.
(I think you want a single equals sign)

> > why delayed? and why only if the dialog is set to auto-close?
> you can not understand this only if you tested the code yourself.
> ftp connection needs time to clean up. if the ftp server limits its
> connection number from one ip, then you got the error information:
> "you can not login more than one connection." that's the reason.

That rather seems like a bug in the FTP implementation of mozilla, it would be
good to report that bug.

> > -<!ENTITY defaultTitle	   "Saving">
> > +<!ENTITY defaultTitle	   "Transfer">
> > +<!ENTITY defaultTitleUpload	 "Uploading">
> > 
> > Can you use a single entity "Transferring"?
> > 
> when we don't know the exact direction , to use transfering is the only
> choice. but after you know the direction, why not use 'saving'/'uploading' ?

I don't understand. Why are you changing "Saving" to "Transfer", then?

> now I feel it is really bad to try to reuse nsIProgressDialog. the same feeling
> with you :)

well, _my_ feeling is really that the nsIProgressDialog API should be more
generic, and the impl should suck less. ah well :)

I wish you had commented on why nsIAuthPrompt needs to implemented by the
progressdialog.

Comment 149

[ZF.Dang]

> I find that hard to believe. As written, that line does _nothing_ _at_ _all_.
> (I think you want a single equals sign)
:) I have to admit that I was wrong here.
> 
> > > why delayed? and why only if the dialog is set to auto-close?
> > you can not understand this only if you tested the code yourself.
> > ftp connection needs time to clean up. if the ftp server limits its
> > connection number from one ip, then you got the error information:
> > "you can not login more than one connection." that's the reason.
> 
> That rather seems like a bug in the FTP implementation of mozilla, it would be
> good to report that bug.
OK, I will do it right now.
> 
> > > -<!ENTITY defaultTitle	   "Saving">
> > > +<!ENTITY defaultTitle	   "Transfer">
> > > +<!ENTITY defaultTitleUpload	 "Uploading">
> > > 
> > > Can you use a single entity "Transferring"?
> > > 
> > when we don't know the exact direction , to use transfering is the only
> > choice. but after you know the direction, why not use 'saving'/'uploading' ?
> 
> I don't understand. Why are you changing "Saving" to "Transfer", then?
> 
> > now I feel it is really bad to try to reuse nsIProgressDialog. the same 
feeling
> > with you :)
> 
> well, _my_ feeling is really that the nsIProgressDialog API should be more
> generic, and the impl should suck less. ah well :)
OK, we have wasted too much time here. Now I am waiting for comments from
darin, and preparing to implement a new progressdialog.

> 
> I wish you had commented on why nsIAuthPrompt needs to implemented by the
> progressdialog.
> 
object for that callback requires this interface.
is there anyway to avoid this? I am a newcomer here, just want to do someting 
for mozilla, I don't know much...

Comment 150

[Christian :Biesinger (don't email me, ping me on IRC)]

> object for that callback requires this interface.

the object that you set as the notificationCallbacks on the channel needs to
implement this. correct. However, I'd rather a differnet object be set as that.

See, the main reason why I dislike the progressdialog implementing this is that
for downloads, it is not implemented here either. I'm not sure if anything
implementes it for the "save link target as" case, but it wouldn't be on the
progressdialog anyway because if dl manager is used, the progress dialog is not
involved at all.

Comment 151

[ZF.Dang]

> See, the main reason why I dislike the progressdialog implementing this is 
that
> for downloads, it is not implemented here either. I'm not sure if anything
> implementes it for the "save link target as" case, but it wouldn't be on the
> progressdialog anyway because if dl manager is used, the progress dialog is 
not
> involved at all.
OK, perhaps we should use a new progressdialog.

Comment 152

[ZF.Dang]

Attachment: this patch is almost the same with the above one except it adopts a new dialog instead of nsProgressDialog

Comment 153

[ZF.Dang]

Attachment: modify the uploading method and dialog. now it can upload files both to ftp server and webdav server.

according to Darin's response, I modify the above patch. This uploading part
can be used to upload files to webdav server, too.

Comment 154

[ZF.Dang]

Comment on attachment 145537 [details] [diff] [review]
modify the uploading method and dialog. now it can upload files both to ftp server and webdav server.

Darin, please take time to have a look.

Comment 155

[ZF.Dang]

Comment on attachment 145537 [details] [diff] [review]
modify the uploading method and dialog. now it can upload files both to ftp server and webdav server.

Darin, please take time to have a look.

Comment 156

[Doug Turner (:dougt)]

your change to netwerk/protocol/ftp/src/nsFtpConnectionThread.cpp will break
some servers.

Comment 157

[Darin Fisher]

ZF.Dang:  FYI, I am currently working on a revised patch for this bug.  I will
be posting something here shortly.

Comment 158

[Darin Fisher]

Attachment: v2 patch

This patch moves most of the methods from nsIDownload onto a new interface
called nsITransfer.  nsIDownload now inherits from nsITransfer instead, and
this allows the nsIProgressDialog to inherit from nsITransfer instead of
nsIDownload.  As a result the progress dialog code can be reused with only a
few modifications.

Unlike the current nsIDownload, nsITransfer uses nsIURIs for both the source
and target of the transfer.  In the download case the transfer is to a file URI
(or some other URI scheme that maps to nsIFile).  nsWebBrowserPersist is
already able to accept a nsIURI for the target, so there is not much of a
paradigm change here.

Much of the changes to this patch result from the nsIDownload interface change.


The result of this patch is a new file menu, labeled "Upload File", that is
enabled whenever a FTP directory listing is selected into the browser frame.

It would be good to add drag-n-drop and/or a context menu in the future, but
this is at least a start.

Comment 159

[ZF.Dang]

I have tested this patch, and found two problems.

1.the first is uploading path. And that is the reason why I modify 
nsFtpConnectionThread.cpp file.
in my test,I open ftp://xxx/incoming/, refresh it, then upload yuquan03.mp3.
it failed to upload. log from ftp server is:
--refresh
[2] Thu 08Apr04 16:34:53 - (000052) CWD /incoming/
[6] Thu 08Apr04 16:34:53 - (000052) 250 Directory changed to /incoming
[2] Thu 08Apr04 16:34:53 - (000052) LIST
[6] Thu 08Apr04 16:34:53 - (000052) 150 Opening ASCII mode data connection 
for /bin/ls.
[6] Thu 08Apr04 16:34:53 - (000052) 226 Transfer complete.
--connection remains.
--uploading
[2] Thu 08Apr04 16:35:04 - (000052) PASV
[6] Thu 08Apr04 16:35:04 - (000052) 227 Entering Passive Mode 
(129,158,217,140,42,211)
[2] Thu 08Apr04 16:35:04 - (000052) STOR incoming/yuquan02.mp3
[6] Thu 08Apr04 16:35:04 - (000052) 550 yuquan02.mp3: Cannot create file.
--closing connection
[5] Thu 08Apr04 16:35:05 - (000052) Closing connection for user MOZILLA 
(00:01:01 connected)

this patch try to upload files to ftp://xxx/incoming/incoming/yuquan02.mp3
But after this failure, I upload the file again, it succeeded. For after the 
above upload operation, ftp connection is closed;mozilla create a new thread 
for uploading.

2. second. nsWebbrowserPersist does not pass correct uploading progress to 
progress dialog. actually, progress dialog shows two progress, the first is 
very fast, so I think perhaps it indicates speed of reading disk file. If you 
upload big files, you can see this problem.

I have files these two bugs before: bugid are 238847 & 238848. you can get 
detailed information there.

and what is more, we can delay other more convenient ways, such as DD && 
context menu. But I think the limitation to upload only one file is hard to 
accept. who will use this crippled uploading function? 

Comment 160

[Zook Valem]

Should we updated the this bug by adding this bug depend on 238847 & 238848 at
the top as stated from the recent comments?

Comment 161

[Darin Fisher]

ZF.Dang:

thank you for pointing out those problems.  i can reproduce the URL generation
problem on my end.  as for the progress dialog problem, i think i have a
solution that will work for that.  when the progress dialog believes it is
uploading, it can simply ignore status/progress messages related to anything
other than the target URI.  see my comments in the other bugs.  marking those as
blocking this bug.

Comment 162

[Darin Fisher]

> and what is more, we can delay other more convenient ways, such as DD && 
> context menu. But I think the limitation to upload only one file is hard to 
> accept. who will use this crippled uploading function? 

I see this patch as only the first step in providing uploading capabilities in
the browser.  (I personally don't believe that this will be any kind of killer
feature for the product.  I don't think many users need to upload anything, and
those that do probably have other tools they like to use.)

In my mind, this patch is about improving the Mozilla platform as much as it is
about providing Netscape 4x parity.  This patch will help us if we ever want to
use GNOME 2.6's new file selection dialog, which enables the user to select
gnome-vfs locations (smb://, sftp://, etc.) as the destination to save a file
to.  So, uploading is a natural piece of that puzzle :-)

Comment 163

[ZF.Dang]

(In reply to comment #161)
> thank you for pointing out those problems.  i can reproduce the URL generation
> problem on my end.  as for the progress dialog problem, i think i have a
> solution that will work for that.  when the progress dialog believes it is
> uploading, it can simply ignore status/progress messages related to anything
> other than the target URI.  see my comments in the other bugs.  marking those as
> blocking this bug.
At first, I use nsFtpChannel to upload files. But I find it can not work with
the current progress dialog. Then I try to use nsWebbrowserPersist. But I can
not solve the problem of wrong progress. Finallly, I pick up nsFtpChannel again
and modify progress dialog a lot. That is the reason.

yeah, D&D, context menu, multiple files are up to you.

BTW:Now my patch can show correct progress, context menu,multiple files
selecting, D&D (I remove it before posting), URI (this is obviously not enough,
for I simply remove a line in nsFtpConnectionThread.cpp)

Comment 164

[Darin Fisher]

Attachment: v2.1 patch

This patch addresses the progress problem by making nsWebBrowserPersist only
report upload progress if the target file is non-local.

Comment 165

[Darin Fisher]

Comment on attachment 145940 [details] [diff] [review]
v2.1 patch

notes on this patch:

- nsITransfer introduced as a superclass of nsIDownload.  nsIProgressDialog now
inherits from nsITransfer instead of nsIDownload.  this allows
nsIProgressDialog to be used more easily to represent upload progress.

- "target" property in nsIDownload::init (now nsITarget::init) is now a nsIURI
instead of a nsIFile.

- nsIDownload provides a nsILocalFile "targetFile" readonly attribute, so that
the local file can be accessed easily when the transfer is really a download.

- my plan is to break nsITransfer out into its own IDL file.  i will probably
ask Leaf to copy nsIDownload.idl,v to nsITransfer.idl,v so i can preserve CVS
history for the bulk of the interface.

- nsWebBrowserPersist needed to be tweaked to properly report either download
or upload progress but not both mixed together.

- most of the nsDownloadManager changes are straightforward to deal with
nsIDownload interface change.

- navigator changes exist to hook up UI.

- filepicker bug is fixed so that the initial filterIndex can be remembered in
a pref and set before showing the filepicker.  fixed this in both xpfe and
toolkit

- nsFTPChannel was not reporting total progress properly.  it was always using
mContentLength, but that value is normally not set to anything other than -1. 
better to use aProgressMax which is determined by the code in
nsFtpConnectionThread.cpp

- i went to great lengths in navigator.js to get the nsIURI for the currently
focused content frame, so that that directory would be the target of the
upload.  actually, the frame could be any FTP file, and the directory of that
FTP file would be used as the target directory of the upload.

Comment 166

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 145940 [details] [diff] [review]
v2.1 patch

+  var targetURI = IOS.newURI(fileURL.file.leafName, null, targetBaseURI);

you don't need a charset here? it seems you can use
contentFrame.document.characterSet...

+  // display name property is unused by the progress dialog implementation,
and
+  // it's not obvious what we should pass for that argument

hm, I would just pass the leafName of the file I guess

why not pass Date.now()*1000 as start time? (*1000 because it is msec while
PRTime is usec...)

+function BrowserUploadFile()

hm, this function looks almost identical to BrowserOpenFileWindow... it would
be nice if they could share the code... maybe a function that shows a
filepicker, taking a filter index pref argument, return a uri?


you are using spaces in the if ( foo ) statements in nsProgressDialog.js
inconsistently...

+			 if ( this.targetFile ) {

above, you used != null

 nsFTPChannel::OnProgress(nsIRequest *request, nsISupports* aContext,
			  PRUint32 aProgress, PRUint32 aProgressMax)

-				   aProgress, (PRUint32) mContentLength);
+				   aProgress, (PRUint32) aProgressMax);

it seems the cast is no longer necessary

nsIDownload.idl
+     * @param aTarget The target (nsIURI) of the transfer.

"The target URI of the transfer", maybe?

xpfe/browser/resources/content/nsBrowserStatusHandler.js
+	   // Enable/disable menu entries for file upload...
+	   if (channel.URI.schemeIs("ftp"))
+	     this.canUploadFile.removeAttribute('disabled');

ok... what this will do, I think, is enable the file item when showing a file
on an ftp server... and uploading it would replace that file...
is that desired? (did ns4 do that?)


would setting .enabled = false/true be better? ask some frontend person, I
guess..


you'll also need to change
xpfe/components/download-manager/src/nsDownloadProgressListener.js I think

xpfe/components/download-manager/src/nsDownloadManager.cpp
   // the persistent descriptor of the target is the unique identifier we use

hm, looks like actually the path is used.
while you're touching this file, could you change this to "the path of the
target ..."

r=me, if you get moa for the xpfe changes from neil or jag; and you need a
firebird peer to review the toolkit/ changes which I didn't look at.

Comment 167

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 145940 [details] [diff] [review]
v2.1 patch

oh, one more thing...
+    this.mReqCount    = 0;
it seems you're never using this variable

Comment 168

[Christian :Biesinger (don't email me, ping me on IRC)]

(in nsProgressDialog.js)

Comment 169

[[e-mail]]

(In reply to comment #166)
>(From update of attachment 145940 [details] [diff] [review])
>>xpfe/browser/resources/content/nsBrowserStatusHandler.js
>>+	   // Enable/disable menu entries for file upload...
>>+	   if (channel.URI.schemeIs("ftp"))
>>+	     this.canUploadFile.removeAttribute('disabled');
> 
>ok... what this will do, I think, is enable the file item when showing a
>file on an ftp server... and uploading it would replace that file...
>is that desired? (did ns4 do that?)
> 
>would setting .enabled = false/true be better? ask some frontend person, I
>guess..
No, menus are magic, because of the Mac, so you can't use xbl properties.

Comment 170

[[e-mail]]

Comment on attachment 145940 [details] [diff] [review]
v2.1 patch

>+    set target(newval) { 
>+        // If newval references a file on the local filesystem, then
>+        // grab a reference to its corresponding nsIFile.
>+        try {
>+            this.mTargetFile = newval.QueryInterface(nsIFileURL).file;
>+        } catch (e) {
>+            this.mTargetFile = null;
>+        }
>+        return this.mTarget = newval;
>+    },
try/catch is not nice for expected QI exceptions, use this:
this.mTargetFile = newval instanceof nsIFileURL ? newval.file : null;

>+            // otherwise, wait for STATE_STOP with aRequest corresponding to
>+            // our target.  XXX redirects might screw up this logic.
>+            try {
>+                var chan = aRequest.QueryInterface(nsIChannel);
>+                if (chan.URI.equals(this.target)) {
>+                    // we are done transfering...
>+                    this.completed = true;
>+                }
>+            }
>+            catch (e) {
>+            }
Now this case I understand that aRequest should be a channel, so in this case
your use of try/catch is OK.

>     QueryInterface: function (iid) {
>         if (!iid.equals(Components.interfaces.nsIProgressDialog) &&
>             !iid.equals(Components.interfaces.nsIDownload) && 
>             !iid.equals(Components.interfaces.nsIWebProgressListener) &&
>             !iid.equals(Components.interfaces.nsIObserver) &&
>+            !iid.equals(Components.interfaces.nsIInterfaceRequestor) &&
>             !iid.equals(Components.interfaces.nsISupports)) {
>             throw Components.results.NS_ERROR_NO_INTERFACE;
>         }
>         return this;
>     },
You forgot to add nsITransfer.

>     // Get filename from target file.
>     fileName: function() {
>-        return this.target ? this.target.leafName : "";
>+        if (this.targetFile)
>+            return this.targetFile.leafName;
>+        if (this.target) {
>+            try {
>+                var url = this.target.QueryInterface(nsIURL); 
>+                if (url)
>+                    return url.fileName;
>+            } catch (e) { /* fall through */ }
>+        }
>+        return "";
>     },
Don't use if to test the return value of QueryInterface, it never returns null.
(Too much C++ programming!)
Hmm... don't you need to unescape the url's fileName? Or don't you have enough
information to do that?

>                 string = this.replaceInsert( string,
>                                              2,
>-                                             this.target.fileSize >> 10 );
>+                                             this.targetFile.fileSize >> 10 );
[OT: I hate this bit, it takes up far too much room]

>     // Hide a given dialog field.
>     hide: function( field ) {
>-        this.dialogElement( field ).setAttribute( "style", "display: none;" );
>-        // Hide the associated separator, too.
>-        this.dialogElement( field+"Separator" ).setAttribute( "style", "display: none;" );
>+        try {
>+            this.dialogElement( field ).setAttribute( "style", "display: none;" );
>+            // Hide the associated separator, too.
>+            this.dialogElement( field+"Separator" ).setAttribute( "style", "display: none;" );
>+        } catch (e) {}
>     },
Eww... I'd prefer to see a null check rather than a try/catch. Also please
change these to use .hidden = true; to hide the XUL elements.

>     // Return input in hex, prepended with "0x" and leading zeros (to 8 digits).
>     hex: function( x ) {
>          var hex = Number(x).toString(16);
>          return "0x" + ( "00000000" + hex ).substring( hex.length );
>     },
[OT: "0x" + ("0000000" + Number(x).toString(16)).slice(-8);]

>+    const filterIndexPref = "browser.open-file.picker-filter-index";
It would be neat if you could save the dir too, as per browser.download.dir or
mail.compose.attach.dir :-)
Personally I don't like that pref name, browser.open.filterIndex would do for
me, or picker_filter_index if you must...

>+          // Enable/disable menu entries for file upload...
>+          if (channel.URI.schemeIs("ftp"))
>+            this.canUploadFile.removeAttribute('disabled');
>+          else
>+            this.canUploadFile.setAttribute('disabled', 'true');
I don't think this is the best place to do this, what happens if you load an
ftp dir listing into one frame and then focus another frame before doing
file/upload? If necessary append the check to the menu_FilePopup onpopupshowing
code.

>       // Create the local directory into which to save associated files. 
>       const lfContractID = "@mozilla.org/file/local;1";
>       const lfIID = Components.interfaces.nsILocalFile;
>       filesFolder = Components .classes[lfContractID].createInstance(lfIID);
>-      filesFolder.initWithPath(persistArgs.target.path);
>+      filesFolder.initWithPath(fp.file.path);
[OT?] filesFolder = fp.file.clone();?

>+/**
>+ * This function extracts the local file path corresponding to the given URI.
>+ */
[OT] Yeah, and someone made the mistake of handing the file path off to
RDF.GetResource :-( With any luck we'll be able to standardize on URLs
throughout.

> public:
>   NS_DECL_NSIWEBPROGRESSLISTENER
>+  NS_DECL_NSITRANSFER
>   NS_DECL_NSIDOWNLOAD
>   NS_DECL_ISUPPORTS
I don't see the equivalent change to NS_IMPL_ISUPPORTS...

Comment 171

[Darin Fisher]

biesi, neil: thank you both for your careful reviews.  new patch coming up....

Comment 172

[Darin Fisher]

(In reply to comment #166)
> (From update of attachment 145940 [details] [diff] [review])
> +  var targetURI = IOS.newURI(fileURL.file.leafName, null, targetBaseURI);
> 
> you don't need a charset here? it seems you can use
> contentFrame.document.characterSet...

No need.  The charset is inherited from targetBaseURI.

Comment 173

[Darin Fisher]

> this.mTargetFile = newval instanceof nsIFileURL ? newval.file : null;

i figured this required the object to implement nsIClassInfo, but now i see that
xpconnect is actually able to remember that instanceof returned true, and then
treat newval as if it had all the attributes and properties of nsIFileURL. 
that's very neat! ;-)

Comment 174

[Darin Fisher]

> Hmm... don't you need to unescape the url's fileName? Or don't you have enough
> information to do that?

right, i was thinking that i don't have enough information to unescape it. 
however, hmm... i could maybe use nsITextToSubURI::unEscapeURIForUI for that. 
it employs an aglo to carefully unescape only stuff that can be interpreted
relative to a particular charset, which is pretty much the best you can hope for.

Comment 175

[Darin Fisher]

Attachment: v2.2 patch

Comment 176

[timeless]

fwiw that's not what classinfo means :). classinfo means js doesn't need to QI
at all for any interface listed in classinfo, they're automatically reflected.

Comment 177

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 146152 [details] [diff] [review]
v2.2 patch

+    this.mReqCount    = 0;

this is still unused...

bug 239115 bitrotted the (seamonkey) downloadmanager changes somewhat, sorry
about that

+  // display name property is unused by the progress dialog implementation,
and
+  // it's not obvious what we should pass for that argument, so just leave it
+  // empty for now.
+  dialog.init(fileURL, targetURI, leafName, null, Date.now()*1000, persist);

the comment doesn't seem to match the code anymore

Comment 178

[Darin Fisher]

> +    this.mReqCount    = 0;
> this is still unused...
...
> the comment doesn't seem to match the code anymore

oops.. thanks biesi!  new patch coming up.

Comment 179

[Darin Fisher]

Attachment: v2.3 patch

revised patch per biesi's last comments

Comment 180

[[e-mail]]

Comment on attachment 146202 [details] [diff] [review]
v2.3 patch

r=me if you can find a way of fixing the size of the upload progress dialog,
it's too small as it stands, probably because the buttons are hidden. Maybe
calling sizeToContent before hiding the buttons would work, or you could just
disable the buttons instead.

When can we expect a multiple upload ;-)

Comment 181

[Darin Fisher]

Neil, I should have commented that I solved the dialog sizing problem with some
help from Ben Goodger.  His suggestion was to add a little bit of CSS to
nsProgressDialog.xul to specify a default width.  It seems that I have not
included that in the v2.3 patch :-(

I'll attach a complete patch shortly.

Comment 182

[Brian Ryner (not reading)]

Comment on attachment 146202 [details] [diff] [review]
v2.3 patch

>--- xpfe/browser/resources/content/navigator.js	10 Apr 2004 09:41:33 -0000	1.535
>+++ xpfe/browser/resources/content/navigator.js	15 Apr 2004 17:38:03 -0000
>+function selectFileToOpen(label, prefRoot)
> {
>+  var fileURL = null;
>+
>   // Get filepicker component.
>+  const nsIFilePicker = Components.interfaces.nsIFilePicker;
>+  var fp = Components.classes["@mozilla.org/filepicker;1"].createInstance(nsIFilePicker);
>+  fp.init(window, gNavigatorBundle.getString(label), nsIFilePicker.modeOpen);
>+  fp.appendFilters(nsIFilePicker.filterAll | nsIFilePicker.filterText | nsIFilePicker.filterImages |
>+                   nsIFilePicker.filterXML | nsIFilePicker.filterHTML);
>+

This will now propagate an exception if one of these methods throws one, is
that what you wanted?

>+  dump("updateFileUploadItem: canUpload=" + canUpload + "\n");

I assume you'll remove this.

sr=bryner with those addressed.

Comment 183

[Darin Fisher]

(In reply to comment #182)
> This will now propagate an exception if one of these methods throws one, is
> that what you wanted?

Yes, each of the callsites traps exceptions.


> >+  dump("updateFileUploadItem: canUpload=" + canUpload + "\n");
> 
> I assume you'll remove this.

oops!  thanks for catching that.  i'll remove it before checking in.

Neil: multi-file upload will have to wait.  the current patch is something i'm
going to try to land for 1.7, so it needs to be kept minimal (as if the current
patch is somehow small)

Comment 184

[Christian :Biesinger (don't email me, ping me on IRC)]

so it looks like this was checked in:
136     set target(newval) { 
137         // If newval references a file on the local filesystem, then grab a
138         // reference to its corresponding nsIFile.
139         this.mTargetFile = newval instanceof nsIFileURL ? newfile.file : null;

note that newfile is not defined in line 139 - it should be newval... supposedly
this breaks progress reporting in the dialog

Comment 185

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: fix for that typo

Comment 186

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146387 [details] [diff] [review]
fix for that typo

r+sr=bzbarsky

Comment 187

[Christian :Biesinger (don't email me, ping me on IRC)]

typo fix checked in
Checking in nsProgressDialog.js;
/cvsroot/mozilla/embedding/components/ui/progressDlg/nsProgressDialog.js,v  <--
 nsProgressDialog.js
new revision: 1.36; previous revision: 1.35
done

this may fix bug 240832

Comment 188

[Jesse Ruderman]

It sounds like that checkin was intended to fix bug 240835.  Did it also fix bug
240811?

Comment 189

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #188)
> It sounds like that checkin was intended to fix bug 240835.  Did it also fix bug
> 240811?

possible... I am not quite sure how firefox's download manager works, and
whether it uses this file. I find it somewhat unlikely, actually.

Comment 190

[[e-mail]]

Attachment: More file open bustage

Two issues:
1. Some platforms don't like a null defaultDirectory
2. openTopWin wants a string, selectFileToOpen returns a file URL.

Comment 191

[Darin Fisher]

Comment on attachment 146495 [details] [diff] [review]
More file open bustage

thanks neil!  my bad for apparently not testing the last round of changes :-(

Comment 192

[Boris Zbarsky [:bzbarsky]]

-                        
fp.file.parent.QueryInterface(Components.interfaces.nsILocalFile));
+                         fp.file.parent.nsILocalFile);

What's with that change exactly?

Comment 193

[Darin Fisher]

> What's with that change exactly?

yeah, i forgot to mention that i spoke to neil about that over IRC.  it turns
out that that change is not needed.  it is valid code, but it is not needed.  in
fact, the patch that i'm going to check in will change that to |fp.file.parent|
since xpconnect will perform the QI automagically based on typelib info.

Comment 194

[Robert Accettura [:raccettura]]

(In reply to comment #190)
> More file open bustage

So I take it this bug is what's causing Mozilla (20040418/WXP) to crash on File
Open?  I noticed that this morning.

Comment 195

[Darin Fisher]

> So I take it this bug is what's causing Mozilla (20040418/WXP) to crash on File
> Open?  I noticed that this morning.

yes, it is.  i'll be checking neil's patch today.

Comment 196

[Darin Fisher]

Comment on attachment 146495 [details] [diff] [review]
More file open bustage

patch checked in with "fp.file.parent.nsILocalFile" changed to "fp.file.parent"

Comment 197

[Darin Fisher]

marking FIXED ... doesn't look like drivers are going to approve this change for
the 1.7 branch.

Comment 198

[Zook Valem]

Is it allowed for this bug to be resolved as fix if there are existing
unresolved dependance bugs already?

Comment 199

[Boris Zbarsky [:bzbarsky]]

Yes, if it's fixed.  Just because there's a dependency doesn't mean this bug
can't be fixed till that one is.  Usually it just means someone thought fixing
that one would make fixing this one easier.

Comment 200

[Jason Bassford]

I think that you're thinking of bug 24496, which was checked in on March 17th. 
However, it may not be hardcoded (it could be a preference) - and, even if it is
hardcoded, I doubt that b.m.o. is using the version of Mozilla to which this was
applied.

FWIW: I completely agree that a bug should be prevented from being marked FIXED
if there are still unresolved dependencies.  (Not blocking bug, but dependent
bugs.)  If it's to be marked fixed, and dependents are still unresolved, then
those dependencies should be removed... (Since then they are, obviously, not
dependencies after all.)

Comment 201

[Boris Zbarsky [:bzbarsky]]

You're using a far narrower definition of "dependency" than most developers do...

Comment 202

[Darin Fisher]

OK, no more dependency "problem" =)

Comment 203

[Zook Valem]

At least, bugzilla got less open bugs today.  :-)  Darin, thank you for all you
do in making the FTP UI and the FTP Upload possible.  It is worth it.

Comment 204

[Robert Accettura [:raccettura]]

I don't see any other bug, or note about it...

Will this patch be ported back to the 1.7 branch for Firefox 1.0?  Or will this
be a post 1.0 Firefox feature?

Comment 205

[Zook Valem]

>>When can we expect a multiple upload ;-)
Any bugs like this in bugzilla, so I can CC's myself to it?  I saw none.

Comment 206

[Jesse Ruderman]

Zook: I reopened bug 208385, which was marked as a dup of this one.  Its summary
is "Drag and drop from Windows Explorer to Mozilla on an ftp site does not
result in an upload".

Comment 207

[Jose Fandos]

Do you know if there is a bug/rfe reported for multiple file uploads?

Comment 208

[Sam Katz]

You mean like a directory upload? In the bug, it's very clear that the aim is to
support drag and drop uploads. I think "Upload File.." does what it has to do
minimally. It would of course be expected that eventually Upload File.. would
support a shift-click to select multiple files, and would queue them.

The thing is this shift-click functionality is probably a different API call..
(not a programmer, but some dialogs support shift-click and some don't.)

Does the current patch / backend subsystem use and anticipate a queue? Because
if it does, we've got part of the implementation already.

--Sam

Comment 209

[Zook Valem]

>>I don't see any other bug, or note about it...
>>Will this patch be ported back to the 1.7 branch for 
>>Firefox 1.0?  Or will this be a post 1.0 Firefox feature?
It is now confirmed from the April 19th, mozilla.org staff meeting report.  What
is confirmed is that this FTP Upload feature will be put into the Mozilla 1.7
Release...   So, look like you got your wish for Firefox 1.0 after all.

--snip--
- Release candidate shooting for tomorrow or Wed
- We need to land a few biggish changes we want to take on the 1.7
   branch first
   - Changes to prevent URL bar spoofing/phishing
   - Find in textareas
   - FTP upload
- These all landed on trunk last week
- As soon as these are in, off we go
- We've scheduled plenty of time on the branch for 1.7.
--snip--

Comment 210

[Asa Dotzler [:asa]]

Comment on attachment 146202 [details] [diff] [review]
v2.3 patch

a=asa (on behalf of drivers) for checkin to 1.7

Comment 211

[Christian :Biesinger (don't email me, ping me on IRC)]

note, bug 241199's patch will also need to be checked in when this lands on branch

Comment 212

[Mike Pinkerton (not reading bugmail)]

aiie, you guys are killing me! ;)

so when darin landed this on the trunk, he also landed a bunch of stuff in
camino (causing bug 241199) to "fix" it. Where does that code live? Won't
landing this break the camino branch?

camino's branch is called MOZILLA_1_7_BRANCH, but is just mozilla/camino, not
the same branch as the rest of seamonkey.

Comment 213

[Mike Pinkerton (not reading bugmail)]

when will this land on the branch?

Comment 214

[Darin Fisher]

Mike: No worries.  I will do my best to ensure that all related changes go in on
the branch when this lands.

Comment 215

[Darin Fisher]

Attachment: complete patch for 1.7 branch (including all followup fixes)

This patch has been checked in on the 1.7 branch.

Comment 216

[Darin Fisher]

fixed1.7

Comment 217

[OstGote!]

Mozilla trunk build 2004050509 WinNT4
Hmmm ... If I login to my webspace at ftp://USERNAME:[e-mail]/ and go
into a subdirectory like ftp://USERNAME:[e-mail]/example/ and then
upload a file the transfer succeeds but the file lands in the "root" of my ftp
space, not in the example directory. I am not sure if this is a Mozilla bug or a
proxy/ISP issue. I can't find an existing, should I file a new one?

Comment 218

[Darin Fisher]

Ack, that problem was supposed to have been fixed by the patch for bug 238847! 
Please comment in that bug that you can still reproduce the problem, and I'll
try to investigate it today.

Comment 219

[Mike Pinkerton (not reading bugmail)]

did the appropriate things land on the camino branch (also called
MOZILLA_1_7_BRANCH)?

Comment 220

[Mike Pinkerton (not reading bugmail)]

i hear it did, thanks ;)

Comment 221

[Darin Fisher]

(In reply to comment #217)
> Mozilla trunk build 2004050509 WinNT4

I just tested today's 1.7 branch build on Linux, and could not reproduce this
problem.  It sounds like the problem you are seeing might be specific to your
ISP's FTP server.  If you could send me more information, like a network trace,
or FTP log, that would be great.  Just set these environment variables before
running Mozilla:

  c:\> set NSPR_LOG_MODULES=nsFTPProtocol:5
  c:\> set NSPR_LOG_FILE=c:\log.txt

Thanks!  (Be sure to inspect the log file for your password... you don't need to
send that to me!)

Comment 222

[Christian :Biesinger (don't email me, ping me on IRC)]

darin: hm.,.. that patch seems to be missing the nsDownloadProgressListener.js
changes (xpfe)?

Comment 223

[Darin Fisher]

(In reply to comment #222)
> darin: hm.,.. that patch seems to be missing the nsDownloadProgressListener.js
> changes (xpfe)?

biesi: thank you for catching that error.  i have checked in the changes to the
1.7 branch.  can you spot any other changes that i forgot?  i spent a lot of
time going through bonsai to make sure that i got everything, but for some
reason i missed this change to nsDownloadProgressListener.js! :(

Comment 224

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #223)
> biesi: thank you for catching that error.  i have checked in the changes to the
> 1.7 branch.  can you spot any other changes that i forgot? 

it looks like you got all the other changes.

Comment 225

[benc]

V branch.

Comment 226

[Valerio Messina]

(In reply to comment #209)
> It is now confirmed from the April 19th, mozilla.org staff meeting report.  What
> is confirmed is that this FTP Upload feature will be put into the Mozilla 1.7
> Release...
> --snip--
> - We need to land a few biggish changes we want to take on the 1.7
>    branch first
>    - FTP upload
there and in whatsnew:
http://www.mozilla.org/releases/mozilla1.7/README.html#new
you speak about FTP upload feature implemented.
Is implemented also the UI?

But where is this new feature???
How can I upload a file?
If I drag a file in Moz opened on a ftp://user:[e-mail] it open then
dialog "what to do with this file?"
For me this bug is also open or it is a hidden feature.
:-))

Comment 227

[Ric Gates]

(In reply to comment #226)
> But where is this new feature???

Same place it's always been, under File

Note the bug Summary "basic UI for FTP upload (menu)"

Comment 228

[Zook Valem]

Remember if you're on the ftp site(s) in the url toolbar then the file upload
link in the File menu would show up, if you on the http site(s) then you
wouldn't find it there.

Comment 229

[Valerio Messina]

Ok, I found it, tried it, and it works well (only for file not directory).
I havent see it because I use localized Mozilla, but (I dont know why) I'm
looked for Upload in File menu, and the traslation dont help.

To ask the delete/rename file capability on an ftp server I must open a new
enhancement bug?
These dont require GUI advanced functionality but little interaction (like right
click on file name).

Comment 230

[Peter Lairo]

I just went to an FTP site(1) with the latest Firefox trunk nightly build(2). I
could see *no UI* for FTP upload in the File menu (or any other menu). Am I
overlooking something? Should this "Core" bug be reopened?

(1) ftp://ftp.netscape.com/pub/netscape8/

(2) Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20050920
Firefox/1.6a1

Comment 231

[Adam Hauner]

In SeaMonkey 1.0a is Upload File command available in File submenu. Maybe you
should file/find bug in Firefox product.

Comment 232

[Daniel de Wildt]

This bug has only fixed the suite/seamonkey. The corresponding firefox bugs are
bug 215673 and bug 208385.