1634766 - [macos] [osx] Window > Zoom menu item should fit the window to the screen even when it is already maximized (but moved elsewhere so it doesn't fit the screen)

Status: RESOLVED FIXED

(Core :: Widget: Cocoa, defect)

Description

[Michael Gauthier [:gauthierm]]

On macOS this menu item is supposed to expand the window to best fit the window content or fill the screen.

The green zoom window stoplight icon on the top left of the window performs the zoom action correctly, but the menu item Window > Zoom does nothing.

Comment 1

[:Gijs (he/him)]

The item does something for me (switches between maximized and non-maximized; not the same as the green button which fullscreens / exits fullscreen for me; tested on macOS 10.15).

Are you testing on release or Nightly? Do you see the same thing on a clean profile? Does it matter what page is visible in the browser, or whether the browser already takes up the entire screen or less space? What version of macOS are you using?

Comment 2

[Mike Conley (:mconley) (:⚙️)]

We haven't heard back from the reporter in 20 days, so closing this out as INCOMPLETE. Feel free to re-open once the information comes in.

Comment 3

[Michael Gauthier [:gauthierm]]

The green button on macOS has multiple modes. The default mode is fullscreen/restore. If you hover over the green button it will eventually show a menu with other window management items. The zoom action is an option in this menu and is also triggered by using Option+Click.

For me, the Zoom action from the green button works, but the Window->Zoom menu item does not do anything.

I'm using macOS Catalina and Firefox 79.0b5 (64-bit)

Comment 4

[:Gijs (he/him)]

(In reply to Michael Gauthier [:gauthierm] from comment #3)

The green button on macOS has multiple modes. The default mode is fullscreen/restore. If you hover over the green button it will eventually show a menu with other window management items. The zoom action is an option in this menu and is also triggered by using Option+Click.

For me, the Zoom action from the green button works, but the Window->Zoom menu item does not do anything.

I'm using macOS Catalina and Firefox 79.0b5 (64-bit)

So I can't reproduce - the Window -> Zoom item has the same effect (restore to a smaller size if the window is maximized, and then restoring to the maximum size when clicked again).

Do you see this problem on a clean profile ? If you first make the window smaller by dragging to resize, and then use Window -> Zoom, does the window not grow?

Comment 5

[:Gijs (he/him)]

Oh, and are there any errors in the browser console (cmd-shift-j to open; this is different from the regular devtools console)

Comment 6

[Michael Gauthier [:gauthierm]]

Ok, I have more info for this bug. The Window->Zoom menu item does nothing in Firefox if the window is already at the correct size. The macOS window management controls move the window to the top of the screen even if it's already the correct size.

I've checked other applications in macOS and they also reposition the window when using Window->Zoom even if it is already the correct size. The zoom behaviour should be consistent when triggered from the Window->Zoom action or when triggered from the green button zoom action (Option+Click).

Comment 7

[Michael Gauthier [:gauthierm]]

I checked the browser console (Cmd+Shift+J) and do not see any errors using Window->Zoom.

Comment 8

[:Gijs (he/him)]

Thanks, that really helps! So STR:

1. maximize / zoom a browser window
2. drag the window to e.g. move it down the screen
3. click window -> zoom

ER:

it moves back to fill the screen

AR:

it does not.

I think the bug here is at https://searchfox.org/mozilla-central/rev/828f2319c0195d7f561ed35533aef6fe183e68e3/toolkit/content/macWindowMenu.js#37 which only checks the window state before deciding whether to restore/maximize - and the window state is "maximized", and so it tries calling window.restore which no-ops.

Haik, is there currently a straightforward way for frontend or cocoa code to fix this?

Comment 9

[Michael Gauthier [:gauthierm]]

Your summarized bug report captures the issue perfectly.

Comment 10

[Haik Aftandilian [:haik]]

(In reply to :Gijs (back Aug 3rd; he/him) from comment #8)

Haik, is there currently a straightforward way for frontend or cocoa code to fix this?

It sounds like we need to reset the window.windowState to window.STATE_NORMAL when the window is moved.

I don't know if that needs to happen in the front end or in cocoa platform code, but I'll look into this more. Leaving the needinfo.

Comment 11

[:Gijs (he/him)]

(In reply to Haik Aftandilian [:haik] from comment #10)

(In reply to :Gijs (back Aug 3rd; he/him) from comment #8)

Haik, is there currently a straightforward way for frontend or cocoa code to fix this?

It sounds like we need to reset the window.windowState to window.STATE_NORMAL when the window is moved.

I don't know if that needs to happen in the front end or in cocoa platform code, but I'll look into this more. Leaving the needinfo.

AFAICT window.windowState is implemented in nsGlobalWindowInner, https://searchfox.org/mozilla-central/source/dom/base/nsGlobalWindowInner.cpp#6626 , which delegates to widget->SizeMode() which is implemented in the nsIBaseWidget code, which simply returns mSizeMode, which is updated by the various platform-specific widget implementations.

I imagine that what needs to happen is for the windowDidMove notification in the cocoa window widget, to check if mSizeMode is set to "maximized", and if so we should do the same as we do for resizes, cf. https://searchfox.org/mozilla-central/rev/72a1334982cadde0ca8b3d3583877afea80f5639/widget/cocoa/nsCocoaWindow.mm#2566-2568 and notify a sizemode change.

I don't have a mac compiled build right now so I can't quickly check if this fixes things and is sufficient. Maybe later this week if you don't beat me to it...

Comment 12

[:Gijs (he/him)]

Attachment: Bug 1634766 - macos: moving a window to/from maximized/zoomed state should update the window's sizemode, r?haik

When a window is maximized/zoomed, and then moved, we should stop treating it
as maximized/zoomed. Likewise, if the window is moved back to being zoomed/
maximized, we should also update our bookkeeping. Updating the internal
mSizemode will fire the right events and cause window.windowState to return
the correct result when JS interrogates it.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69f169186494
macos: moving a window to/from maximized/zoomed state should update the window's sizemode, r=spohl

Comment 14

[Haik Aftandilian [:haik]]

Thanks for taking this.

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/69f169186494

Comment 16

[:Gijs (he/him)]

Reporter, would you mind checking with a nightly build if the behaviour is now fixed, as far as you can tell? Thanks!

Comment 17

[Michael Gauthier [:gauthierm]]

The nightly build is working fantastically!