Prompted to close 2 windows when closing parent window of popup

RESOLVED FIXED in Firefox 66

Status

()

defect
P2
normal
RESOLVED FIXED
6 months ago
3 months ago

People

(Reporter: jscher2000, Assigned: Gijs)

Tracking

({regression})

63 Branch
Firefox 66
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(firefox-esr60 unaffected, firefox63 wontfix, firefox64 wontfix, firefox65 wontfix, firefox66 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 months ago
User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

(1) With any number n of tabs in a regular window

(2) Click a link that triggers a window.open() with features (e.g., third link on https://www.jeffersonscher.com/res/popit.html )

(3) Click the X close button on the regular window


Actual results:

Firefox displayed "Confirm close" dialog reading: "You are about to close 2 windows with (n+1) tabs. Are you sure you want to continue?"

If you continue, Firefox closes the main window, but does not close the popup.


Expected results:

Since Firefox is not going to close the popup, it should at most display a "Confirm close" dialog reading: "You are about to close n tabs. Are you sure you want to continue?" (assuming n > 1).

Ref. https://support.mozilla.org/questions/1238132
(Reporter)

Comment 1

6 months ago
Running Mozregression for this was a bit of a nightmare because I had to keep resetting browser.tabs.warnOnClose and browser.warnOnQuit to their default values, and our security software needs 5 confirmation prompts clicked for the "in between" builds... 

2018-10-25T14:22:07: DEBUG : Found commit message:
Bug 1483935 - correctly check all windows for tabs when quitting, r=mconley

Bug 1438499 added an optional parameter to warnAboutClosingTabs.
In bug 1475427, the arguments to warnAboutClosingTabs changed, and instead
of passing a closing tab reference as the second argument, we now need to
pass the number of tabs as the first argument. The patch in that bug
updated the callsite in nsBrowserGlue.js to add the new argument, but
didn't remove the `null` argument that we were passing for the 'extra' tab.

Additionally, the change in bug 1475427 bails early from
warnAboutClosingTabs if the number of tabs passed is less than 2. That tab
count, too, needs to take into account multiple windows and not just the
last window iterated over.

This patch fixes both of these issues.

Differential Revision: https://phabricator.services.mozilla.com/D3807
(In reply to jscher2000 from comment #0)
> User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:63.0) Gecko/20100101
> Firefox/63.0
> 
> Steps to reproduce:
> 
> (1) With any number n of tabs in a regular window
> 
> (2) Click a link that triggers a window.open() with features (e.g., third
> link on https://www.jeffersonscher.com/res/popit.html )
> 
> (3) Click the X close button on the regular window
> 
> 
> Actual results:
> 
> Firefox displayed "Confirm close" dialog reading: "You are about to close 2
> windows with (n+1) tabs. Are you sure you want to continue?"
> 
> If you continue, Firefox closes the main window, but does not close the
> popup.
> 
> 

I don't get these results on my Surface Book, Windows 10, Firefox 63.
i.e. a dialogue box saying you are about to close n tabs is displayed, i click on the diaglue and the main windows is closed and the popup is not closed

> Expected results:
> 
> Since Firefox is not going to close the popup, it should at most display a
> "Confirm close" dialog reading: "You are about to close n tabs. Are you sure
> you want to continue?" (assuming n > 1).
> 
> Ref. https://support.mozilla.org/questions/1238132
Blocks: 1483935
Status: UNCONFIRMED → NEW
Component: Untriaged → Tabbed Browser
Ever confirmed: true
Keywords: regression
(Reporter)

Comment 3

6 months ago
(In reply to Roland Tanglao needinfo please :rolandtanglao, :mohnkuchen, :adobo, :sinigang, :roland from comment #2)
> (In reply to jscher2000 from comment #0)
> 
> I don't get these results on my Surface Book, Windows 10, Firefox 63.
> i.e. a dialogue box saying you are about to close n tabs is displayed, i
> click on the diaglue and the main windows is closed and the popup is not
> closed

Sorry, I somehow forgot to mention that this occurs when the "main window" is the only regular window.
(Assignee)

Comment 4

6 months ago
This is a result of `warnAboutClosingWindow` having logic that doesn't consider popups real windows. As a result, it treats this case as closing the last "real" window, triggering the `browser-lastwindow-close-requested` observer topic, which nsBrowserGlue treats the same as quitting, even though it doesn't, in fact, close the last window (or close the browser).

I don't really understand why we decided not to treat popups as real windows and therefore treat it as quitting - maybe because it's not trivial to "get back to" a working browser in those cases (on non-macOS)?

I'd be curious what happens on builds prior to bug 1438499, if you close the last "real" window, leaving the popup open, with the `warnOnQuit` dialog on, and pick 'Save and Quit'. Do navigations in the popup window get saved? What kind of session is even saved? Or do we just actually quit the browser in that case? What happens on macOS?

This is pretty messed up, and it's not clear to me what the correct behavior would be.
(Reporter)

Comment 5

6 months ago
(In reply to :Gijs (he/him) from comment #4)
> I don't really understand why we decided not to treat popups as real windows
> and therefore treat it as quitting - maybe because it's not trivial to "get
> back to" a working browser in those cases (on non-macOS)?

"Trivial" is hard to define. Photon's menu button appears along with the location bar in popups-with-features, at least on Windows, so New Window is immediately available, and with more digging, Library > History > Recently Closed Windows is available. On the other hand, the location bar is read-only, there's no Home button, there's no Bookmarks Toolbar, and the Bookmarks Sidebar is a chore to open (buried deeply in the Library menu). So while the popup is a usable window for some purposes, many users may find it difficult to figure out what to do next, other than close it and start Firefox again.
Priority: -- → P2
Getting a bit late for 64, but we can still take a patch for 65.
(Assignee)

Comment 7

5 months ago
(In reply to :Gijs (he/him) from comment #4)
> I'd be curious what happens on builds prior to bug 1438499, if you close the
> last "real" window, leaving the popup open, with the `warnOnQuit` dialog on,
> and pick 'Save and Quit'. Do navigations in the popup window get saved? What
> kind of session is even saved? Or do we just actually quit the browser in
> that case?

I used mozregression, so unsure what gets saved, but you do get a "Quit" dialog, and if you either 'save and quit' or 'quit' then we leave the popup window open. So in that sense, it's just as broken as today, but it's easier to trigger this today because you don't need to trip a bunch of about:config pref to get it.


Feels like we should just only ask about the current window and avoid hitting the 'last window' case if this isn't, in fact, the last window, even if the remaining windows are popups. That's what happened before without the 'save and quit' dialog manually enabled, and seems not completely unreasonable, even if it could be improved.
Too late to fix in 64. Marking this issue as fix-optional for 65; if you land a patch in nightly and think it's low-risk for beta, please request uplift.
(Assignee)

Updated

4 months ago
Assignee: nobody → gijskruitbosch+bugs
Status: NEW → ASSIGNED
Attachment #9033399 - Attachment description: Bug 1502179 - don't treat closing last non-popup window as quitting when a popup window is still open (on Windows/Linux), r?jaws → Bug 1502179 - don't treat closing last non-popup window as quitting when a popup window is still open (on Windows/Linux), r?jaws!,mikedeboer!

Comment 11

3 months ago
Pushed by gijskruitbosch@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/1e4397c150d9
don't treat closing last non-popup window as quitting when a popup window is still open (on Windows/Linux), r=jaws,mikedeboer

Comment 12

3 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 3 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
Flags: in-testsuite+

Per IRC discussion with Gijs, it'd be better to let this ride the trains.

Comment 14

3 months ago

I can reproduce this issue with Nightly 65.0a1 (2018-10-25) (64-bit) on Linux x86_64

I am verifying that this is fixed in latest Nightly 66.0a1

Build id             20190117215514
User Agent       Mozilla/5.0 (X11; Linux x86_64; rv:66.0) Gecko/20100101 Firefox/66.0

QA Whiteboard: [bugday-20190116]
You need to log in before you can comment on or make changes to this bug.