Skip to content

Commit

Permalink
fix: honor dialog.showMessageBox()'s Icon argument on Linux (#15326)
Browse files Browse the repository at this point in the history
* Don't call gtk_widget_show_all() on popup dialog.

Fixes #15317.

Notes: Fixed incorrect display of some GtkMessageDialog icons.

The issue is caused because GtkMessageDialog contains an icon widget
which is not shown when there's no associated icon. Our call to
`gtk_widget_show_all()` overrides this, showing the uninitialized
icon widget.

This PR fixes the issue by calling `gtk_widget_show()` where needed
and removing use of `gtk_widget_show_all()` in the message dialog.

* use gtk_dialog_set_default_response() for default

* fix: support icons on gtk+ messageboxes.
  • Loading branch information
ckerr committed Oct 23, 2018
1 parent 260778e commit 869e0ab
Showing 1 changed file with 28 additions and 11 deletions.
39 changes: 28 additions & 11 deletions atom/browser/ui/message_box_gtk.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ class GtkMessageBox : public NativeWindowObserver {
const std::string& message,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked)
bool checkbox_checked,
const gfx::ImageSkia& icon)
: cancel_id_(cancel_id),
parent_(static_cast<NativeWindow*>(parent_window)) {
// Create dialog.
Expand All @@ -56,6 +57,21 @@ class GtkMessageBox : public NativeWindowObserver {
if (!title.empty())
gtk_window_set_title(GTK_WINDOW(dialog_), title.c_str());

if (!icon.isNull()) {
// No easy way to obtain this programmatically, but GTK+'s docs
// define GTK_ICON_SIZE_DIALOG to be 48 pixels
static constexpr int pixel_width = 48;
static constexpr int pixel_height = 48;
GdkPixbuf* pixbuf = libgtkui::GdkPixbufFromSkBitmap(*icon.bitmap());
GdkPixbuf* scaled_pixbuf = gdk_pixbuf_scale_simple(
pixbuf, pixel_width, pixel_height, GDK_INTERP_BILINEAR);
GtkWidget* w = gtk_image_new_from_pixbuf(scaled_pixbuf);
gtk_message_dialog_set_image(GTK_MESSAGE_DIALOG(dialog_), w);
gtk_widget_show(w);
g_clear_pointer(&scaled_pixbuf, gdk_pixbuf_unref);
g_clear_pointer(&pixbuf, gdk_pixbuf_unref);
}

if (!checkbox_label.empty()) {
GtkWidget* message_area =
gtk_message_dialog_get_message_area(GTK_MESSAGE_DIALOG(dialog_));
Expand All @@ -66,15 +82,15 @@ class GtkMessageBox : public NativeWindowObserver {
gtk_toggle_button_set_active(GTK_TOGGLE_BUTTON(check_button),
checkbox_checked);
gtk_container_add(GTK_CONTAINER(message_area), check_button);
gtk_widget_show(check_button);
}

// Add buttons.
GtkDialog* dialog = GTK_DIALOG(dialog_);
for (size_t i = 0; i < buttons.size(); ++i) {
GtkWidget* button = gtk_dialog_add_button(
GTK_DIALOG(dialog_), TranslateToStock(i, buttons[i]), i);
if (static_cast<int>(i) == default_id)
gtk_widget_grab_focus(button);
gtk_dialog_add_button(dialog, TranslateToStock(i, buttons[i]), i);
}
gtk_dialog_set_default_response(dialog, default_id);

// Parent window.
if (parent_) {
Expand Down Expand Up @@ -122,7 +138,7 @@ class GtkMessageBox : public NativeWindowObserver {
}

void Show() {
gtk_widget_show_all(dialog_);
gtk_widget_show(dialog_);
// We need to call gtk_window_present after making the widgets visible to
// make sure window gets correctly raised and gets focus.
int time = ui::X11EventSource::GetInstance()->GetTimestamp();
Expand Down Expand Up @@ -195,9 +211,9 @@ int ShowMessageBox(NativeWindow* parent,
const std::string& title,
const std::string& message,
const std::string& detail,
const gfx::ImageSkia& /*icon*/) {
const gfx::ImageSkia& icon) {
return GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
message, detail, "", false)
message, detail, "", false, icon)
.RunSynchronous();
}

Expand All @@ -212,18 +228,19 @@ void ShowMessageBox(NativeWindow* parent,
const std::string& detail,
const std::string& checkbox_label,
bool checkbox_checked,
const gfx::ImageSkia& /*icon*/,
const gfx::ImageSkia& icon,
const MessageBoxCallback& callback) {
(new GtkMessageBox(parent, type, buttons, default_id, cancel_id, title,
message, detail, checkbox_label, checkbox_checked))
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",
base::UTF16ToUTF8(title).c_str(),
base::UTF16ToUTF8(content).c_str(), "", false)
base::UTF16ToUTF8(content).c_str(), "", false,
gfx::ImageSkia())
.RunSynchronous();
} else {
fprintf(stderr,
Expand Down

0 comments on commit 869e0ab

Please sign in to comment.