1564738 - Missing window controls (minimize, maximize and close buttons) on a window opened as a popup and restored

Status: RESOLVED FIXED

(Firefox :: Session Restore, defect, P2)

Description

[YUKI "Piro" Hiroshi]

Attachment: Screenshot to describe how the problem appears

Steps to reproduce

1. Start Firefox.
2. Go to about:config.
3. Ensure that browser.link.open_newwindow.restriction is 2, not 0.
4. Hit Ctrl-Shift-K to open the web console below the about:config page.
5. Enter a script window.open('about:blank?1', '_blank', 'all=yes') and run it.
6. A popup window is opened without regular UI.
7. Close the popup window.
8. Restore the last closed window with Ctrl-Shift-N.

Expected result

A window is restored with regular UI, or restored as a popup window.

Actual result

A window is restored without regular UI and it has no window controls.

Environment

• Windows 10 Pro 1809
• Firefox 68.0
• Nightly 70.0a1, build ID 20190709153742

Comment 1

[YUKI "Piro" Hiroshi]

Attachment: mozregression log

I've tried to find the regression window with mozregression, and I've realized that the problem didn't happen on a nightly build of mozilla-central at 2018-03-22 but happens on a build at 2018-03-23. However I couldn't narrow down the window more smaller, due to mozregression's error. The attachment is the log of the regression.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:mikedeboer, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 3

[Mike de Boer [:mikedeboer]]

It'd be great if someone could find the bug that regressed this!

Comment 4

[Alice0775 White]

Regression window(from local cache):
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=5bc910c1f477d8095dcde2c96b7e8639211b9a84&tochange=15b7991887bc8021ba21108c3d31f461fab47a7f

Suspect: 9dd8a5bb9f13de8747538469c749d60b95b812fe Dão Gottwald — Bug 1448286 - Remove redundant gBrowser.tabContainer.updateVisibility call. r=florian

FYI,
After happen the issue, if you run gBrowser.tabContainer.updateVisibility() in Browser toolbox, then the control button reappears.

Comment 5

[Kim Moir [:kmoir] ET]

Mike Is this bug still considered S2 and will the team have time to work on this for 79?

Comment 6

[Dão Gottwald [:dao]]

(In reply to Kim Moir [:kmoir] ET from comment #5)

Mike Is this bug still considered S2 and will the team have time to work on this for 79?

Unclear if we'll have a fix ready for 79 but I'm looking into it.

Comment 7

[Peter Magyari (Desktop QA)]

Hi,

I know this issue is old and hasn't been updated in a while but it seems to have been fixed so think we could close it.

Would you mind checking if it still occurs on your end?

Thanks!

Comment 8

[YUKI "Piro" Hiroshi]

This still happens. Environment:

• Nightly 87.0a1 build ID: 20210215162656
• Windows 10

Comment 9

[u608768]

Attachment: Bug 1564738 - Stop passing arguments to openWindow for restored windows, r?annyG

1. nsWindowWatcher::OpenWindow uses argv length to determine if the window being
   opened is a "dialog".

   We currently pass a single empty string argument for session restore, so
   session restored windows are treated as dialogs. This was added in bug 337375
   because "session restore disturbs secondary firefox window sizing", but
   Session Store now tracks and restores window sizes on its own, so this
   shouldn't be required anymore.

   https://searchfox.org/mozilla-central/rev/2aa97aea1085cf1363582725407c514833ad47e4/toolkit/components/windowwatcher/nsWindowWatcher.cpp#291

2. When calculating chrome flags, if the dialog flag is set, and window features
   includes "all=yes" (which it does for session restore), then we'll set the
   flag to nsIWebBrowserChrome::CHROME_ALL.

   https://searchfox.org/mozilla-central/rev/2aa97aea1085cf1363582725407c514833ad47e4/toolkit/components/windowwatcher/nsWindowWatcher.cpp#1907-1909

3. When setting up style data for the widget that we're creating, if CHROME_ALL
   is set on the mask, then we set the border style to eBorderStyle_all, instead
   of calculating specific border styles, as we do for windows opened via
   window.open().

   https://searchfox.org/mozilla-central/rev/2aa97aea1085cf1363582725407c514833ad47e4/xpfe/appshell/nsAppShellService.cpp#660-662

Comment 13

[petr.laskevic]

(In reply to YUKI "Piro" Hiroshi from comment #8)

This still happens. Environment:

• Nightly 87.0a1 build ID: 20210215162656
• Windows 10

Also happens on LInux Mint MATE 20.2 and macOS

Comment 14

[u608768]

The attached patch doesn't actually fix some cases (notably bug 1659844 IIRC). I think we'll need to track the original window.arguments and use that in the Services.ww.openWindow() call when opening a window for session restore. There was some chat about this in the #fission channel on Matrix.

Edit: per discussion, Session Restore may just need to track more window features here, and use those when opening the new window for restore.

Comment 15

[u608768]

Attachment: Bug 1564738 - Unconditionally call TabBarVisbility.update() when adding "multiple" tabs, r?Gijs

The regressing bug (bug 1448286) removed a redundant call to
gBrowser.tabContainer.updateVisibility. It appears that removed call was
necessary for restoring the tabbar of windows that were originally opened as
dialogs.

In D53692, we tried to avoid the extra cost of removing and readding the tab if
we were restoring a single-tabbed window, but it turns out that it's required to
get around the above.

Comment 16

[u608768]

See https://phabricator.services.mozilla.com/D124358#4103871.

Comment 19

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/78000a724660
Unconditionally call TabBarVisbility.update() when adding "multiple" tabs, r=Gijs

Comment 20

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/267258da3564
Backed out changeset 78000a724660 for tab related bc failures. CLOSED TREE

Comment 21

[Cosmin Sabou [:CosminS]]

Backed out for tab related bc failures.

Push with failures

Failure logs:

• https://treeherder.mozilla.org/logviewer?job_id=366253389&repo=autoland
• https://treeherder.mozilla.org/logviewer?job_id=366254078&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/267258da3564b461e5e184dc3898aa6135e25e3e

Comment 22

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e00a268f35f3
Unconditionally call TabBarVisbility.update() when adding "multiple" tabs. r=Gijs CLOSED TREE

Comment 23

[Cosmin Sabou [:CosminS]]

The failures in comment 21 may be caused by the changes in Bug 1750070 https://hg.mozilla.org/integration/autoland/rev/f341a77792975d571bc48ebaba38095e7b9ac967 so I relanded this to see if they still happen.

Comment 24

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/e00a268f35f3

Comment 26

[Masatoshi Kimura [:emk]]

Please backout e00a268f35f3 due to regression (bug 1753106).

I found yet another problem. Initially the maximize is disabled in uBO log window (or comment #0 test window), but it is enabled in the restored window.

Comment 27

[Cosmin Sabou [:CosminS]]

Backed out changeset e00a268f35f3 (Bug 1564738) for causing Bug 1753106 as requested by emk.

Backout link: https://hg.mozilla.org/integration/autoland/rev/d5803055f54132f6726d159fead9a926eb9607d3. This will be merged to central later on.

Comment 28

[Cristina Cozmuta (:CrissCozmuta)]

Backout merged to cental https://hg.mozilla.org/mozilla-central/rev/d5803055f54132f6726d159fead9a926eb9607d3

Comment 29

[Masatoshi Kimura [:emk]]

Attachment: Bug 1564738 - Set correct features from the start instead of changing visibity after window creation. r?Gijs

Changing visibiity causes some subtle problems.

Comment 30

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0b374a002803
Set correct features from the start instead of changing visibity after window creation. r=Gijs

Comment 31

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/0b374a002803

Comment 32

[petr.laskevic]

I can still reproduce this in for addon popups in Nightly 98.0a1 as in this bug https://bugzilla.mozilla.org/show_bug.cgi?id=1659844
The window opened with window.open('about:blank?1', '_blank', 'all=yes') and restored with ctrl shift n opens correctly though.