From 5f365858c931cc4e0b021f8aaecca51ceeb22e68 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Tue, 26 Nov 2019 23:39:07 -0800 Subject: [PATCH] fix: correctly plumb checkboxChecked on win (#21248) * fix: correctly plumb checkboxChecked on win * address final style comment --- lib/browser/api/dialog.js | 3 ++ shell/browser/ui/message_box.h | 3 ++ shell/browser/ui/message_box_win.cc | 56 ++++++++++++++--------------- 3 files changed, 34 insertions(+), 28 deletions(-) diff --git a/lib/browser/api/dialog.js b/lib/browser/api/dialog.js index fbe864528b89..e6ab30718343 100644 --- a/lib/browser/api/dialog.js +++ b/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) { diff --git a/shell/browser/ui/message_box.h b/shell/browser/ui/message_box.h index 514fd7704b88..d976b293226b 100644 --- a/shell/browser/ui/message_box.h +++ b/shell/browser/ui/message_box.h @@ -6,6 +6,7 @@ #define SHELL_BROWSER_UI_MESSAGE_BOX_H_ #include +#include #include #include "base/callback_forward.h" @@ -29,6 +30,8 @@ enum MessageBoxOptions { MESSAGE_BOX_NO_LINK = 1 << 0, }; +using DialogResult = std::pair; + struct MessageBoxSettings { electron::NativeWindow* parent_window = nullptr; MessageBoxType type = electron::MessageBoxType::kNone; diff --git a/shell/browser/ui/message_box_win.cc b/shell/browser/ui/message_box_win.cc index 208bf0b22d93..205b64238a3a 100644 --- a/shell/browser/ui/message_box_win.cc +++ b/shell/browser/ui/message_box_win.cc @@ -78,18 +78,18 @@ void MapToCommonID(const std::vector& buttons, } } -int ShowTaskDialogUTF16(NativeWindow* parent, - MessageBoxType type, - const std::vector& 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& 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 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