From 1c7de7e5ee3b2958b1d74f9110bb6b9255e95b6e Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Mon, 28 Aug 2023 11:05:29 +0900 Subject: [PATCH] fix: ensure windows respect fullscreenability with different resizability values (#39642) * fix: ensure child windows respect fullscreenability/resizability when parent is fullscreen Co-authored-by: Shelley Vohr * test: add an extra resize test Co-authored-by: Shelley Vohr --------- Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Shelley Vohr --- shell/browser/native_window_mac.mm | 24 +++++++++----------- spec/api-browser-window-spec.ts | 36 ++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 14 deletions(-) diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm index 87d6e5729e673..e12827ed50767 100644 --- a/shell/browser/native_window_mac.mm +++ b/shell/browser/native_window_mac.mm @@ -837,23 +837,19 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) { ScopedDisableResize disable_resize; SetStyleMask(resizable, NSWindowStyleMaskResizable); + bool was_fullscreenable = IsFullScreenable(); + // Right now, resizable and fullscreenable are decoupled in // documentation and on Windows/Linux. Chromium disables - // fullscreenability if resizability is false on macOS as well - // as disabling the maximize traffic light unless the window - // is both resizable and maximizable. To work around this, we want - // to match behavior on other platforms by disabiliting the maximize - // button but keeping fullscreenability enabled. - // TODO(codebytere): refactor this once we have a better solution. + // fullscreen collection behavior as well as the maximize traffic + // light in SetCanResize if resizability is false on macOS unless + // the window is both resizable and maximizable. We want consistent + // cross-platform behavior, so if resizability is disabled we disable + // the maximize button and ensure fullscreenability matches user setting. SetCanResize(resizable); - if (!resizable) { - SetFullScreenable(true); - [[window_ standardWindowButton:NSWindowZoomButton] setEnabled:false]; - } else { - SetFullScreenable(true); - [[window_ standardWindowButton:NSWindowZoomButton] - setEnabled:IsFullScreenable()]; - } + SetFullScreenable(was_fullscreenable); + [[window_ standardWindowButton:NSWindowZoomButton] + setEnabled:resizable ? was_fullscreenable : false]; } bool NativeWindowMac::IsResizable() { diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts index 70c63002762b1..9c7d8230cad8c 100644 --- a/spec/api-browser-window-spec.ts +++ b/spec/api-browser-window-spec.ts @@ -5450,6 +5450,42 @@ describe('BrowserWindow module', () => { expect(w.isFullScreenable()).to.be.true('isFullScreenable'); }); }); + + it('does not open non-fullscreenable child windows in fullscreen if parent is fullscreen', async () => { + const w = new BrowserWindow(); + + const enterFS = once(w, 'enter-full-screen'); + w.setFullScreen(true); + await enterFS; + + const child = new BrowserWindow({ parent: w, resizable: false, fullscreenable: false }); + const shown = once(child, 'show'); + await shown; + + expect(child.resizable).to.be.false('resizable'); + expect(child.fullScreen).to.be.false('fullscreen'); + expect(child.fullScreenable).to.be.false('fullscreenable'); + }); + + it('is set correctly with different resizable values', async () => { + const w1 = new BrowserWindow({ + resizable: false, + fullscreenable: false + }); + + const w2 = new BrowserWindow({ + resizable: true, + fullscreenable: false + }); + + const w3 = new BrowserWindow({ + fullscreenable: false + }); + + expect(w1.isFullScreenable()).to.be.false('isFullScreenable'); + expect(w2.isFullScreenable()).to.be.false('isFullScreenable'); + expect(w3.isFullScreenable()).to.be.false('isFullScreenable'); + }); }); ifdescribe(process.platform === 'darwin')('isHiddenInMissionControl state', () => {