1854820 - browser.windows.update() incorrectly shrinks height of window

Status: VERIFIED FIXED

(WebExtensions :: Frontend, defect, P3)

Description

[Alex Eng]

Attachment: Panic Button - Restored window height shrunk - macOS.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/117.0

Steps to reproduce:

The Panic Button extension (current version 4.4.4) uses the browser extension API browser.windows.update() to restore the user's browser windows from their minimized state. Since Firefox 115, this API call is causing the browser window height to be incorrectly shrunk down, instead of restoring the original height as set by the user.

The Panic Button extension is available here: https://addons.mozilla.org/firefox/addon/panic-button/

This issue is reproducible in Firefox 117.0.1 on macOS and Linux; not reproducible on Windows. This issue is not reproducible on Firefox ESR 102.15.1. I am using macOS Monterey 12.6.9 and Ubuntu 20.04.6

Link to Panic Button source code where browser.windows.update() is called to restore the minimized browser window(s): https://github.com/aecreations/panicbutton/blob/3eb3786764dc2e45d77282cb15070c49c19dccd2/wx-src/background.js#L62

Steps to reproduce:

1. Install Panic Button.
2. Open the extension preferences page and change the Panic Button action to "Minimize All Windows"
3. Turn on the option to "Open a new window after all browser windows are minimized"
4. Invoke the Panic Button action in the browser by clicking the toolbar button or by pressing F8
5. When the temporary replacement browser window appears, repeat step 4 to close it and restore the minimized browser window(s)

Actual results:

At first, the browser window is restored to its original size, but is then immediately resized (this occurs very quickly, in less than a second); the window height is shrunk down, showing only the browser tab bar and the address bar. See attached screen shot.

Expected results:

The browser window should be restored to its original size, and should remain at those dimensions.

Comment 1

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

The Bugbug bot thinks this bug should belong to the 'WebExtensions::Untriaged' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[Alex Cornestean]

Hello,

Apparently, I cannot reproduce the issue on my end on the latest Release (117.0.1/20230912013654) or Nightly (119.0a1/20230924092410) under either Windows 10, Ubuntu 22.04 LTS and macOS 11.3.1.

Following the STR, the first click on the Panic Button will minimize the original window and create a new window. Clicking the Panic Button on that new window will close it and maximize the original one, without afterwards shrinking it.

The only issue I faced is on Linux where upon closing the second window via the Panic Button, the original window is not restored. It stays minimized.

Comment 3

[Alex Eng]

Try resizing the browser window so that it is not maximized (or "zoomed" in macOS terminology). I found that if the browser window is maximized and I follow the steps to reproduce, the issue doesn't occur.

Comment 4

[Alex Cornestean]

Hello Alex and thank you for the tip !

Indeed the window needs to not be maximized when performing the STR and then the issue can be reproduced.

Reproduced on the latest Release (117.0.1/20230912013654) and Nightly (120.0a1/20230925160932) under macOS 11.3.1.

Comment 5

[Luca Greco [:rpl] [:luca] [:lgreco]]

Hey Alex, given that it seems you are now able to reproduce the issue consistently, would you mind to try to see if the regressing change can be bisect using mozregression?

Comment 6

[Alex Eng]

Ran mozregression against Firefox on macOS 12.7 Monterey. Results:

2023-10-02T23:00:20.610000: DEBUG : Found commit message:
Bug 1631735 Part 4: Make tests that minimize windows wait for size mode change events. r=mstange

Depends on D170841

Differential Revision: https://phabricator.services.mozilla.com/D171626

Comment 7

[Alex Cornestean]

Thank you, Alex, for performing the bisection and finding the regressing bug. I’ll clear the flag and set Bug 1631735 as the regressor.

Comment 8

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

Set release status flags based on info from the regressing bug 1631735

:bradwerth, since you are the author of the regressor, bug 1631735, could you take a look? Also, could you set the severity field?

For more information, please visit BugBot documentation.

Comment 9

[Brad Werth [:bradwerth]]

I'll figure this out.

Comment 10

[Brad Werth [:bradwerth]]

Looks like the issue is that window.update triggers setState({state: "normal"}) which then hits a series of calls to window.restore() followed by a call to window.sizeToContent(). The size to content happens in nsGlobalWindowOuter::SizeToContentOuter. None of this was changed in Bug 1631735. What changed in that Bug is that the window transition back from minimization is done asynchronously, so script has the opportunity to make more calls while the transition is still happening. It's an issue of changed timing, in other words. The minimized window is tiny, so resizing to its content is choosing the smallest possible size that will fit the content.

Perhaps the "hack" call to sizeToContent is really what needs to be removed. I doubt we have good test coverage here, so I'm going to look back at the history of that code and what case prompted its addition. It's possible that that motivating case is already solved by Bug 1631735 and the sizeToContent is just causing this new problem.

Comment 11

[Brad Werth [:bradwerth]]

Attachment: Bug 1854820: Make the transition to window normal state work the same on extension windows as it does elsewhere.

Calling window.restore will return the window to its previous
dimensions. Prior to this patch, extension windows transitioning to
normal state would add a second step where they would forcibly resize
the window to content. The motivating reason for this additional step is
some kind of difficulty with macOS windows restoring from other states.
Those issues were handled comprehensively in Bug 1631735, and now the
extra step is unnecessary and surprising to users. Since the call was
being made potentially during a deminimization native transition, the
window is at a very small size and expanding it to fit the content is
resizing it to a strange small size. Window.restore will do the right
thing, so this patch stops calling sizeToContent.

Comment 12

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

Set release status flags based on info from the regressing bug 1631735

Comment 13

[Dianna Smith [:diannaS]]

:bradwerth any update here for the 120 or 121 cycle? is this just waiting on a review?

Comment 14

[Brad Werth [:bradwerth]]

(In reply to Dianna Smith [:diannaS] from comment #13)

:bradwerth any update here for the 120 or 121 cycle? is this just waiting on a review?

It has just now been reviewed and I'll make the requested changes and land this.

Comment 15

[Brad Werth [:bradwerth]]

Attachment: Bug 1854820 Part 2: Update an extension window test to check window sizes.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e61a1f47853c
Make the transition to window normal state work the same on extension windows as it does elsewhere. r=extension-reviewers,robwu
https://hg.mozilla.org/integration/autoland/rev/caeb06504910
Part 2: Update an extension window test to check window sizes. r=extension-reviewers,robwu

Comment 17

[Serban Stanca [:SerbanS]]

Backed out for causing browser-chrome failures in browser_ext_windows_update.js.

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | browser/components/extensions/test/browser/browser_ext_windows_update.js | Got expected value for window.width - Expected: 1280, Actual: 1863 -

Comment 18

[Brad Werth [:bradwerth]]

I should be able to fix the issues with the test.

Comment 19

[Brad Werth [:bradwerth]]

Looks like the test is failing on try because it's checking the "normal" window's size when it first comes back from minimization, when it is still zoomed. This can be fixed by making the window go to "normal" before it gets minimized. I'll do that, and the re-land.

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/16675e8622b0
Make the transition to window normal state work the same on extension windows as it does elsewhere. r=extension-reviewers,robwu
https://hg.mozilla.org/integration/autoland/rev/fba4a34242f8
Part 2: Update an extension window test to check window sizes. r=extension-reviewers,robwu

Comment 21

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/16675e8622b0
https://hg.mozilla.org/mozilla-central/rev/fba4a34242f8

Comment 22

[Alex Cornestean]

Verified as Fixed. Tested on the latest Nightly (121.0a1/20231119091854) under macOS 11.3.1.

Following the original STR, the issue is no longer reproducible. The browser window is now restored to its original size.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for ESR115 approval.

Comment 24

[Brad Werth [:bradwerth]]

Comment on attachment 9356463 [details]
Bug 1854820: Make the transition to window normal state work the same on extension windows as it does elsewhere.

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: This makes extension windows behave the same as Firefox windows in regards to de-minimization.
• User impact if declined: Minimized extension windows will be shrunk to unusable sizes when de-minimized.
• Fix Landed on Version: 121
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Removes a small amount of unneeded code; it would be challenging for this to create new problems.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9356463 [details]
Bug 1854820: Make the transition to window normal state work the same on extension windows as it does elsewhere.

Approved for 115.6esr.

Comment 26

[Pulsebot]

https://hg.mozilla.org/releases/mozilla-esr115/rev/6327ae0fdd21
https://hg.mozilla.org/releases/mozilla-esr115/rev/aa9f02961b2b