355222 - downloads fail when a window triggers a download in a new window and then closes itself

Status: VERIFIED FIXED

(Core Graveyard :: File Handling, defect, P1)

Description

[Steve England [:stevee]]

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061002 BonEcho/2.0 ID:2006100204

From chitotuason @ http://forums.mozillazine.org/viewtopic.php?t=470594

1. New profile, start firefox
2. Tools > Options > Main, set Downloads to 'Always ask me where to save files'
3. Go to http://www-307.ibm.com/pc/support/site.wss/document.do?lndocid=MIGR-41929
4. Under the 'File details' heading, click 1awc32ww.exe (The 12,602,784 byte file , IBM High Rate Wireless LAN Mini PCI adapter driver)
5. A popup window appears. Choose either an FTP or an HTTP download.
6. Click "I Agree" to accept the download
7. The popup goes to get the file, and then a firefox dialog pops up "You have chosen to open 1awc32ww.exe which is an Application from ftp://ftp.software.ibm.com. Would you like to save this file? [Save File] [Cancel]". Click 'Save File'
8. An exception is thrown, and the file does not download.

Error: [Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIFilePicker.init]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: file:///C:/Documents%20and%20Settings/logonname/Desktop/Firefox%202.0/firefox/components/nsHelperAppDlg.js :: anonymous :: line 148"  data: no]
Source File: file:///C:/Documents%20and%20Settings/logonname/Desktop/Firefox%202.0/firefox/components/nsHelperAppDlg.js
Line: 148

Works in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.0.7) Gecko/20060909 Firefox/1.5.0.7

Doesn't work in:
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20060918 Firefox/2.0 (rc1)
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061002 BonEcho/2.0 ID:2006100204 (nightly)

Comment 1

[Reed Loden [:reed]]

I can confirm this on Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1) Gecko/20061002 BonEcho/2.0 ID:2006100204.

Comment 2

[Michael Ventnor]

I can confirm this too.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/2006100104 Minefield/3.0a1

Comment 3

[Steve England [:stevee]]

Works (Clicking 'Save File' displays filepicker dialog):
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b1) Gecko/2006080803 BonEcho/2.0b1

Doesn't work (Clicking 'Safe File' throws exception and download doesn't start):
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1b1) Gecko/2006080903 BonEcho/2.0b1

Checkins
http://tinderbox.mozilla.org/bonsai/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=MOZILLA_1_8_BRANCH&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2006-08-08+02%3A00%3A00&maxdate=2006-08-09+04%3A00%3A00&cvsroot=%2Fcvsroot

Comment 4

[WildcatRay]

FYI: When I first tried this, I had not yet given (NoScript) permission for www.ibm.com and the file downloaded without issue. I tried again with NoScript not blocking scripts from ibm.com, got the popup asking for agreement and chose the http download site, got the Save File dialog, chose to save the file and the file did not download.

Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1) Gecko/20061002 BonEcho/2.0 ID:2006100204

Comment 5

[:Gavin Sharp [email: [e-mail]]]

Attachment: part 2 of testcase

Comment 6

[:Gavin Sharp [email: [e-mail]]]

Attachment: testcase

Comment 7

[:Gavin Sharp [email: [e-mail]]]

The "pick download method" window opens a window to download the file, and then closes itself. Bug 343921 made us close this second popup and made the passed in window context be the "pick download method" window, but by the time the dialog appears that window is closed.

Comment 8

[:Gavin Sharp [email: [e-mail]]]

Attachment: patch

This fixes the bug for me, by not closing the download-triggering window if it's parent has been closed.

Comment 9

[:Gavin Sharp [email: [e-mail]]]

This patch reverts to our old behavior (before bug 343921) in the case where the original window is closed. Perhaps instead we should walk the window.opener chain and find the closest remaining open window instead of giving up if the immediate parent isn't open?

Comment 10

[:Gavin Sharp [email: [e-mail]]]

Attachment: alternate patch (diff -w)

I'm not nearly as confident about this patch, but it does fix this bug, by using the original parent (the original ibm.com page) as the parent. This might have other consequences that I haven't considered.

Comment 11

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #10)
> I'm not nearly as confident about this patch, but it does fix this bug, by
> using the original parent (the original ibm.com page) as the parent.

As the "Save dialogs" parent, I mean. This has the advantage of still closing the blank popup after the download is complete, which doesn't happen with the first patch.

Comment 12

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 241080 [details] [diff] [review]
alternate patch (diff -w)

Biesi and I discussed this on IRC, and my alternate patch has the problem that it may parent the filepicker to a completely unrelated window, and is a more complicated change to make on the branch at this stage, so I'm going to stick with attachment 241057 [details] [diff] [review] for the branch, and try to work on a better solution for the trunk.

Comment 13

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 241080 [details] [diff] [review]
alternate patch (diff -w)

Biesi and I discussed this on IRC, and my alternate patch has the problem that it may parent the filepicker to a completely unrelated window, and is a more complicated change to make on the branch at this stage, so I'm going to stick with attachment 241057 [details] [diff] [review] for the branch, and try to work on a better solution for the trunk.

Comment 14

[:Gavin Sharp [email: [e-mail]]]

I've tested this patch with "Ask me" pref on and off, on both the original IBM.com site and the testcase attached in this bug. I've looked through the other bugs related to this feature (bug 343921 and bug 241972) for other testcases (there are none), and tested that the functionality added in those bugs still works (the original site from bug 241972, the test page from bug 241972 comment 45, various links at download.com and snapfiles.com).

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 241057 [details] [diff] [review]
patch

a=beltzner on behalf of drivers

Comment 16

[:Gavin Sharp [email: [e-mail]]]

mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp 	1.290.4.10

Comment 17

[Marcia Knous [:marcia]]

verified on the 1.8.1 branch using Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1) Gecko/20061003 Firefox/2.0. I followed the steps in the report and did not receive any exception. I also downloaded a number of other files from download.com and other locations and did not see any issues. adding verified keyword.

Comment 18

[:Gavin Sharp [email: [e-mail]]]

I haven't had a chance to look into this further since the branch patch was landed. Perhaps we should land that patch on the trunk.

Comment 20

[:Gavin Sharp [email: [e-mail]]]

As mentioned in bug 383812, bug 353560 made this problem occur regardless of whether the "ask me" pref is set.

Comment 21

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

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

in-litmus+ (Gavin, if you could review the testcase, that'd be great; thanks!)

Comment 22

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #21)
> (Gavin, if you could review the testcase, that'd be great; thanks!)

Looks good.

Comment 23

[Damon Sicore (:damons)]

Poke.  What's the story on landing this on trunk?  

Comment 24

[Damon Sicore (:damons)]

Poke #2.  Update?

Comment 25

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 241057 [details] [diff] [review]
patch

Sorry, I've been trying to get a hold of biesi to make a decision here. I'd like to just land the patch that was landed on the 1.8 branch on the trunk. It might not be perfect, but it fixes the bug and is known not to cause any big problems. I can file a followup to come up with a cleaner solution.

Comment 26

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 241057 [details] [diff] [review]
patch

that's ok with me

Comment 27

[Damon Sicore (:damons)]

Yay.  :)

Comment 28

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 241057 [details] [diff] [review]
patch

Oh, this is a blocker. Right!

Comment 29

[:Gavin Sharp [email: [e-mail]]]

mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp 	1.352 

Comment 30

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

Verified FIXED using Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9a9pre) Gecko/2007102005 Minefield/3.0a9pre and Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.9a9pre) Gecko/2007102004 Minefield/3.0a9pre

Comment 31

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #25)
> I can file a followup to come up with a cleaner solution.

Gavin, could you do that, though?

Comment 32

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #31)
> Gavin, could you do that, though?

Yep, filed bug 402977.