Browse Source

fix: correctly plumb checkboxChecked on win (#21312)

* fix: correctly plumb checkboxChecked on win

* address final style comment
trop[bot] 5 years ago
parent
commit
2bd83d0e89
3 changed files with 34 additions and 28 deletions
  1. 3 0
      lib/browser/api/dialog.js
  2. 3 0
      shell/browser/ui/message_box.h
  3. 28 28
      shell/browser/ui/message_box_win.cc

+ 3 - 0
lib/browser/api/dialog.js

@@ -178,6 +178,9 @@ const messageBox = (sync, window, options) => {
   if (typeof checkboxLabel !== 'string') throw new TypeError('checkboxLabel must be a string')
 
   checkboxChecked = !!checkboxChecked
+  if (checkboxChecked && !checkboxLabel) {
+    throw new Error('checkboxChecked requires that checkboxLabel also be passed')
+  }
 
   // Choose a default button to get selected when dialog is cancelled.
   if (cancelId == null) {

+ 3 - 0
shell/browser/ui/message_box.h

@@ -6,6 +6,7 @@
 #define SHELL_BROWSER_UI_MESSAGE_BOX_H_
 
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/callback_forward.h"
@@ -29,6 +30,8 @@ enum MessageBoxOptions {
   MESSAGE_BOX_NO_LINK = 1 << 0,
 };
 
+using DialogResult = std::pair<int, bool>;
+
 struct MessageBoxSettings {
   electron::NativeWindow* parent_window = nullptr;
   MessageBoxType type = electron::MessageBoxType::kNone;

+ 28 - 28
shell/browser/ui/message_box_win.cc

@@ -78,18 +78,18 @@ void MapToCommonID(const std::vector<base::string16>& buttons,
   }
 }
 
-int ShowTaskDialogUTF16(NativeWindow* parent,
-                        MessageBoxType type,
-                        const std::vector<base::string16>& buttons,
-                        int default_id,
-                        int cancel_id,
-                        int options,
-                        const base::string16& title,
-                        const base::string16& message,
-                        const base::string16& detail,
-                        const base::string16& checkbox_label,
-                        bool* checkbox_checked,
-                        const gfx::ImageSkia& icon) {
+DialogResult ShowTaskDialogUTF16(NativeWindow* parent,
+                                 MessageBoxType type,
+                                 const std::vector<base::string16>& buttons,
+                                 int default_id,
+                                 int cancel_id,
+                                 int options,
+                                 const base::string16& title,
+                                 const base::string16& message,
+                                 const base::string16& detail,
+                                 const base::string16& checkbox_label,
+                                 bool checkbox_checked,
+                                 const gfx::ImageSkia& icon) {
   TASKDIALOG_FLAGS flags =
       TDF_SIZE_TO_CONTENT |           // Show all content.
       TDF_ALLOW_DIALOG_CANCELLATION;  // Allow canceling the dialog.
@@ -148,10 +148,8 @@ int ShowTaskDialogUTF16(NativeWindow* parent,
 
   if (!checkbox_label.empty()) {
     config.pszVerificationText = checkbox_label.c_str();
-
-    if (checkbox_checked && *checkbox_checked) {
+    if (checkbox_checked)
       config.dwFlags |= TDF_VERIFICATION_FLAG_CHECKED;
-    }
   }
 
   // Iterate through the buttons, put common buttons in dwCommonButtons
@@ -172,22 +170,24 @@ int ShowTaskDialogUTF16(NativeWindow* parent,
       config.dwFlags |= TDF_USE_COMMAND_LINKS;  // custom buttons as links.
   }
 
+  int button_id;
+
   int id = 0;
   BOOL verificationFlagChecked = FALSE;
   TaskDialogIndirect(&config, &id, nullptr, &verificationFlagChecked);
-  if (checkbox_checked) {
-    *checkbox_checked = verificationFlagChecked;
-  }
 
   if (id_map.find(id) != id_map.end())  // common button.
-    return id_map[id];
+    button_id = id_map[id];
   else if (id >= kIDStart)  // custom button.
-    return id - kIDStart;
+    button_id = id - kIDStart;
   else
-    return cancel_id;
+    button_id = cancel_id;
+
+  return std::make_pair(button_id,
+                        checkbox_checked ? verificationFlagChecked : false);
 }
 
-int ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
+DialogResult ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
   std::vector<base::string16> utf16_buttons;
   for (const auto& button : settings.buttons)
     utf16_buttons.push_back(base::UTF8ToUTF16(button));
@@ -197,21 +197,20 @@ int ShowTaskDialogUTF8(const MessageBoxSettings& settings) {
   const base::string16 detail_16 = base::UTF8ToUTF16(settings.detail);
   const base::string16 checkbox_label_16 =
       base::UTF8ToUTF16(settings.checkbox_label);
-  bool cb_checked = settings.checkbox_checked;
 
   return ShowTaskDialogUTF16(
       settings.parent_window, settings.type, utf16_buttons, settings.default_id,
       settings.cancel_id, settings.options, title_16, message_16, detail_16,
-      checkbox_label_16, &cb_checked, settings.icon);
+      checkbox_label_16, settings.checkbox_checked, settings.icon);
 }
 
 void RunMessageBoxInNewThread(base::Thread* thread,
                               const MessageBoxSettings& settings,
                               MessageBoxCallback callback) {
-  int result = ShowTaskDialogUTF8(settings);
+  DialogResult result = ShowTaskDialogUTF8(settings);
   base::PostTask(
       FROM_HERE, {content::BrowserThread::UI},
-      base::BindOnce(std::move(callback), result, settings.checkbox_checked));
+      base::BindOnce(std::move(callback), result.first, result.second));
   content::BrowserThread::DeleteSoon(content::BrowserThread::UI, FROM_HERE,
                                      thread);
 }
@@ -220,7 +219,8 @@ void RunMessageBoxInNewThread(base::Thread* thread,
 
 int ShowMessageBoxSync(const MessageBoxSettings& settings) {
   electron::UnresponsiveSuppressor suppressor;
-  return ShowTaskDialogUTF8(settings);
+  DialogResult result = ShowTaskDialogUTF8(settings);
+  return result.first;
 }
 
 void ShowMessageBox(const MessageBoxSettings& settings,
@@ -243,7 +243,7 @@ void ShowMessageBox(const MessageBoxSettings& settings,
 void ShowErrorBox(const base::string16& title, const base::string16& content) {
   electron::UnresponsiveSuppressor suppressor;
   ShowTaskDialogUTF16(nullptr, MessageBoxType::kError, {}, -1, 0, 0, L"Error",
-                      title, content, L"", nullptr, gfx::ImageSkia());
+                      title, content, L"", false, gfx::ImageSkia());
 }
 
 }  // namespace electron