Browse Source

fix: unify `BrowserWindow.isVisible()` logic cross-platform (#38242)

Shelley Vohr 1 year ago
parent
commit
e83197669c

+ 1 - 1
docs/api/browser-window.md

@@ -667,7 +667,7 @@ Hides the window.
 
 #### `win.isVisible()`
 
-Returns `boolean` - Whether the window is visible to the user.
+Returns `boolean` - Whether the window is visible to the user in the foreground of the app.
 
 #### `win.isModal()`
 

+ 0 - 4
shell/browser/native_window_mac.mm

@@ -491,10 +491,6 @@ void NativeWindowMac::Hide() {
 
 bool NativeWindowMac::IsVisible() {
   bool occluded = [window_ occlusionState] == NSWindowOcclusionStateVisible;
-
-  // For a window to be visible, it must be visible to the user in the
-  // foreground of the app, which means that it should not be minimized or
-  // occluded
   return [window_ isVisible] && !occluded && !IsMinimized();
 }
 

+ 10 - 0
shell/browser/native_window_views.cc

@@ -551,7 +551,17 @@ void NativeWindowViews::Hide() {
 }
 
 bool NativeWindowViews::IsVisible() {
+#if BUILDFLAG(IS_WIN)
+  // widget()->IsVisible() calls ::IsWindowVisible, which returns non-zero if a
+  // window or any of its parent windows are visible. We want to only check the
+  // current window.
+  bool visible =
+      ::GetWindowLong(GetAcceleratedWidget(), GWL_STYLE) & WS_VISIBLE;
+  // WS_VISIBLE is true even if a window is miminized - explicitly check that.
+  return visible && !IsMinimized();
+#else
   return widget()->IsVisible();
+#endif
 }
 
 bool NativeWindowViews::IsEnabled() {

+ 12 - 0
spec/api-browser-window-spec.ts

@@ -1095,6 +1095,18 @@ describe('BrowserWindow module', () => {
       });
     });
 
+    describe('BrowserWindow.minimize()', () => {
+      // TODO(codebytere): Enable for Linux once maximize/minimize events work in CI.
+      ifit(process.platform !== 'linux')('should not be visible when the window is minimized', async () => {
+        const minimize = once(w, 'minimize');
+        w.minimize();
+        await minimize;
+
+        expect(w.isMinimized()).to.equal(true);
+        expect(w.isVisible()).to.equal(false);
+      });
+    });
+
     describe('BrowserWindow.showInactive()', () => {
       it('should not focus on window', () => {
         w.showInactive();