Closed Bug 1656753 Opened 6 months ago Closed 4 months ago

Cannot download files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2) - the new window closes before the download takes place

Categories

(Core :: DOM: Navigation, defect, P2)

79 Branch
Unspecified
Windows
defect

Tracking

()

VERIFIED FIXED
83 Branch
Tracking Status
firefox-esr78 82+ verified
firefox79 --- wontfix
firefox80 --- wontfix
firefox81 --- wontfix
firefox82 --- verified
firefox83 --- verified

People

(Reporter: bob, Assigned: nika)

References

(Regressed 1 open bug, Regression)

Details

(Keywords: regression)

Attachments

(5 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0

Steps to reproduce:

I'm using Firefox v79 on window 10 2004, though the issue probably occurs in other versions of Firefox as well

  1. issues occurs when attempting to download a zip file from "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/", Issue occurs on any website, this simply is the one I've experienced the issue on
  2. go to the Firefox options page, search for tabs, check the "Open links in tabs instead of new windows"
  3. go to the "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/", make on OS section, and then click on the VGA download like or any other down which has a zip payload. Note that the file downloads correctly.
  4. Now go to the Firefox options page, search for tabs, uncheck the "Open links in tabs instead of new windows"
  5. now go to the "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/" page again and attempt to to download the same file previously downloaded
  6. Note the Firefox attempts to open a new window to download the zip file but immediately closes without downloading the file
  7. Now if you again check the "Open links in tabs instead of new windows" the problem goes away. Downloading a zip file should not be affected by this option.
  8. Also note the action for zip files is set to "always ask".
  9. I believe this issue occurs with other file types as well when using the "always ask" action..

Actual results:

Does ask to save the file to and doesn't download the file. It appears that Firefox is crashing when attempting to open the new window (i.e. not tab) for downloading the file.

Expected results:

Should have asked to save the file, should have download the file, and should not have crashed.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → File Handling

(In reply to bob from comment #0)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0

Steps to reproduce:

I'm using Firefox v79 on window 10 2004, though the issue probably occurs in other versions of Firefox as well

  1. issues occurs when attempting to download a zip file from "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/", Issue occurs on any website, this simply is the one I've experienced the issue on
  2. go to the Firefox options page, search for tabs, check the "Open links in tabs instead of new windows"
  3. go to the "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/", make on OS section, and then click on the VGA download like or any other down which has a zip payload. Note that the file downloads correctly.
  4. Now go to the Firefox options page, search for tabs, uncheck the "Open links in tabs instead of new windows"
  5. now go to the "https://www.asus.com/us/Motherboards/P8Z77V_PRO/HelpDesk_Download/" page again and attempt to to download the same file previously downloaded
  6. Note the Firefox attempts to open a new window to download the zip file but immediately closes without downloading the file
  7. Now if you again check the "Open links in tabs instead of new windows" the problem goes away. Downloading a zip file should not be affected by this option.
  8. Also note the action for zip files is set to "always ask".
  9. I believe this issue occurs with other file types as well when using the "always ask" action..

Actual results:

Does ask to save the file to and doesn't download the file. It appears that Firefox is crashing when attempting to open the new window (i.e. not tab) for downloading the file.

Expected results:

Should have asked to save the file, should have download the file, and should not have crashed.

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0 and Firefox/81.0

I could recreate this bug on the given site, but not on any other similar sites.

I tried recreating the bug on this site and was able to download the zip file:

https://downloadcenter.intel.com/download/29786/Intel-Graphics-BETA-Windows-10-DCH-Drivers?product=80939

Bob, can you download the zip file off of this site? could you give us another example site that you are experiencing the bug on?

Flags: needinfo?(bob)

The link on this page has a download attribute.

While the links on this page has the target attribute with the value _blank.

I can also confirm this bug.

Flags: needinfo?(gijskruitbosch+bugs)
Attached video JC9EGpVGK6.mp4
Attached video uGP8MVNan1.mp4

Choosing "Save File" in the options also does not work.

I have tested it on 81.0a1 (2020-08-05) (64-Bit) and the bug also happens here.

If you remove the download attribute and add target="_blank" then the bug will also happen on this page.

Here is another site that has the same behavior ... https://www.iogear.com/support/dm/driver/GBU421

Akso note that this works perfectly well in IE11, Chrome, and MS-Edge.

Flags: needinfo?(bob)

I can repro on Windows but not macOS, for some reason.

:baku, this regressed with this window: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=d405a129956e8714fe211844a72ee5db2c97d5f6&tochange=3d8f1d454ced501fc5baf70a2c0c58c49acb2eaa

Can you take a look?

Status: UNCONFIRMED → NEW
Component: File Handling → DOM: Navigation
Ever confirmed: true
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(amarchesini)
Keywords: regression
Product: Firefox → Core
Regressed by: 1531289
Summary: Cannot download zip files → [Windows] Cannot download zip files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2)
Has Regression Range: --- → yes
Has STR: --- → yes

(In reply to bob from comment #8)

Here is another site that has the same behavior ... https://www.iogear.com/support/dm/driver/GBU421

Also note that this works perfectly well in IE11, Chrome, and MS-Edge.

(In reply to kernp25 from comment #3)

The link on this page has a download attribute.

While the links on this page has the target attribute with the value _blank.

I can also confirm this bug.

Issue not zip specific can occur with other file types as well. Here's a .pdf example of the same issue
webpage: https://oem.sena.com/harley-davidson/
select any PDF download

Yeah, I figured as much but didn't want to mess too much with the summary you chose as the bugreporter. Updated now.

Summary: [Windows] Cannot download zip files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2) → [Windows] Cannot download files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2) - the new window closes before the download takes place

Not sure what you mean by wontfix, is this comment for firefox80 only? Or simply wontfix in any release?

(In reply to bob from comment #13)

Not sure what you mean by wontfix, is this comment for firefox80 only? Or simply wontfix in any release?

Only for 80 - the last beta build for 80 is being created today and there's no patch to fix the issue yet, so it can't make that release anymore.

Thanks you Gijs!

Baku, do you have any updates on this download bug?

The download prompt might be trying to target the download window after it has been closed.

Severity: -- → S2
Flags: needinfo?(nika)
OS: Unspecified → Windows
Priority: -- → P2
Summary: [Windows] Cannot download files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2) - the new window closes before the download takes place → Cannot download files opened with target _blank when _blank opens windows instead of tabs (browser.link.open_newwindow=2) - the new window closes before the download takes place

I did some investigation, and I think I know the cause of this issue.

When we begin a download in response to _blank-targeted load, we open the new window or tab, and start the load as a normal navigation. The load only switches to a download load after the initial response is received from the network with the MIME type. In order to avoid empty windows or tabs, we automatically close them if we determine they're not needed anymore using MaybeCloseWindowHelper: https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/docshell/base/nsDSURIContentListener.cpp#46

The specific BrowsingContext which triggered the load is important, as it's used to determine the parent window for the download dialog, but we can't continue to use the existing BrowisngContext once we've closed it, so we instead switch the target context to a different one using ChooseNewBrowsingContext here: https://searchfox.org/mozilla-central/rev/969fc7fa6c3c7fc489f53b7b7f8c902028b5169f/docshell/base/nsDSURIContentListener.cpp#57

In the past this was always handled by following the "opener" relationship between windows, however in bug 1531289, it was noticed that noopener loads would not have an opener, so the tab would open and not close. To fix this, the code was updated to re-parent the dialog to the embedding chrome window when the tab is going to be closed.

Unfortunately, in the new-window case, this would behave strangely, as the chrome window will also be closed when the last tab in that window is closed. It seems that for non-windows platforms, this ends up being OK, as the dialog is created before the chrome window is fully destroyed, and functions without a parent window. On Windows, however, it seems we inform the OS of the relationship, and it takes the liberty of closing our dependent dialogs for us when the window goes away.

I don't think there's a great way to detect that the tab-to-be-closed is the only toplevel tab in it's window from native code, as that's mostly a frontend concept, and ideally we would continue closing these empty windows anyway. I think the best solution here may be to track the "cross-group opener" on the CanonicalBrowsingContext. This would be like the normal opener, except that we'd also track it for noopener window loads, and it's not synced between processes. We could then change the dialog parent selection logic to use cross-group opener instead.

Assignee: nobody → nika
Flags: needinfo?(nika)
Duplicate of this bug: 1661486
Duplicate of this bug: 1661656
Pushed by nlayzell@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/451025ce8016
Track CrossGroupOpener on CanonicalBrowsingContext, r=farre
Status: NEW → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → 83 Branch
Flags: needinfo?(amarchesini)

Awesome! So I tested this out on the Firefox Nightly Build 83.0a1 (2020-09-26) and it worked as perfectly! Thanks for fixing this issue, greatly appreciated!!!

The patch landed in nightly and beta is affected.
:nika, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(nika)

Comment on attachment 9176922 [details]
Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext,

Beta/Release Uplift Approval Request

  • User impact if declined: When Firefox is configured to open pop-ups in new windows, instead of tabs, clicking on a download link may not work correctly on some websites.
  • Is this code covered by automated tests?: No
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: Yes
  • If yes, steps to reproduce: See comment 1
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Fairly simple change with low chance of crashes or other issues.
  • String changes made/needed: None
Flags: needinfo?(nika)
Attachment #9176922 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

Reproduced the issue on affected Beta 82.0b4 on Windows 10 (can't reproduce on MacOS nor Linux)
Can also confirm the fix on latest Nightly 83.0a1 (2020-09-28) (64-bit) on Windows 10 x64.
Waiting for the fix to also land on Beta to verify further.

Comment on attachment 9176922 [details]
Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext,

approved for 82.0b5

Attachment #9176922 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Verified-fixed on latest Firefox Beta 82.0b5 (64-bit) on Windows 10 x64.
Hi Nika, does this need further uplift to ESR78?

Flags: needinfo?(nika)

Comment on attachment 9176922 [details]
Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext,

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Regression showing up in enterprise contexts
  • User impact if declined: Unable to open certain applications.
  • Fix Landed on Version: 82
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky):
  • String or UUID changes made by this patch:
Attachment #9176922 - Flags: approval-mozilla-esr78?

Yes, this needs uplift to ESR.

Comment on attachment 9176922 [details]
Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext,

Verified by QA on nightly and beta. Uplift approved, thanks.

Attachment #9176922 - Flags: approval-mozilla-esr78? → approval-mozilla-esr78+
Flags: needinfo?(nika)

I'm hitting conflicts when uplifting to esr78, AFAICT with the changes in bug 1640019. Can you provide a rebased patch?

Flags: needinfo?(nika)

(In reply to Julien Cristau [:jcristau] from comment #33)

I'm hitting conflicts when uplifting to esr78, AFAICT with the changes in bug 1640019. Can you provide a rebased patch?

I haven't had a chance to test this to make sure it builds / acts properly yet, but I've attached a rebased version of the patch as https://phabricator.services.mozilla.com/D93396

Flags: needinfo?(nika)

Verified-fixed on latest ESR 78.4.0esr (64-bit) on Windows 10 x64.

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+
See Also: → 1678965
Regressions: 1678965
See Also: 1678965
You need to log in before you can comment on or make changes to this bug.