150984 - Chimera fails to download non-displayable files via FTP (single-click link or button)

Status: VERIFIED FIXED

(Camino Graveyard :: General, defect)

Description

[Chris Petersen]

Build: 2002-06-10-05
Platform: OS X 10.1.5
Expected Results: Clicking  on file or link should open save dialog immediately
What I got: Clicking on file or link appears to do nothing. The user must right-
click or control- click on link , then select Save file as.

BTW, This is probably a dup of another bug but couldn't find this specific issue
in bug summary.

Steps to reproduce:

1) Go to url
2) Click on Chimera build
3) Save dialog doesn't appear like expected

Comment 1

[Chris Petersen]

This is very cumbersome on the user to have to select Save File as to properly
download a file. Needs to automatically open save dialog or open Download window
after clicking.

Comment 2

[Johann Ali]

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

Comment 3

[saari (gone)]

->dagley

Comment 4

[Winnie Lam]

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

Comment 5

[Stephane Moureau]

Build 20020621 16:08
With ftp, an alert says "550 ..path../Chimera.dmg.gz: Not a directory."
ftp://ftp.mozilla.org/pub/chimera/nightly/latest-trunk/

Comment 6

[Greg K.]

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

Comment 7

[Steve Dagley]

This should just fall out when carlen lands 118203.  Leaving open to verify.

Comment 8

[saari (gone)]

local file landing is done, but this isn't fixed yet. Needs more work

Comment 9

[Steve Dagley]

Hrm, now I'm doubting this is directly related to 118203.  It looks more like
the backend code isn't figuring out that it needs to download the file.  Digging
deeper...

Comment 10

[Conrad Carlen (not reading bugmail)]

There are at least these issues:
1. We don't have anything which implements NS_IHELPERAPPLAUNCHERDLG_CONTRACTID.
The lack of that causes nsExternalAppHandler::OnStartRequest to abort. Even
making a dummy impl of the component which would just answer "open using helper
app" without posing any UI is some work.
2. The mac version of nsOSHelperAppService.cpp needs to be hooked into the build
instead of the Unix one. This is just a matter of file movement and Makefile
changes. This is the code that makes use of nsILocalFileMac.

Comment 11

[Mike Pinkerton (not reading bugmail)]

Attachment: as far as i've gotten

this is as far as i got. i still get 

WARNING: Cannot tell if
file:///Users/pink/Library/Chimera/Profiles/default/8lavktqv.slt/downloads.rdf
is a directory or file, file nsIOServiceUnix.cpp, line 71

and the file doesn't start downloading.

Comment 12

[benc]

Can someone modify this summary so that it reflects the symptom more? This
effectively breaks ftp URLs for people, but the summary doesn't say "ftp". I
found this bug by searching RESOLVED bugs w/ "ftp" in the summary.

Comment 13

[Greg K.]

Taking a stab at a new summary. Chimera will successfully access files via FTP
that it can display such as HTML, text, or image files, but fails when directed
to access a file it must download, such as .gz.

Comment 14

[Conrad Carlen (not reading bugmail)]

Attachment: patch for uriloader Makefile

With the files in their new place, it builds (haven't tested) with these
Makefile changes.

Comment 15

[Conrad Carlen (not reading bugmail)]

Attachment: complete patch to uriloader to build IC

Comment 16

[Steve Dagley]

Attachment: prompts for file name & dest, still doesn't work

minor update to pink's patch.  Still not working with Mac version of uriloader
building :-(

Comment 17

[Conrad Carlen (not reading bugmail)]

In nsExternalHelperAppHandler::ExecuteDesiredAction(), mProgressWindowCreated ==
FALSE. This stops the code from moving the downloaded file from temp to where it
should be, or launching it, etc.

Comment 18

[Conrad Carlen (not reading bugmail)]

Attachment: allows download to complete - though no progress shown

With this patch, the download actually completes. There is no progress shown
though, so that needs to be filled in. It replaces the component which
implements nsIDownload rather than using the one from xpfe which depends on
having a download manager. Right now, I don't think we need a manager. Let's
just hook the stubbed out impl of nsIDownload and nsIWebProgressListener to our
download dialog.

Comment 19

[saari (gone)]

can we get what we have checked in please? We need to burn a CD early on monday,
and we really need this working, even without a progress dialog.

Comment 20

[Steve Dagley]

Unfortunately we can not use the progress dialog implementation in
ProgressDlgController.mm as the progress dialog for downloads initiated by
uriloader.  It's not just a dialog, it initiates and controls the download itself.  

Comment 21

[Steve Dagley]

Ok, it's 5:30 in the morning and I'm still finding problems with the automatic
download and launch helper app sans progress dialog so it looks like Conrad's
patch that prompts and saves sans progress is the best we have for now.

Comment 22

[saari (gone)]

Is there any way we can toss up a simple dialog with this patch that simply says
something like "downloading..."?

Comment 23

[Mike Pinkerton (not reading bugmail)]

patch landed, with no visible progress. I took a look at separating the progress
dialog controller so it can be re-used and it's not hard, but it will take more
than the two hours i had available to do the work.

qa: verify that files are downloaded, but i'm opening another bug on the lack of
progress (bug 156300)

Comment 24

[Simon Fraser [no longer active]]

> Unfortunately we can not use the progress dialog implementation in
> ProgressDlgController.mm as the progress dialog for downloads initiated by
> uriloader.  It's not just a dialog, it initiates and controls the download
itself.  

So can't we have some kind of proxy or controller that controls the download,
and have both speak to the same dialog?

Comment 25

[Conrad Carlen (not reading bugmail)]

> So can't we have some kind of proxy or controller that controls the download,
and have both speak to the same dialog?

That's my plan.

Comment 26

[Mike Pinkerton (not reading bugmail)]

after looking at the code, there's only a little in the progress dialog
controller that knows about the particular download listener that impls the
weird "manual save" behavior. it should be easy, as conrad noted, to abstract
this out so we can re-use the controller.

Comment 27

[Chris Petersen]

With the 2002-07-08-13 build, single-clicking on ftp link results in a save
dialog opening. I can specify the name and location of file. Verifying that file
is saved and can be opened in Finder.

Comment 28

[Brian Ryner (not reading)]

I think this is missing uriloader/exthandler/mac/Makefile.in in the patch. To
diff new files, you need to first add the file with 'cvs add', then use the -N
option to 'cvs diff'.

Comment 29

[Steve Dagley]

Just to clarify the details on bryner's comment re: a missing Makefile.in -
uriloader/exthandler/Makefile.in takes care of building files in
uriloader/exthandler/mac/