Closed Bug 355222 Opened 14 years ago Closed 13 years ago

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

Categories

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

defect

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.9beta1

People

(Reporter: stevee, Assigned: Gavin)

References

()

Details

(Keywords: regression, testcase, verified1.8.1)

Attachments

(2 files, 2 obsolete files)

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)
Flags: blocking1.8.1.1?
Flags: blocking-firefox2?
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.
I can confirm this too.

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/2006100104 Minefield/3.0a1
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
Summary: Download fails when using 'Always ask me where to save files' NS_ERROR_FAILURE @ nsIFilePicker.init Line 148 → Some download can fail when using 'Always ask me where to save files' NS_ERROR_FAILURE @ nsIFilePicker.init Line 148
Summary: Some download can fail when using 'Always ask me where to save files' NS_ERROR_FAILURE @ nsIFilePicker.init Line 148 → Some downloads can fail when using 'Always ask me where to save files' NS_ERROR_FAILURE @ nsIFilePicker.init Line 148
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
Attached file part 2 of testcase (obsolete) —
Attached file testcase
Attachment #241052 - Attachment is obsolete: true
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.
Assignee: nobody → file-handling
Blocks: 343921
Component: Download Manager → File Handling
Flags: blocking-firefox2?
Keywords: testcase
Product: Firefox → Core
QA Contact: download.manager → ian
Version: 2.0 Branch → 1.8 Branch
Flags: blocking1.8.1?
Attached patch patchSplinter Review
This fixes the bug for me, by not closing the download-triggering window if it's parent has been closed.
Assignee: file-handling → gavin.sharp
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8.1
Attachment #241057 - Flags: superreview?(darin)
Attachment #241057 - Flags: review?(cbiesinger)
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?
Flags: blocking1.8.1? → blocking1.8.1+
Attached patch alternate patch (diff -w) (obsolete) — Splinter Review
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.
(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 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.
Attachment #241080 - Attachment is obsolete: true
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.
Attachment #241057 - Flags: review?(cbiesinger) → review+
Attachment #241057 - Flags: superreview?(darin) → superreview+
Attachment #241057 - Flags: approval1.8.1?
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).
Flags: blocking1.8.1.1?
Comment on attachment 241057 [details] [diff] [review]
patch

a=beltzner on behalf of drivers
Attachment #241057 - Flags: approval1.8.1? → approval1.8.1+
mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp 	1.290.4.10
Keywords: fixed1.8.1
OS: Windows 2000 → All
Hardware: PC → All
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.
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.
Priority: -- → P1
Target Milestone: mozilla1.8.1 → mozilla1.9beta1
Version: 1.8 Branch → Trunk
Duplicate of this bug: 383812
As mentioned in bug 383812, bug 353560 made this problem occur regardless of whether the "ask me" pref is set.
Summary: Some downloads can fail when using 'Always ask me where to save files' NS_ERROR_FAILURE @ nsIFilePicker.init Line 148 → downloads fail when a window triggers a download in a new window and then closes itself
Flags: blocking1.9?
http://litmus.mozilla.org/show_test.cgi?id=4588

in-litmus+ (Gavin, if you could review the testcase, that'd be great; thanks!)
Flags: in-litmus? → in-litmus+
(In reply to comment #21)
> (Gavin, if you could review the testcase, that'd be great; thanks!)

Looks good.
Flags: blocking1.9? → blocking1.9+
Target Milestone: mozilla1.9 M8 → mozilla1.9 M9
Target Milestone: mozilla1.9 M9 → ---
Target Milestone: --- → mozilla1.9 M9
Poke.  What's the story on landing this on trunk?  
Poke #2.  Update?
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.
Attachment #241057 - Flags: review?(cbiesinger)
Comment on attachment 241057 [details] [diff] [review]
patch

that's ok with me
Attachment #241057 - Flags: review?(cbiesinger) → review+
Yay.  :)
Attachment #241057 - Flags: approval1.9?
Comment on attachment 241057 [details] [diff] [review]
patch

Oh, this is a blocker. Right!
Attachment #241057 - Flags: approval1.9?
mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp 	1.352 
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
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
Status: RESOLVED → VERIFIED
(In reply to comment #25)
> I can file a followup to come up with a cleaner solution.

Gavin, could you do that, though?

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

Yep, filed bug 402977.
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.