Browse Source

fix: crash when `dialog.showMessageBoxSync` with missing buttons (#40996)

* fix: crash when dialog.showMessageBoxSync missing buttons

* chore: feedback from review
Shelley Vohr 1 year ago
parent
commit
7e6fb97a2f
2 changed files with 25 additions and 8 deletions
  1. 9 8
      shell/browser/ui/message_box_mac.mm
  2. 16 0
      spec/api-dialog-spec.ts

+ 9 - 8
shell/browser/ui/message_box_mac.mm

@@ -76,15 +76,16 @@ NSAlert* CreateNSAlert(const MessageBoxSettings& settings) {
     [[ns_buttons objectAtIndex:settings.default_id] setKeyEquivalent:@"\r"];
   }
 
-  // Bind cancel id button to escape key if there is more than one button
-  if (button_count > 1 && settings.cancel_id >= 0 &&
-      settings.cancel_id < button_count) {
-    [[ns_buttons objectAtIndex:settings.cancel_id] setKeyEquivalent:@"\e"];
-  }
+  if (button_count > 1 && settings.cancel_id >= 0) {
+    // Bind cancel id button to escape key if there is more than one button.
+    if (settings.cancel_id < button_count) {
+      [[ns_buttons objectAtIndex:settings.cancel_id] setKeyEquivalent:@"\e"];
+    }
 
-  // TODO(@codebytere): This behavior violates HIG & should be deprecated.
-  if (settings.cancel_id >= 0 && settings.cancel_id == settings.default_id) {
-    [[ns_buttons objectAtIndex:settings.default_id] highlight:YES];
+    // TODO(@codebytere): This behavior violates HIG & should be deprecated.
+    if (settings.cancel_id == settings.default_id) {
+      [[ns_buttons objectAtIndex:settings.default_id] highlight:YES];
+    }
   }
 
   if (!settings.checkbox_label.empty()) {

+ 16 - 0
spec/api-dialog-spec.ts

@@ -146,6 +146,22 @@ describe('dialog module', () => {
       expect(result.response).to.equal(0);
     });
 
+    it('does not crash when there is a defaultId but no buttons', async () => {
+      const controller = new AbortController();
+      const signal = controller.signal;
+      const w = new BrowserWindow();
+      const p = dialog.showMessageBox(w, {
+        signal,
+        message: 'i am message',
+        type: 'info',
+        defaultId: 0,
+        title: 'i am title'
+      });
+      controller.abort();
+      const result = await p;
+      expect(result.response).to.equal(0);
+    });
+
     it('cancels message box', async () => {
       const controller = new AbortController();
       const signal = controller.signal;