47248 - files aren't saved where expected via Download [new helper app dialog]

Status: VERIFIED FIXED

(Core :: Networking, defect, P3)

Description

[sairuh (rarely reading bugmail)]

spun off from bug 46836, specifically my comment at 2000-08-01 16:27... tested
on Linux and Mac using pr2 comm branch bits 2000.08.01.04-m17. reproducible on
Linux, erratic on Mac.

1. go to http://www.mozilla.org
2. in the Nightly Build section, click the Macintosh link. (other links on linux
and mac will result in the Unknown File Type dialog, which is not applicable to
this bug, methinks.)
3. the Download [new help app/save dialog] appears. select the Save on Disk
radiobutton. on the Mac you'll encounter bug 47240, but in spite of that (or,
rather, when it's done involuntarily downloading) click that radiobutton anyhow.
4. click the OK button.
5. the file picker dialog should appear --shrir was able to get this far on Mac,
i couldn't on my machine, perhaps due to bug 46313 or some machine-specific
weirdness. anyway, if you get this far, select a location other than the default
setting (somewhere easy to find, tho', and where you have write privs,
o'course).
6. click OK in the file picker dialog.

results: the Download dialog will persist (bug 46836). moreover, however, no
file is ever downloaded --check the directory that was set in step 5, and the
file won't be there.

Comment 1

[sairuh (rarely reading bugmail)]

kw's-o-rama.

Comment 2

[sairuh (rarely reading bugmail)]

comment from joseph in bug 46836: "on linux, I think that the file is downloaded
to /tmp, so although it appears the file was not downloaded - it's actually in
/tmp"

yep, that's where they're hiding on my linux box, too... i wonder, then, where
they're [mysteriously] stored on the Mac? shrir?

if in both cases they are being saved, just not where the user specifies, then
i'll resummarize this...

Comment 3

[leger]

Putting on [nsbeta2+] radar to review with PDT today.

Comment 4

[sairuh (rarely reading bugmail)]

modified summary per joseph's comment.

shrir, could you pls vrfy/find where the files were saved on your Mac (since i
cannot seem to cleanly repro this on mine)? thx!

Comment 5

[Scott MacGregor]

The only way I can see where hitting the OK button from this dialog would cause
an unimplemented error to get dumped to the console is if nsIFile::MoveTo isn't
implemented on the Mac for some reason.

Are we sure this problem is real for linux? i.e. yes we are saving to a temp
file but after we are done downloading you should see the file get moved to the
location you specified in the save as dialog.

Taking from gagan. This isn't related to necko.

Comment 6

[Scott MacGregor]

Wait a minute if the only problem in this bug is that files aren't saved where
they are expected than this is invalid. By design, we start downloading to a
temp file in your temp directory (or onto the desktop for mac users). Then the
user chooses a destination via the file picker that you access from the helper
app dialog. When you hit okay, we move the file from the tmp location to the
final location you specified in the dialog.

I suspect the only real bug here is Mac. This works for me as specified on Linux
and Windows. I think the problem is that hitting the OK button generates this JS
exception which of course prevents the dialog from doing its duty and moving the
file to th new location. Hence, the file isn't saved where expected. 

Comment 7

[sairuh (rarely reading bugmail)]

h'm, if it is Mac only, i wonder: could this be in anyway related to bug 47240
(also a Mac only bug dealing with this dialog)?

Comment 8

[Scott MacGregor]

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

Comment 9

[Paul Wyskoczka]

This occurs on all platforms using today's branch builds.

Comment 10

[sairuh (rarely reading bugmail)]

scott: heh, was so busy trying narrow down this issue w/paul that i didn't see
your comments today's till now.

"I think the problem is that hitting the OK button generates this JS
exception which of course prevents the dialog from doing its duty and moving the
file to th new location. Hence, the file isn't saved where expected." thanks,
that definitely clarifies things...

however, as paul noted above, i see this occur on linux, winnt and mac using
today's branch commercial bits. is it working for you using trunk bits? feel
free to come over to my cube, if you're local, and i'll be happy to show you
what we've been seeing. (if you're not local, i'm on IRC #qa as 'se'.)

Comment 11

[Scott MacGregor]

Yeah this is working for me using todays windows release builds. What exactly
are you still seeing on linux and windows? The file save as dialog isn't getting
dimissed? 

Comment 12

[Scott MacGregor]

Okay i wasn't using a big enough file. Now I can see it. Necko is no longer
issuing an OnStopRequest for these large file downloads.......*sigh* so we never
try to bring up the save location dialog (this happens after the on stop request
is issued).

Comment 13

[sairuh (rarely reading bugmail)]

hi scott: your comment above at 2000-08-02 14:45 sounds like that's the cause of
bug 46313 --could it be? just wondering...

Comment 14

[Scott MacGregor]

I take that last comment back. It looks like dimissing the helper app dialog
triggers the on unload event which is issing a cancel event to the window! So
the helper app dialog is canceling the download. That's why we don't bring up
the save to location dialog. 

Comment 15

[Tom Everingham]

just checked Linux and NT and it does look as though the files are saved to the 
/tmp directory and then moved to the directory selected with filepicker.  On NT 
though, if you select 'OK' in the 'Downloading' dialog before the file is 
completely transfered to the temp directory, you never get a chance to specify 
where to store the file.  

Comment 16

[Scott MacGregor]

attaching a fix. Guys, can you help me test this out. You should be able to find
this JS file in the installed seamonkey directory (just do a search for it and
replace it with this attached helperAppLauncher.js file).

For me, with this file, I'm seeing the save as dialog come up AFTER we are done
downloading the file (so you have to wait for it). But it does come up and the
file ends up in the correct location. 

Comment 17

[Scott MacGregor]

the http post port for bugzilla seems to be down I keep timing out when I try to
post. 

Comment 18

[Scott MacGregor]

No bugzilla just won't accept attachments right now.

There were two problems with the helper app dialog that I just fixed:
1) It had an unload handler which called Cancel! so when you hit save to disk
and hit okay, we'd end up flagging the load as being canceled when the dialog
went away.
2) in the save to disk case the helper app dialog was not marking the mime info
as save to disk. So we didn't know to bring up the save to disk prompt after we
downloaded the file.

With my changes for these two things, this works for me just great on windows.
The only catch is that you have to wait until we are done downloading before we
bring up the save as dialog.

It's also working for me on Mac as well.

I'm going to mark the other bugs which are beta2+ and all seem to be related as
dups of this one. 

Comment 19

[Scott MacGregor]

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

Comment 20

[Scott MacGregor]

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

Comment 21

[Scott MacGregor]

Since I can't post the fix I'll show you what to do by hand:
1) find helperAppLauncher.xul
line 41 has an onunload= .Cancel();
Delete that line

2) find helperAppLauncher.js
around line 102 you need to add the following:
this.appLauncher.MIMEInfo.preferredAction = this.nsIHelperAppLauncher.saveToDisk;
so the else case now looks like:
else {
            this.appLauncher.MIMEInfo.preferredAction =
this.nsIHelperAppLauncher.saveToDisk;
            this.appLauncher.saveToDisk( null, dontAskNextTime );
        }

Comment 22

[leger]

Adding 8/02

Comment 23

[leger]

Approved, check into branch ASAP please.

Comment 24

[Scott MacGregor]

Fix checked into the branch. Waiting for a green tree to check into the tip.
Thanks for the help folks!

Comment 25

[Tom Everingham]

tried your changes on win NT.  'Save as' dialog is now showing up after download 
and file ends up in correct dir.

Comment 26

[leger]

Moving to paw to verify with Aug 03 branch bits in AM....since he is VERY 
familiar with this :-)

Comment 27

[Paul Wyskoczka]

Works on WinNT 4.0 but not on Linux or Mac. Saved file in temp dir or desktop is
not moved to user defined destination.  I will reopen the bug.

Comment 28

[Scott MacGregor]

sorry i've been at an api review meeting all day and didn't get back until just
now.

What are you seeing on mac and linux? You are asked for the new file location
corretly right? Does the file get copied to the new location and we just aren't
deleting the original or is it more severe than that?

Comment 29

[sairuh (rarely reading bugmail)]

more observations: looks like the js and xul files were updated on all platforms
(checking the 2000.08.03.04-m17 branch). however, i see what Paul sees --it's
not working *completely* on the Mac or Linux.

steps/observations:

1. go to the usual URL above.
2. scroll down to Nightly Builds.
3. click the Macintosh link.
4. the Download dialog [helper app] appears. also, while the statusbar's
progress meter is "going" i click the Save to Disk radiobutton then OK.
5. after the progress meter is done, the Download dialog goes away and the file
picker appears (as expected, yay!).
6. i select another directory (other than the desktop on mac, other than /tmp on
linux for which i've write privs), click OK.

result: the file picker dialog goes away...but the file isn't moved from the
desktop or /tmp.

mscott: any ideas why the file mv is failing on linux and mac?

Comment 30

[Scott MacGregor]

oh dear....looks like nsIFile::MoveTo isn't implemented on the Mac or linux.
cc'ing dougt the guru for this stuff. 

Comment 31

[Scott MacGregor]

Okay there are two problems (one for linux, one for mac)
1) There's a bug in nsLocalFileMac::GetParent which was causing us to get a
bogus parent under certain conditions.
There was an if clause that looked like this:
if (appendedPath == nsnull || (appendedPath[0] == ':' && appendedPath[1] == '\0' ))
we should also enter this clause if appendedPath is an empty string. Adding that
fixed the mac problems for me.

2) On Linux, it's more serious. nsLocalFileUnix::MoveTo is unimplemented!!

I need dougt to review my change to nsLOcalFileMac (and possibly a Mac person? I
also need his guidance on what to do about ::MoveTo on Linux.

Comment 32

[Scott MacGregor]

Attachment: unix fix to moveto

Comment 33

[Scott MacGregor]

The fix I described above for the Mac problem has been checked into the branch.
MoveTo is working again on the Mac for me.

I have a patch for unix which I have to test to make sure it works.

Comment 34

[Scott MacGregor]

Okay I've landed the fixes for both of the problems on both the branch and the
tip. Re-closing. 

Comment 35

[John Unruh]

Verified. Works on Mac, Linux and WinNT.

Comment 36

[benc]

-relnote2 - this is fixed and not needed in current relnotes.