1656753 - 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

Status: VERIFIED FIXED

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

Description

[bob]

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.

Comment 1

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

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

Comment 2

[J]

(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?

Comment 3

[kernp25]

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.

Comment 4

[kernp25]

Attachment: JC9EGpVGK6.mp4

Comment 5

[kernp25]

Attachment: uGP8MVNan1.mp4

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

Comment 6

[kernp25]

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

Comment 7

[kernp25]

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

Comment 8

[bob]

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.

Comment 9

[:Gijs (he/him)]

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?

Comment 10

[bob]

(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

Comment 11

[bob]

Attachment: Issues occurs with other files types as well, here's a PDF example.

Comment 12

[:Gijs (he/him)]

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

Comment 13

[bob]

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

Comment 14

[:Gijs (he/him)]

(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.

Comment 15

[bob]

Thanks you Gijs!

Comment 16

[Chris Peterson [:cpeterson]]

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.

Comment 17

[Nika Layzell [:nika] (ni? for response)]

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.

Comment 20

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext,

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/451025ce8016
Track CrossGroupOpener on CanonicalBrowsingContext, r=farre

Comment 22

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/451025ce8016

Comment 23

[bob]

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!!!

Comment 24

[Release mgmt bot [:sylvestre / :calixte / :marco for bugbug]]

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.

Comment 25

[Nika Layzell [:nika] (ni? for response)]

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

Comment 26

[Timea Cernea [:tbabos]]

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 27

[Julien Cristau [:jcristau]]

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

approved for 82.0b5

Comment 28

[Julien Cristau [:jcristau]]

https://hg.mozilla.org/releases/mozilla-beta/rev/dcef1bf63160

Comment 29

[Timea Cernea [:tbabos]]

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

Comment 30

[Mike Kaply [:mkaply]]

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:

Comment 31

[Mike Kaply [:mkaply]]

Yes, this needs uplift to ESR.

Comment 32

[Pascal Chevrel:pascalc]

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

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

Comment 33

[Julien Cristau [:jcristau]]

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

Comment 34

[Nika Layzell [:nika] (ni? for response)]

Attachment: Bug 1656753 - Track CrossGroupOpener on CanonicalBrowsingContext

Rebased for esr78

Comment 35

[Nika Layzell [:nika] (ni? for response)]

(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

Comment 36

[Julien Cristau [:jcristau]]

Thanks Nika!

https://hg.mozilla.org/releases/mozilla-esr78/rev/35408d78e4eed57838117d7a38e9a6b2b0bd3420

Comment 37

[Timea Cernea [:tbabos]]

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