fix: crash when calling moveTop() on modal children

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
electron · Aug 16, 2023 · 940096b · 940096b
1 parent ec4c609
commit 940096b
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 3 deletions.
diff --git a/shell/browser/native_window_mac.mm b/shell/browser/native_window_mac.mm
@@ -815,7 +815,7 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
   if (!webrtc::GetWindowOwnerPid(window_id))
     return false;
 
-  if (!parent()) {
+  if (!parent() || is_modal()) {
     [window_ orderWindow:NSWindowAbove relativeTo:window_id];
   } else {
     NSWindow* other_window = [NSApp windowWithWindowNumber:window_id];
@@ -826,10 +826,11 @@ void ReorderChildWindowAbove(NSWindow* child_window, NSWindow* other_window) {
 }
 
 void NativeWindowMac::MoveTop() {
-  if (!parent())
+  if (!parent() || is_modal()) {
     [window_ orderWindow:NSWindowAbove relativeTo:0];
-  else
+  } else {
     ReorderChildWindowAbove(window_, nullptr);
+  }
 }
 
 void NativeWindowMac::SetResizable(bool resizable) {

diff --git a/spec/api-browser-window-spec.ts b/spec/api-browser-window-spec.ts
@@ -921,6 +921,8 @@ describe('BrowserWindow module', () => {
     });
 
     describe('BrowserWindow.moveTop()', () => {
+      afterEach(closeAllWindows);
+
       it('should not steal focus', async () => {
         const posDelta = 50;
         const wShownInactive = emittedOnce(w, 'show');
@@ -962,6 +964,15 @@ describe('BrowserWindow module', () => {
         await closeWindow(otherWindow, { assertNotWindows: false });
         expect(BrowserWindow.getAllWindows()).to.have.lengthOf(1);
       });
+
+      it('should not crash when called on a modal child window', async () => {
+        const shown = once(w, 'show');
+        w.show();
+        await shown;
+
+        const child = new BrowserWindow({ modal: true, parent: w });
+        expect(() => { child.moveTop(); }).to.not.throw();
+      });
     });
 
     ifdescribe(features.isDesktopCapturerEnabled())('BrowserWindow.moveAbove(mediaSourceId)', () => {