Closed Bug 1301056 Opened 3 years ago Closed 3 years ago

[e10s] Link with target="_blank" to download a file leaves a about:blank window/tab open after the download

Categories

(Firefox :: File Handling, defect, P2)

48 Branch
Unspecified
Windows 7
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
e10s + ---
firefox-esr45 --- unaffected
firefox-esr52 - wontfix
firefox53 --- wontfix
firefox54 + verified
firefox55 + verified

People

(Reporter: jkdefusco, Assigned: mrbkap)

References

Details

(Keywords: regression, Whiteboard: [e10s-multi:-])

Attachments

(2 files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:48.0) Gecko/20100101 Firefox/48.0
Build ID: 20160823121617

Steps to reproduce:

With 48.0.2 click a link, for example to a PDF file or .JNLP file, that has target="_blank" set and PDF attachments set to open automatically and it will open a new tab (or window depending on the browser.link.open_newwindow option) and then open in the PDF client but that tab/window will remain open. In the past that tab/window would instantly closed.

Setting browser.link.open_newwindow to 1 so it forces use of the current tab is a workaround but may cause unexpected issues with other links.

This isn't consistent behavior however as we have ~250 users (Windows 7 32-bit and 64-bit) and it's not happening to everyone but it only started when they upgraded to 48.0.8.


Actual results:

The tab/window for the file that downloads and opens wasn't closed


Expected results:

The tab/window for the file that downloads and opens should've closed
OS: Unspecified → Windows 7
Attached file 1301056.html
I can't reproduce it with the testcase I attached.
If I select "Save File" for PDF in Firefox options, the PDF is downloaded and the extra about:blank tab disappears just after the download.
Ok, I understand the issue. The issue appears only with e10s enabled (which is the case for a small group of FF48 users, for testing purpose).

I'm pretty sure all your affected users have e10s enabled, you can check that in the page about:support, there is a line called "Multiprocess Windows" in the " Application Basics" table.
Summary: Clicking a link that downloads and automatically opens the file causes a new tab/window and leaves that tab/window open when it used to close automatically → [e10s] Clicking a link that downloads and automatically opens the file causes a new tab/window and leaves that tab/window open when it used to close automatically
I tested with Nightly 40 and e10s enabled, same issue. It's probably here since e10s has been implemented.

STR:
1) Be sure e10s is on.
2) about:preferences#applications > Portable Document Format (PDF): Safe File
3) Click on the link of the testcase
Blocks: e10s
tracking-e10s: --- → ?
Component: Untriaged → General
Flags: needinfo?(mconley)
Summary: [e10s] Clicking a link that downloads and automatically opens the file causes a new tab/window and leaves that tab/window open when it used to close automatically → [e10s] Link with target="_blank" to download a file leaves a about:blank window/tab open after the download
Component: General → File Handling
The DOMWindowClose event doesn't appear to be being fired here... we don't seem to enter this block: http://searchfox.org/mozilla-central/rev/591354c5e5521cf845bf6b6ef44e8b3b0aeda17d/docshell/base/nsDocShell.cpp#11093

Which means we don't set docshell.newWindowTarget to true on the channel properties, which means we don't set mShouldCloseWindow here: http://searchfox.org/mozilla-central/rev/591354c5e5521cf845bf6b6ef44e8b3b0aeda17d/uriloader/exthandler/nsExternalHelperAppService.cpp#1639

Which means we don't queue this timer which fires the event which removes the tab: http://searchfox.org/mozilla-central/rev/591354c5e5521cf845bf6b6ef44e8b3b0aeda17d/uriloader/exthandler/nsExternalHelperAppService.cpp#2569

That's as far as I've gotten with my initial pokings. We'll triage this on Thursday.
Flags: needinfo?(mconley)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: polish
Priority: -- → P2
Duplicate of this bug: 1319515
Duplicate of this bug: 1341556
Duplicate of this bug: 1343232
Hi, any news on when the devs are planning to fix this? Thanks.
Hey Erin, just want to make sure this is still on the radar of the e10s team.

If this is what I believe, it's pretty bad as we leave users staring at a blank page in the middle of navigation, and with no "back" button enabled, when the site doesn't actually intend to open a popup window. Some of these users may have never used tabs. I haven't tested this though, maybe one of the bug reporters can comment on whether this is actually what they're observing.
Flags: needinfo?(elancaster)
Keywords: polish
Hey Paolo, I confirm what you say, Yes, that is what I am observing. Still an issue on recent versions.
My experience was, that if you would start a download it will open a new blank tab, but in previous versions (48/49) the tab would close after the download had started, whereas in Version 51 the tab will stay openned.
Thank you for reporting and raising this. I will escalate.
Whiteboard: [e10s-multi:?]
Flags: needinfo?(elancaster)
Whiteboard: [e10s-multi:?] → [e10s-multi:-]
Attachment #8844968 - Flags: review?(mconley)
Assignee: nobody → mrbkap
Comment on attachment 8844968 [details]
Bug 1301056 - Fix auto-closing of windows when they divert to an external app.

https://reviewboard.mozilla.org/r/118224/#review120606

D'oh, good find. We should probably consider uplifting this.
Attachment #8844968 - Flags: review?(mconley) → review+
Pushed by mrbkap@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/43cc8e497475
Fix auto-closing of windows when they divert to an external app. r=mconley
https://hg.mozilla.org/mozilla-central/rev/43cc8e497475
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Depends on: 1350058
Looks like a trivial fix? Shall we nominate for Beta/ESR52 approval?
Flags: needinfo?(mrbkap)
Don't forget bug 1350058. I'd like to get a mochitest in the tree before requesting approval. I'll try as hard as I can to get to that today or tomorrow (leaving the NI on me for that).
Tracking 54/55 and esr52+ for this e10s regression.
Comment on attachment 8844968 [details]
Bug 1301056 - Fix auto-closing of windows when they divert to an external app.

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: New tabs and windows opened for content types we don't handle won't be automatically closed.
Fix Landed on Version: 55
Risk to taking this patch (and alternatives if risky): Medium risk, given that there was one regression (bug 1350058).
String or UUID changes made by this patch: n/a

See https://wiki.mozilla.org/Release_Management/ESR_Landing_Process for more info.

Approval Request Comment
[Feature/Bug causing the regression]: e10s
[Is this code covered by automated tests?]: Yes, see bug 1359651
[Has the fix been verified in Nightly?]: Yes.
[Needs manual test from QE? If yes, steps to reproduce]: No.
[List of other uplifts needed for the feature/fix]: bug 1350058 bug 1359651
Flags: needinfo?(mrbkap)
Attachment #8844968 - Flags: approval-mozilla-esr52?
Attachment #8844968 - Flags: approval-mozilla-beta?
Hi Brindusa, could you help find someone to verify if this issue was fixed as expected on a latest Nightly build? Thanks!
Flags: qe-verify+
Flags: needinfo?(brindusa.tot)
I tested this issue on Windows 10 x64 with FF Nightly 55.0a1(2017-04-27) and I can confirm the fix.
Status: RESOLVED → VERIFIED
Flags: needinfo?(brindusa.tot)
Comment on attachment 8844968 [details]
Bug 1301056 - Fix auto-closing of windows when they divert to an external app.

Fix a regression related to e10s and was verified. Beta54+. Should be in 54 beta 4.
Attachment #8844968 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Flags: in-testsuite+
Verified fixed Fx 54b4, Win 10.
Flags: qe-verify+
Comment on attachment 8844968 [details]
Bug 1301056 - Fix auto-closing of windows when they divert to an external app.

Given that the risk to uplift this is medium and that it led to at least one regression, I am leaning towards not uplifting this to ESR52. Tabs not closing automatically is not ideal but it's not as severe as a data loss/crashy situation.
Attachment #8844968 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52-
Depends on: 1373109
See Also: → 1376404
You need to log in before you can comment on or make changes to this bug.