nsExternalAppHandler needs to be able to support auto-saving and auto-dispatching of downloaded files



Core Graveyard
File Handling
15 years ago
2 years ago


(Reporter: Simon Fraser, Assigned: Bill Law)


Mac OS X
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment)



15 years ago
Camino needs to be able to provide to the user a streamlined downloading
experience, where the download is automatically saved into their
system-specified download folder, and the download is auto-dispatched to the
helper app when done (as controlled by a pref, off by default).

We hacked nsExternalHelperAppService.cpp on the chimera branch to do this. We
now need a cleaner way to do it on the trunk.

Comment 1

15 years ago
*struggles through the morass of downloading APIs*

So you'd think that an embedder could just run a 'save to disk'-type download,
and launch the file themselves at the end of it. But no! There's logic in
nsExternalAppHandler::MoveFile that assumes that if the file is a 'save to disk'
download, then it will already have a unique name so we don't need to uniquify
the name. It assumes that if the file exists, the user had hit 'Replace' in the
file picker, so if a similarly named file exists when done (which might be hours
later), we can happily go ahead and replace it with our downloaded file. This is

Here's what I think should happen. Implementors of 'PromptForSaveToFile' should,
if the user hits 'Replace' in the file picker, promptly delete that file. Then,
nsExternalAppHandler::MoveFile() should always uniquify the name, never
replacing existing files.

Of course, the real fix would be to remove the name salting altogether. Only
then do users get what they really expect (specify a file location to save to,
then way data being accumulated into that file).

Comment 2

15 years ago
not that camino cares, but don't forget that a user might want to use mozilla to
resume a download into a file on disk (which mozilla didn't download).
So... is this a memo-to-self bug?  Or rather, do we plan to try and fix this
before fixing the general pre-downloading mess?

Comment 4

15 years ago
This is a "we have fix regressions in downloading in trunk-based camino builds".
Patches coming.

Comment 5

15 years ago
Created attachment 117439 [details] [diff] [review]
Fix nsExternalAppHandler::SaveToDisk when aNewFileLocation is non-null

This change allows an embedder to call nsExternalAppHandler::SaveToDisk,
providing a file to download to. It ensures that CreateProgressListener() and
ProcessAnyRefreshTags() are called in this case.

Comment 6

15 years ago
Associated Camino changes are in the patch in bug 192461.


15 years ago
Blocks: 192461
Comment on attachment 117439 [details] [diff] [review]
Fix nsExternalAppHandler::SaveToDisk when aNewFileLocation is non-null

this looks fine to me.	Let me know whether you want an r or an sr....

And I'd really like to clean up these interfaces to make them actually useful
to embeddors.  Feedback is very welcome (someone else doing the changing is
even more welcome.... ;) ).  Interface elimination, creation, heavy
modification, all are fair game here.

Comment 8

15 years ago
Comment on attachment 117439 [details] [diff] [review]
Fix nsExternalAppHandler::SaveToDisk when aNewFileLocation is non-null

I lxr'd, and no-one currently calls SaveToDisk with a non-null
aNewFileLocation. So this won't break anything.
Attachment #117439 - Flags: superreview?(bzbarsky)
Attachment #117439 - Flags: review?(sdagley)
Attachment #117439 - Flags: superreview?(bzbarsky) → superreview+


15 years ago
Attachment #117439 - Flags: review?(sdagley) → review+


15 years ago
Blocks: 197865

Comment 9

15 years ago
Checked in.
Last Resolved: 15 years ago
Resolution: --- → FIXED


15 years ago
Keywords: nsbeta1+

Comment 10

15 years ago
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.