Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for checkbox with dialog.showMessageBox #8590

Merged
merged 2 commits into from
Feb 9, 2017
Merged

Add support for checkbox with dialog.showMessageBox #8590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c8c11e6
Add support for checkbox with dialog.showMessageBox
poiru Feb 6, 2017
7994acf
Use CHROMEGTK_CALLBACK_0 and ignore gpointer data
kevinsawicki Feb 9, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions atom/browser/api/atom_api_dialog.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ void ShowMessageBox(int type,
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon,
atom::NativeWindow* window,
mate::Arguments* args) {
Expand All @@ -56,8 +58,8 @@ void ShowMessageBox(int type,
peek,
&callback)) {
atom::ShowMessageBox(window, (atom::MessageBoxType)type, buttons,
default_id, cancel_id, options, title,
message, detail, icon, callback);
default_id, cancel_id, options, title, message, detail,
checkbox_label, checkbox_checked, icon, callback);
} else {
int chosen = atom::ShowMessageBox(window, (atom::MessageBoxType)type,
buttons, default_id, cancel_id,
Expand Down
15 changes: 6 additions & 9 deletions atom/browser/atom_javascript_dialog_manager.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,14 +38,9 @@ void AtomJavaScriptDialogManager::RunJavaScriptDialog(
}

atom::ShowMessageBox(NativeWindow::FromWebContents(web_contents),
atom::MessageBoxType::MESSAGE_BOX_TYPE_NONE,
buttons,
-1,
0,
atom::MessageBoxOptions::MESSAGE_BOX_NONE,
"",
base::UTF16ToUTF8(message_text),
"",
atom::MessageBoxType::MESSAGE_BOX_TYPE_NONE, buttons, -1,
0, atom::MessageBoxOptions::MESSAGE_BOX_NONE, "",
base::UTF16ToUTF8(message_text), "", "", false,
gfx::ImageSkia(),
base::Bind(&OnMessageBoxCallback, callback));
}
Expand All @@ -66,7 +61,9 @@ void AtomJavaScriptDialogManager::CancelDialogs(

// static
void AtomJavaScriptDialogManager::OnMessageBoxCallback(
const DialogClosedCallback& callback, int code) {
const DialogClosedCallback& callback,
int code,
bool checkbox_checked) {
callback.Run(code == 0, base::string16());
}

Expand Down
3 changes: 2 additions & 1 deletion atom/browser/atom_javascript_dialog_manager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ class AtomJavaScriptDialogManager : public content::JavaScriptDialogManager {

private:
static void OnMessageBoxCallback(const DialogClosedCallback& callback,
int code);
int code,
bool checkbox_checked);
};

} // namespace atom
Expand Down
5 changes: 4 additions & 1 deletion atom/browser/ui/message_box.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ enum MessageBoxOptions {
MESSAGE_BOX_NO_LINK = 1 << 0,
};

typedef base::Callback<void(int code)> MessageBoxCallback;
typedef base::Callback<void(int code, bool checkbox_checked)>
MessageBoxCallback;

int ShowMessageBox(NativeWindow* parent_window,
MessageBoxType type,
Expand All @@ -54,6 +55,8 @@ void ShowMessageBox(NativeWindow* parent_window,
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon,
const MessageBoxCallback& callback);

Expand Down
45 changes: 36 additions & 9 deletions atom/browser/ui/message_box_gtk.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ class GtkMessageBox : public NativeWindowObserver {
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon)
: cancel_id_(cancel_id),
checkbox_checked_(false),
parent_(static_cast<NativeWindowViews*>(parent_window)) {
// Create dialog.
dialog_ = gtk_message_dialog_new(
Expand Down Expand Up @@ -68,6 +71,18 @@ class GtkMessageBox : public NativeWindowObserver {
g_object_unref(pixbuf);
}

if (!checkbox_label.empty()) {
GtkWidget* message_area =
gtk_message_dialog_get_message_area(GTK_MESSAGE_DIALOG(dialog_));
GtkWidget* check_button =
gtk_check_button_new_with_label(checkbox_label.c_str());
g_signal_connect(check_button, "toggled",
G_CALLBACK(OnCheckboxToggledThunk), this);
gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check_button),
checkbox_checked);
gtk_container_add(GTK_CONTAINER(message_area), check_button);
}

// Add buttons.
for (size_t i = 0; i < buttons.size(); ++i) {
GtkWidget* button = gtk_dialog_add_button(
Expand Down Expand Up @@ -154,13 +169,16 @@ class GtkMessageBox : public NativeWindowObserver {
}

CHROMEGTK_CALLBACK_1(GtkMessageBox, void, OnResponseDialog, int);
CHROMEGTK_CALLBACK_0(GtkMessageBox, void, OnCheckboxToggled);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@poiru I tested this out on Ubuntu 16.04 and initially saw this crash when CHROMEGTK_CALLBACK_1 was being used:

Received signal 11 SEGV_ACCERR 000000cf5b00
#0 0x7fd99563fd07 <unknown>
#1 0x7fd98c02e390 <unknown>
#2 0x000000cf5d90 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggled()
#3 0x000000cf5b20 atom::(anonymous namespace)::GtkMessageBox::OnCheckboxToggledThunk()
#4 0x7fd98a387fa5 g_closure_invoke
#5 0x7fd98a399fc1 <unknown>
#6 0x7fd98a3a2d5c g_signal_emit_valist
#7 0x7fd98a3a308f g_signal_emit
#8 0x7fd98bbc7553 <unknown>
#9 0x7fd98a3881d4 <unknown>
#10 0x7fd98a3a29a6 g_signal_emit_valist
#11 0x7fd98a3a308f g_signal_emit
#12 0x000000cf4cd4 atom::(anonymous namespace)::GtkMessageBox::GtkMessageBox()

Switching it to a CHROMEGTK_CALLBACK_0 instead of CHROMEGTK_CALLBACK_1 made it go away and provided the right checkbox value. It looked like the gpointer was unused so it seemed safe to ignore.


private:
atom::UnresponsiveSuppressor unresponsive_suppressor_;

// The id to return when the dialog is closed without pressing buttons.
int cancel_id_;

bool checkbox_checked_;

NativeWindowViews* parent_;
GtkWidget* dialog_;
MessageBoxCallback callback_;
Expand All @@ -172,12 +190,16 @@ void GtkMessageBox::OnResponseDialog(GtkWidget* widget, int response) {
gtk_widget_hide(dialog_);

if (response < 0)
callback_.Run(cancel_id_);
callback_.Run(cancel_id_, checkbox_checked_);
else
callback_.Run(response);
callback_.Run(response, checkbox_checked_);
delete this;
}

void GtkMessageBox::OnCheckboxToggled(GtkWidget* widget) {
checkbox_checked_ = GTK_TOGGLE_BUTTON(widget)->active;
}

} // namespace

int ShowMessageBox(NativeWindow* parent,
Expand All @@ -190,8 +212,9 @@ int ShowMessageBox(NativeWindow* parent,
const std::string& message,
const std::string& detail,
const gfx::ImageSkia& icon) {
return GtkMessageBox(parent, type, buttons, default_id, cancel_id,
title, message, detail, icon).RunSynchronous();
return GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
message, detail, "", false, icon)
.RunSynchronous();
}

void ShowMessageBox(NativeWindow* parent,
Expand All @@ -203,18 +226,22 @@ void ShowMessageBox(NativeWindow* parent,
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon,
const MessageBoxCallback& callback) {
(new GtkMessageBox(parent, type, buttons, default_id, cancel_id,
title, message, detail, icon))->RunAsynchronous(callback);
(new GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
message, detail, checkbox_label, checkbox_checked, icon))
->RunAsynchronous(callback);
}

void ShowErrorBox(const base::string16& title, const base::string16& content) {
if (Browser::Get()->is_ready()) {
GtkMessageBox(nullptr, MESSAGE_BOX_TYPE_ERROR, { "OK" }, -1, 0, "Error",
GtkMessageBox(nullptr, MESSAGE_BOX_TYPE_ERROR, {"OK"}, -1, 0, "Error",
base::UTF16ToUTF8(title).c_str(),
base::UTF16ToUTF8(content).c_str(),
gfx::ImageSkia()).RunSynchronous();
base::UTF16ToUTF8(content).c_str(), "", false,
gfx::ImageSkia())
.RunSynchronous();
} else {
fprintf(stderr,
ANSI_TEXT_BOLD ANSI_BACKGROUND_GRAY
Expand Down
25 changes: 17 additions & 8 deletions atom/browser/ui/message_box_mac.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ - (id)initWithCallback:(const atom::MessageBoxCallback&)callback
- (void)alertDidEnd:(NSAlert*)alert
returnCode:(NSInteger)returnCode
contextInfo:(void*)contextInfo {
callback_.Run(returnCode);
callback_.Run(returnCode, alert.suppressionButton.state == NSOnState);
[alert_ release];
[self release];

Expand All @@ -60,6 +60,8 @@ - (void)alertDidEnd:(NSAlert*)alert
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon) {
// Ignore the title; it's the window title on other platforms and ignorable.
NSAlert* alert = [[NSAlert alloc] init];
Expand Down Expand Up @@ -95,6 +97,12 @@ - (void)alertDidEnd:(NSAlert*)alert
[[ns_buttons objectAtIndex:default_id] setKeyEquivalent:@"\r"];
}

if (!checkbox_label.empty()) {
alert.showsSuppressionButton = YES;
alert.suppressionButton.title = base::SysUTF8ToNSString(checkbox_label);
alert.suppressionButton.state = checkbox_checked ? NSOnState : NSOffState;
}

if (!icon.isNull()) {
NSImage* image = skia::SkBitmapToNSImageWithColorSpace(
*icon.bitmap(), base::mac::GetGenericRGBColorSpace());
Expand All @@ -104,7 +112,7 @@ - (void)alertDidEnd:(NSAlert*)alert
return alert;
}

void SetReturnCode(int* ret_code, int result) {
void SetReturnCode(int* ret_code, int result, bool checkbox_checked) {
*ret_code = result;
}

Expand All @@ -120,9 +128,8 @@ int ShowMessageBox(NativeWindow* parent_window,
const std::string& message,
const std::string& detail,
const gfx::ImageSkia& icon) {
NSAlert* alert = CreateNSAlert(
parent_window, type, buttons, default_id, title, message,
detail, icon);
NSAlert* alert = CreateNSAlert(parent_window, type, buttons, default_id,
title, message, detail, "", false, icon);

// Use runModal for synchronous alert without parent, since we don't have a
// window to wait for.
Expand Down Expand Up @@ -154,11 +161,13 @@ void ShowMessageBox(NativeWindow* parent_window,
const std::string& title,
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& icon,
const MessageBoxCallback& callback) {
NSAlert* alert = CreateNSAlert(
parent_window, type, buttons, default_id, title, message,
detail, icon);
NSAlert* alert =
CreateNSAlert(parent_window, type, buttons, default_id, title, message,
detail, checkbox_label, checkbox_checked, icon);
ModalDelegate* delegate = [[ModalDelegate alloc] initWithCallback:callback
andAlert:alert
callEndModal:false];
Expand Down
Loading