159162 - Downloading continues after clicking Cancel or close window button in download dialog

Status: VERIFIED FIXED

(Camino Graveyard :: Downloading, defect)

Description

[Winnie Lam]

07-24 build on OS 10.1.5

Found this bug while verifying 145800.

1.  Download a file, e.g. from ftp://ftp.mozilla.org/pub/chimera/nightly/latest
2.  Single click to download the file.
3.  Download dialog comes up.  Select where the file is to be downloaded to.
4.  Download begins.
5.  Click the Cancel button (or the red close window button) in the download dialog.
6.  Download dialog disappears.

Expected result:  Download stops.
Actual result:  Download continues and finishes.  The downloaded file appears
where I had specified it to go in step 3.

Comment 1

[Winnie Lam]

This behavior does not happen when right-clicking to download a file.

Comment 2

[Greg K.]

Using FizzillaCFM/2002072203, I never even saw a download progress dialog; the
download simply happened in the background.

Comment 3

[sairuh (rarely reading bugmail)]

i'm wondering if this might be affected by the annoying "start predownloading
before the user selects target location" feature. see bug 55690 (and bug 129923)
for the gorey details.

Comment 4

[Steve Dagley]

taking.  I was in the urloader code recently so I'll see what's happening here

Comment 5

[Steve Dagley]

I can't duplicate this problem ina  debug build.  anybody seen it recently in a
production build?

Comment 6

[Steve Dagley]

Here's the problem:

NS_IMETHODIMP
nsDownloadListener::SetObserver(nsIObserver * aObserver)
{
  return NS_ERROR_NOT_IMPLEMENTED;
}

the uriloader code is never being notified when the Cancel button is hit in the
download progress dialog.

Comment 7

[Steve Dagley]

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

Comment 8

[Steve Dagley]

Attachment: Implement nsDownloadListener::SetObserver

nsDownloadListener::SetObserver is needed so uriloader is notified when a
download is canceled otherwise it'll attempt to post-process the partially
downloaded file rather than deleting it

Comment 9

[Steve Dagley]

Comment on attachment 108235 [details] [diff] [review]
Implement nsDownloadListener::SetObserver

asking ccarlen for his review since code was lifted from UDownload

Comment 10

[Simon Fraser [no longer active]]

Nuke the " - se sez ccarlen.". otherwise it looks OK.

Comment 11

[Conrad Carlen (not reading bugmail)]

Comment on attachment 108235 [details] [diff] [review]
Implement nsDownloadListener::SetObserver

>? Chimera.pbproj/sdagley.pbxuser
>Index: src/download/nsDownloadListener.h
>===================================================================
> 
>+    // These two are mutually exclusive - se sez ccarlen.

Don't do it just because I said so in the context of PPEmbed ;-) (get rid of
it)

>Index: src/download/nsDownloadListener.mm
>===================================================================
> NS_IMETHODIMP
> nsDownloadListener::SetObserver(nsIObserver * aObserver)
> {
>-  return NS_ERROR_NOT_IMPLEMENTED;
>+  if (aObserver)
>+    aObserver->QueryInterface(NS_GET_IID(nsIHelperAppLauncher), getter_AddRefs(mHelperAppLauncher));

    Shorten to:
    mHelperAppLauncher = do_QueryInterface(aObserver);
>+  return NS_OK;
> }
> 

Otherwise, good.

Comment 12

[Steve Dagley]

Checked in

Comment 13

[Chris Petersen]

Verified in the 2002-12-05-04 NB under 10.2.2