393556 - Failed downloads not removed from active downloads array

Status: VERIFIED FIXED

(Toolkit :: Downloads API, defect)

Description

[Marco Bonardo [:mak]]

I had tried to download a no more existent file from the web, i ended up with 3 failed and 1 cancelled downloads (i clicked retry and cancel, one time). notice that it's the same file, so i was expecting to only get one entry instead of 4, is this by design?

Now when i ask to exit Firefox i get "If you exit now, 4 downloads will be cancelled"... those are already failed or cancelled, so the dialog should not warn me, i have no downloads in progress. 
I tryed to find a dupe but i can't

Build identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a8pre) Gecko/2007082415 Minefield/3.0a8pre

Comment 1

[Shawn Wilsher :sdwilsh]

if you can get me a reproducible test case for this, I can fix it, but I haven't been able to reproduce this (but I have heard of reports of this, so I'm not denying that it exists).

Comment 2

[Marco Bonardo [:mak]]

i have found this in this way, i have no time now to find more correct steps, probably they are not all needed

1. start with an empty profile (empty folder, so it's recreated on launch) [maybe optional] and use only the starting tab (no other tabs)
2. go to www.google.it and search pdf
3. find "[PDF] UNIX: introduzione elementare" right click on link and choose save as
4. save to desktop
5. the download fails with an error dialog, repeat steps 3 & 4 a couple of times without waiting for error dialog (is this error dialog by design?)
6. close the download manager, then close Firefox

i think that point 3 and 4 (repeated a couple of time) should be enough to reproduce, i'll try to find better steps in the next days, if this does not help

Comment 3

[Marco Bonardo [:mak]]

mh that download works now so i cannot reproduce with that link... before i was getting an error dialog, probably the server was temporarly down... i'll try to find another link to test...

Comment 4

[Shawn Wilsher :sdwilsh]

This looks suspect:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp&rev=1.106#1529
We need to add nsIDownloadManager::DOWNLOAD_FAILED here...

Comment 5

[Shawn Wilsher :sdwilsh]

Yes, that looks to be about all we are missing.

Comment 6

[Shawn Wilsher :sdwilsh]

Litmus test would need to try a download that will fail.

Comment 8

[Reed Loden [:reed]]

Attachment: patch - v1

As you say...

Comment 9

[Marco Bonardo [:mak]]

maybe i've found a simple way to reproduce, using a not existant domain, try to download this with Save As:

http://www.domain.fake/not_existant_document.pdf

i have tested your suggested fix in comment #4 and it works.

On failing i get an error dialog, telling me that the file cannot be downloaded because the source is no more available, this is an annoyance, why should i get a dialog, i already see the FAILED indicator in the dm, i'd prefer to have some way in the dm to see why it failed. 
What if i download a bunch of not existant files, have i to click OK on all dialogs? 
Is this the correct behaviour or should i fill another bug to search for a better solution in showing download errors?

Comment 10

[Reed Loden [:reed]]

Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
/cvsroot/mozilla/toolkit/components/downloads/src/nsDownloadManager.cpp,v  <--  nsDownloadManager.cpp
new revision: 1.107; previous revision: 1.106
done

Comment 11

[Stephen Donner [:stephend] Not actively reading bugmail]

http://litmus.mozilla.org/show_test.cgi?id=4595

in-litmus+

Comment 12

[Marco Bonardo [:mak]]

ehy stephen, the litmus test is not valid, you should use comment #9 link, maybe an html with such a link in it...
Will not work using original steps, since the link was temporarly invalid, while now it's valid and cannot be used to reproduce

the only steps are
1. try to download with save as the link http://www.domain.fake/not_existant_document.pdf
2. wait for the download to fail
3. close the DM
4. close the browser

Expected Results:
the quit dialog does not complain about losing in progress downloads

Comment 13

[Shawn Wilsher :sdwilsh]

You also shouldn't need a clean profile.  It would have happened all the time.

Comment 14

[Stephen Donner [:stephend] Not actively reading bugmail]

Litmus testcase fixed; can you guys refresh and let me know?

Thanks!

Comment 15

[Marco Bonardo [:mak]]

s/DM/Download Manager/
s/pending-dialog/pending-download/

it's ok, i've tried to follow steps with current nightly and it fails, while it was fine with custom build (including the fix)

Comment 16

[Stephen Donner [:stephend] Not actively reading bugmail]

I'm going to spin off a bug that, when you right-click and try to save that non-existent PDF file, although you get the "...could not be saved, because the source file cannot be read..." alert ("not_existant_document.pdf [sic]"), the bogus file's entry isn't populated until you close and reopen the DM window; oddly enough, successive files (if you repeat the steps) seem to populate their entries correctly.  What happens, instead, is that the "Active--------" section appears, but with no entries.

Anyway, I'm splitting that off because one can still exit cleanly without any bogus "Downloads in progress" warning message.

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a8pre) Gecko/2007082618 Minefield/3.0a8pre on Windows Vista, Mac OS X 10.4, and Fedora Core 7.
 

Comment 17

[Stephen Donner [:stephend] Not actively reading bugmail]

I filed bug 393821 to get a cleaner bug.