From 4d09bf3b9ba5130e804ff6d9b11d5f146c41619e Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 13:30:51 +0100 Subject: [PATCH 01/13] demos: Remove gtk_dialog_run() Use modal dialogs and the "response" signal. --- demos/gtk-demo/builder.c | 6 +++-- demos/gtk-demo/demo.ui | 1 + demos/gtk-demo/dialog.c | 48 +++++++++++++++++++++++++++++++--------- 3 files changed, 43 insertions(+), 12 deletions(-) diff --git a/demos/gtk-demo/builder.c b/demos/gtk-demo/builder.c index b0c3f3cdc3..56d2fb08ea 100644 --- a/demos/gtk-demo/builder.c +++ b/demos/gtk-demo/builder.c @@ -26,8 +26,10 @@ about_activate (GSimpleAction *action, builder = g_object_get_data (G_OBJECT (window), "builder"); about_dlg = GTK_WIDGET (gtk_builder_get_object (builder, "aboutdialog1")); - gtk_dialog_run (GTK_DIALOG (about_dlg)); - gtk_widget_hide (about_dlg); + gtk_window_set_transient_for (GTK_WINDOW (about_dlg), GTK_WINDOW (window)); + gtk_window_set_hide_on_close (GTK_WINDOW (about_dlg), TRUE); + g_signal_connect (about_dlg, "response", G_CALLBACK (gtk_widget_hide), NULL); + gtk_widget_show (about_dlg); } static void diff --git a/demos/gtk-demo/demo.ui b/demos/gtk-demo/demo.ui index a25c12d6a1..7cb39a33e7 100644 --- a/demos/gtk-demo/demo.ui +++ b/demos/gtk-demo/demo.ui @@ -94,6 +94,7 @@ Builder demo gtk3-demo + True diff --git a/demos/gtk-demo/dialog.c b/demos/gtk-demo/dialog.c index 99953a5cb5..ccc16c9f6f 100644 --- a/demos/gtk-demo/dialog.c +++ b/demos/gtk-demo/dialog.c @@ -25,11 +25,36 @@ message_dialog_clicked (GtkButton *button, "number of times:"); gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "%d", i); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); + gtk_widget_show (dialog); i++; } +typedef struct { + GtkWidget *local_entry1; + GtkWidget *local_entry2; + GtkWidget *global_entry1; + GtkWidget *global_entry2; +} ResponseData; + +static void +on_dialog_response (GtkDialog *dialog, + int response, + gpointer user_data) +{ + ResponseData *data = user_data; + + if (response == GTK_RESPONSE_OK) + { + gtk_editable_set_text (GTK_EDITABLE (data->global_entry1), + gtk_editable_get_text (GTK_EDITABLE (data->local_entry1))); + gtk_editable_set_text (GTK_EDITABLE (data->global_entry2), + gtk_editable_get_text (GTK_EDITABLE (data->local_entry2))); + } + + gtk_window_destroy (GTK_WINDOW (dialog)); +} + static void interactive_dialog_clicked (GtkButton *button, gpointer user_data) @@ -42,7 +67,7 @@ interactive_dialog_clicked (GtkButton *button, GtkWidget *local_entry1; GtkWidget *local_entry2; GtkWidget *label; - gint response; + ResponseData *data; dialog = gtk_dialog_new_with_buttons ("Interactive Dialog", GTK_WINDOW (window), @@ -81,15 +106,18 @@ interactive_dialog_clicked (GtkButton *button, gtk_grid_attach (GTK_GRID (table), local_entry2, 1, 1, 1, 1); gtk_label_set_mnemonic_widget (GTK_LABEL (label), local_entry2); - response = gtk_dialog_run (GTK_DIALOG (dialog)); + data = g_new (ResponseData, 1); + data->local_entry1 = local_entry1; + data->local_entry2 = local_entry2; + data->global_entry1 = entry1; + data->global_entry2 = entry2; - if (response == GTK_RESPONSE_OK) - { - gtk_editable_set_text (GTK_EDITABLE (entry1), gtk_editable_get_text (GTK_EDITABLE (local_entry1))); - gtk_editable_set_text (GTK_EDITABLE (entry2), gtk_editable_get_text (GTK_EDITABLE (local_entry2))); - } + g_signal_connect_data (dialog, "response", + G_CALLBACK (on_dialog_response), + data, (GClosureNotify) g_free, + 0); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_widget_show (dialog); } GtkWidget * From f573a1f3f2e841eeaa23cf771ee277cc34b8cb5b Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 13:43:17 +0100 Subject: [PATCH 02/13] Remove gtk_dialog_run() from GtkMountOperation --- gtk/gtkmountoperation.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/gtk/gtkmountoperation.c b/gtk/gtkmountoperation.c index 1347cd489e..c39ef85aac 100644 --- a/gtk/gtkmountoperation.c +++ b/gtk/gtkmountoperation.c @@ -1343,6 +1343,18 @@ update_process_list_store (GtkMountOperation *mount_operation, g_array_unref (pid_indices_to_remove); } +static void +on_dialog_response (GtkDialog *dialog, + int response) +{ + /* GTK_RESPONSE_NONE means the dialog were programmatically destroy, e.g. that + * GTK_DIALOG_DESTROY_WITH_PARENT kicked in - so it would trigger a warning to + * destroy the dialog in that case + */ + if (response != GTK_RESPONSE_NONE) + gtk_window_destroy (GTK_WINDOW (dialog)); +} + static void on_end_process_activated (GtkModelButton *button, gpointer user_data) @@ -1379,7 +1391,6 @@ on_end_process_activated (GtkModelButton *button, if (!_gtk_mount_operation_kill_process (pid_to_kill, &error)) { GtkWidget *dialog; - gint response; /* Use GTK_DIALOG_DESTROY_WITH_PARENT here since the parent dialog can be * indeed be destroyed via the GMountOperation::abort signal... for example, @@ -1396,14 +1407,8 @@ on_end_process_activated (GtkModelButton *button, error->message); gtk_widget_show (dialog); - response = gtk_dialog_run (GTK_DIALOG (dialog)); - /* GTK_RESPONSE_NONE means the dialog were programmatically destroy, e.g. that - * GTK_DIALOG_DESTROY_WITH_PARENT kicked in - so it would trigger a warning to - * destroy the dialog in that case - */ - if (response != GTK_RESPONSE_NONE) - gtk_window_destroy (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (on_dialog_response), NULL); g_error_free (error); } From 3212b07cf15927a5c03c6e26923eacdebdb48d53 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 13:51:44 +0100 Subject: [PATCH 03/13] docs: Remove use of gtk_native_dialog_run() from examples Use the "response" signal instead. --- gtk/gtkfilechoosernative.c | 108 +++++++++++++++++++++---------------- 1 file changed, 62 insertions(+), 46 deletions(-) diff --git a/gtk/gtkfilechoosernative.c b/gtk/gtkfilechoosernative.c index 0abb9201ab..4b1c346abd 100644 --- a/gtk/gtkfilechoosernative.c +++ b/gtk/gtkfilechoosernative.c @@ -69,63 +69,79 @@ * In the simplest of cases, you can the following code to use * #GtkFileChooserDialog to select a file for opening: * - * |[ - * GtkFileChooserNative *native; - * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; - * gint res; + * |[ + * static void + * on_response (GtkNativeDialog *dialog, + * int response) + * { + * if (response == GTK_RESPONSE_ACCEPT) + * { + * GtkFileChooser *chooser = GTK_FILE_CHOOSER (native); + * GFile *file = gtk_file_chooser_get_file (chooser); * - * native = gtk_file_chooser_native_new ("Open File", - * parent_window, - * action, - * "_Open", - * "_Cancel"); + * open_file (file); * - * res = gtk_native_dialog_run (GTK_NATIVE_DIALOG (native)); - * if (res == GTK_RESPONSE_ACCEPT) - * { - * char *filename; - * GtkFileChooser *chooser = GTK_FILE_CHOOSER (native); - * filename = gtk_file_chooser_get_filename (chooser); - * open_file (filename); - * g_free (filename); - * } + * g_object_unref (file); + * } * - * g_object_unref (native); + * g_object_unref (native); + * } + * + * // ... + * GtkFileChooserNative *native; + * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; + * + * native = gtk_file_chooser_native_new ("Open File", + * parent_window, + * action, + * "_Open", + * "_Cancel"); + * + * g_signal_connect (native, "response", G_CALLBACK (on_response), NULL); + * gtk_native_dialog_show (GTK_NATIVE_DIALOG (native)); * ]| * * To use a dialog for saving, you can use this: * - * |[ - * GtkFileChooserNative *native; - * GtkFileChooser *chooser; - * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_SAVE; - * gint res; + * |[ + * static void + * on_response (GtkNativeDialog *dialog, + * int response) + * { + * if (response == GTK_RESPONSE_ACCEPT) + * { + * GtkFileChooser *chooser = GTK_FILE_CHOOSER (dialog); + * GFile *file = gtk_file_chooser_get_file (chooser); * - * native = gtk_file_chooser_native_new ("Save File", - * parent_window, - * action, - * "_Save", - * "_Cancel"); - * chooser = GTK_FILE_CHOOSER (native); + * save_to_file (file); * - * if (user_edited_a_new_document) - * gtk_file_chooser_set_current_name (chooser, + * g_object_unref (file); + * } + * + * g_object_unref (native); + * } + * + * // ... + * GtkFileChooserNative *native; + * GtkFileChooser *chooser; + * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_SAVE; + * + * native = gtk_file_chooser_native_new ("Save File", + * parent_window, + * action, + * "_Save", + * "_Cancel"); + * chooser = GTK_FILE_CHOOSER (native); + * + * if (user_edited_a_new_document) + * gtk_file_chooser_set_current_name (chooser, * _("Untitled document")); - * else - * gtk_file_chooser_set_filename (chooser, - * existing_filename); + * else + * gtk_file_chooser_set_filename (chooser, + * existing_filename); * - * res = gtk_native_dialog_run (GTK_NATIVE_DIALOG (native)); - * if (res == GTK_RESPONSE_ACCEPT) - * { - * char *filename; - * - * filename = gtk_file_chooser_get_filename (chooser); - * save_to_file (filename); - * g_free (filename); - * } - * - * g_object_unref (native); + * g_signal_connect (native, "response", G_CALLBACK (on_response), NULL); + * gtk_native_dialog_show (GTK_NATIVE_DIALOG (native)); * ]| * * For more information on how to best set up a file dialog, see #GtkFileChooserDialog. From 5d272a12cb05cd813f791d28f5c8a563b93d50e2 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 13:56:41 +0100 Subject: [PATCH 04/13] Remove gtk_native_dialog_run() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nested main loops are bad, as they introduce layers of complexity caused by the potential re-entrancy in the case of multiple event sources, like IPC, threads, etc. Additionally, the programming model they provide—stop the world while spinning a new loop—does not conform to the event-driven model employed by GTK. --- docs/reference/gtk/gtk4-sections.txt | 1 - gtk/gtknativedialog.c | 97 ---------------------------- gtk/gtknativedialog.h | 3 - 3 files changed, 101 deletions(-) diff --git a/docs/reference/gtk/gtk4-sections.txt b/docs/reference/gtk/gtk4-sections.txt index 5c017f0111..68c436cf84 100644 --- a/docs/reference/gtk/gtk4-sections.txt +++ b/docs/reference/gtk/gtk4-sections.txt @@ -1767,7 +1767,6 @@ gtk_native_dialog_set_title gtk_native_dialog_get_title gtk_native_dialog_set_transient_for gtk_native_dialog_get_transient_for -gtk_native_dialog_run GtkNativeDialog diff --git a/gtk/gtknativedialog.c b/gtk/gtknativedialog.c index 1a060be27a..6a4a5f07ac 100644 --- a/gtk/gtknativedialog.c +++ b/gtk/gtknativedialog.c @@ -56,10 +56,6 @@ * various common properties on the dialog, as well as show and hide * it and get a #GtkNativeDialog::response signal when the user finished * with the dialog. - * - * There is also a gtk_native_dialog_run() helper that makes it easy - * to run any native dialog in a modal way with a recursive mainloop, - * similar to gtk_dialog_run(). */ typedef struct _GtkNativeDialogPrivate GtkNativeDialogPrivate; @@ -558,96 +554,3 @@ gtk_native_dialog_get_transient_for (GtkNativeDialog *self) return priv->transient_for; } - -static void -run_response_cb (GtkNativeDialog *self, - gint response_id, - gpointer data) -{ - GtkNativeDialogPrivate *priv = gtk_native_dialog_get_instance_private (self); - - priv->run_response_id = response_id; - if (priv->run_loop && g_main_loop_is_running (priv->run_loop)) - g_main_loop_quit (priv->run_loop); -} - -/** - * gtk_native_dialog_run: - * @self: a #GtkNativeDialog - * - * Blocks in a recursive main loop until @self emits the - * #GtkNativeDialog::response signal. It then returns the response ID - * from the ::response signal emission. - * - * Before entering the recursive main loop, gtk_native_dialog_run() - * calls gtk_native_dialog_show() on the dialog for you. - * - * After gtk_native_dialog_run() returns, then dialog will be hidden. - * - * Typical usage of this function might be: - * |[ - * gint result = gtk_native_dialog_run (GTK_NATIVE_DIALOG (dialog)); - * switch (result) - * { - * case GTK_RESPONSE_ACCEPT: - * do_application_specific_something (); - * break; - * default: - * do_nothing_since_dialog_was_cancelled (); - * break; - * } - * g_object_unref (dialog); - * ]| - * - * Note that even though the recursive main loop gives the effect of a - * modal dialog (it prevents the user from interacting with other - * windows in the same window group while the dialog is run), callbacks - * such as timeouts, IO channel watches, DND drops, etc, will - * be triggered during a gtk_native_dialog_run() call. - * - * Returns: response ID - **/ -gint -gtk_native_dialog_run (GtkNativeDialog *self) -{ - GtkNativeDialogPrivate *priv = gtk_native_dialog_get_instance_private (self); - gboolean was_modal; - guint response_handler; - - g_return_val_if_fail (GTK_IS_NATIVE_DIALOG (self), -1); - g_return_val_if_fail (!priv->visible, -1); - g_return_val_if_fail (priv->run_loop == NULL, -1); - - if (priv->visible || priv->run_loop != NULL) - return -1; - - g_object_ref (self); - - priv->run_response_id = GTK_RESPONSE_NONE; - priv->run_loop = g_main_loop_new (NULL, FALSE); - - was_modal = priv->modal; - gtk_native_dialog_set_modal (self, TRUE); - - response_handler = - g_signal_connect (self, - "response", - G_CALLBACK (run_response_cb), - NULL); - - gtk_native_dialog_show (self); - - g_main_loop_run (priv->run_loop); - - g_signal_handler_disconnect (self, response_handler); - - g_main_loop_unref (priv->run_loop); - priv->run_loop = NULL; - - if (!was_modal) - gtk_native_dialog_set_modal (self, FALSE); - - g_object_unref (self); - - return priv->run_response_id; -} diff --git a/gtk/gtknativedialog.h b/gtk/gtknativedialog.h index 00d34532d0..f6a2dc20ae 100644 --- a/gtk/gtknativedialog.h +++ b/gtk/gtknativedialog.h @@ -73,9 +73,6 @@ void gtk_native_dialog_set_transient_for (GtkNativeDialog *self GDK_AVAILABLE_IN_ALL GtkWindow * gtk_native_dialog_get_transient_for (GtkNativeDialog *self); -GDK_AVAILABLE_IN_ALL -gint gtk_native_dialog_run (GtkNativeDialog *self); - G_END_DECLS #endif /* __GTK_NATIVE_DIALOG_H__ */ From f81e6042be9e15a3f14b02e9ffc8f1cbafd995d8 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 14:05:23 +0100 Subject: [PATCH 05/13] docs: Remove use of gtk_dialog_run() Direct people to use GTK_DIALOG_MODAL and the "response" signal instead of nested main loops. --- gtk/gtkaboutdialog.c | 5 +- gtk/gtkfilechooserdialog.c | 111 +++++++++++++++++++++++-------------- gtk/gtkmessagedialog.c | 30 +++++----- 3 files changed, 88 insertions(+), 58 deletions(-) diff --git a/gtk/gtkaboutdialog.c b/gtk/gtkaboutdialog.c index 6b18fe8413..96411e26a8 100644 --- a/gtk/gtkaboutdialog.c +++ b/gtk/gtkaboutdialog.c @@ -104,8 +104,9 @@ * ]| * * It is also possible to show a #GtkAboutDialog like any other #GtkDialog, - * e.g. using gtk_dialog_run(). In this case, you might need to know that - * the “Close” button returns the #GTK_RESPONSE_CANCEL response id. + * and use the #GtkDialog::response signal to catch user responses. In this + * case, you might need to know that the “Close” button returns the + * %GTK_RESPONSE_CANCEL response id. */ typedef struct diff --git a/gtk/gtkfilechooserdialog.c b/gtk/gtkfilechooserdialog.c index 8449c321a6..46ce76e4d7 100644 --- a/gtk/gtkfilechooserdialog.c +++ b/gtk/gtkfilechooserdialog.c @@ -70,61 +70,86 @@ * #GtkFileChooserDialog to select a file for opening: * * |[ - * GtkWidget *dialog; - * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; - * gint res; + * static void + * on_open_response (GtkDialog *dialog, + * int response) + * { + * if (response == GTK_RESPONSE_ACCEPT) + * { + * GtkFileChooser *chooser = GTK_FILE_CHOOSER (dialog); * - * dialog = gtk_file_chooser_dialog_new ("Open File", - * parent_window, - * action, - * _("_Cancel"), - * GTK_RESPONSE_CANCEL, - * _("_Open"), - * GTK_RESPONSE_ACCEPT, - * NULL); + * g_autoptr(GFile) file = gtk_file_chooser_get_file (chooser); * - * res = gtk_dialog_run (GTK_DIALOG (dialog)); - * if (res == GTK_RESPONSE_ACCEPT) - * { - * GtkFileChooser *chooser = GTK_FILE_CHOOSER (dialog); - * g_autoptr(GFile) filen = gtk_file_chooser_get_file (chooser); - * open_file (file); - * } + * open_file (file); + * } * - * gtk_window_destroy (dialog); + * gtk_window_destroy (GTK_WINDOW (dialog)); + * } + * + * // ... + * GtkWidget *dialog; + * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_OPEN; + * + * dialog = gtk_file_chooser_dialog_new ("Open File", + * parent_window, + * action, + * _("_Cancel"), + * GTK_RESPONSE_CANCEL, + * _("_Open"), + * GTK_RESPONSE_ACCEPT, + * NULL); + * + * gtk_widget_show (dialog); + * + * g_signal_connect (dialog, "response", + * G_CALLBACK (on_open_response), + * NULL); * ]| * * To use a dialog for saving, you can use this: * * |[ - * GtkWidget *dialog; - * GtkFileChooser *chooser; - * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_SAVE; - * gint res; + * static void + * on_save_response (GtkDialog *dialog, + * int response) + * { + * if (response == GTK_RESPONSE_ACCEPT) + * { + * GtkFileChooser *chooser = GTK_FILE_CHOOSER (dialog); * - * dialog = gtk_file_chooser_dialog_new ("Save File", - * parent_window, - * action, - * _("_Cancel"), - * GTK_RESPONSE_CANCEL, - * _("_Save"), - * GTK_RESPONSE_ACCEPT, - * NULL); - * chooser = GTK_FILE_CHOOSER (dialog); + * g_autoptr(GFile) file = gtk_file_chooser_get_file (chooser); * - * if (user_edited_a_new_document) - * gtk_file_chooser_set_current_name (chooser, _("Untitled document")); - * else - * gtk_file_chooser_set_file (chooser, existing_filename); + * save_to_file (file); + * } * - * res = gtk_dialog_run (GTK_DIALOG (dialog)); - * if (res == GTK_RESPONSE_ACCEPT) - * { - * g_autoptr(GFile) file = gtk_file_chooser_get_file (chooser); - * save_to_file (file); - * } + * gtk_window_destroy (GTK_WINDOW (dialog)); + * } * - * gtk_window_destroy (dialog); + * // ... + * GtkWidget *dialog; + * GtkFileChooser *chooser; + * GtkFileChooserAction action = GTK_FILE_CHOOSER_ACTION_SAVE; + * + * dialog = gtk_file_chooser_dialog_new ("Save File", + * parent_window, + * action, + * _("_Cancel"), + * GTK_RESPONSE_CANCEL, + * _("_Save"), + * GTK_RESPONSE_ACCEPT, + * NULL); + * chooser = GTK_FILE_CHOOSER (dialog); + * + * if (user_edited_a_new_document) + * gtk_file_chooser_set_current_name (chooser, _("Untitled document")); + * else + * gtk_file_chooser_set_file (chooser, existing_filename); + * + * gtk_widget_show (dialog); + * + * g_signal_connect (dialog, "response", + * G_CALLBACK (on_save_response), + * NULL); * ]| * * ## Setting up a file chooser dialog ## {#gtkfilechooserdialog-setting-up} diff --git a/gtk/gtkmessagedialog.c b/gtk/gtkmessagedialog.c index be0b4afe2c..0d8636a016 100644 --- a/gtk/gtkmessagedialog.c +++ b/gtk/gtkmessagedialog.c @@ -49,14 +49,15 @@ * convenience widget; you could construct the equivalent of #GtkMessageDialog * from #GtkDialog without too much effort, but #GtkMessageDialog saves typing. * - * The easiest way to do a modal message dialog is to use gtk_dialog_run(), though - * you can also pass in the %GTK_DIALOG_MODAL flag, gtk_dialog_run() automatically - * makes the dialog modal and waits for the user to respond to it. gtk_dialog_run() - * returns when any dialog button is clicked. + * The easiest way to do a modal message dialog is to use the %GTK_DIALOG_MODAL + * flag, which will call gtk_window_set_modal() internally. The dialog will + * prevent interaction with the parent window until it's hidden or destroyed. + * You can use the #GtkDialog::response signal to know when the user dismissed + * the dialog. * * An example for using a modal dialog: * |[ - * GtkDialogFlags flags = GTK_DIALOG_DESTROY_WITH_PARENT; + * GtkDialogFlags flags = GTK_DIALOG_DESTROY_WITH_PARENT | GTK_DIALOG_MODAL; * dialog = gtk_message_dialog_new (parent_window, * flags, * GTK_MESSAGE_ERROR, @@ -64,13 +65,17 @@ * "Error reading “%s”: %s", * filename, * g_strerror (errno)); - * gtk_dialog_run (GTK_DIALOG (dialog)); - * gtk_window_destroy (GTK_WINDOW (dialog)); + * // Destroy the dialog when the user responds to it + * // (e.g. clicks a button) + * + * g_signal_connect (dialog, "response", + * G_CALLBACK (gtk_window_destroy), + * NULL); * ]| * - * You might do a non-modal #GtkMessageDialog as follows: + * You might do a non-modal #GtkMessageDialog simply by omitting the + * %GTK_DIALOG_MODAL flag: * - * An example for a non-modal dialog: * |[ * GtkDialogFlags flags = GTK_DIALOG_DESTROY_WITH_PARENT; * dialog = gtk_message_dialog_new (parent_window, @@ -83,10 +88,9 @@ * * // Destroy the dialog when the user responds to it * // (e.g. clicks a button) - * - * g_signal_connect_swapped (dialog, "response", - * G_CALLBACK (gtk_window_destroy), - * dialog); + * g_signal_connect (dialog, "response", + * G_CALLBACK (gtk_window_destroy), + * NULL); * ]| * * # GtkMessageDialog as GtkBuildable From 96856527e643cc8fc48fb78f40485974efd6a5a8 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 14:41:57 +0100 Subject: [PATCH 06/13] Drop gtk_dialog_run() from GtkFileChooserWidget The various dialogs we use inside the file chooser are modal already, and do no need a nested loop. --- gtk/gtkfilechooserwidget.c | 123 ++++++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 44 deletions(-) diff --git a/gtk/gtkfilechooserwidget.c b/gtk/gtkfilechooserwidget.c index ab36bded5a..e9cb8cd06c 100644 --- a/gtk/gtkfilechooserwidget.c +++ b/gtk/gtkfilechooserwidget.c @@ -765,9 +765,11 @@ error_message (GtkFileChooserWidget *impl, gtk_window_group_add_window (gtk_window_get_group (parent), GTK_WINDOW (dialog)); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", + G_CALLBACK (gtk_window_destroy), + NULL); } /* Shows a simple error dialog relative to a path. Frees the GError as well. */ @@ -1231,14 +1233,40 @@ add_to_shortcuts_cb (GSimpleAction *action, impl); } -static gboolean +typedef struct { + GtkFileChooserWidget *impl; + GFile *file; +} ConfirmDeleteData; + +static void +on_confirm_delete_response (GtkWidget *dialog, + int response, + gpointer user_data) +{ + ConfirmDeleteData *data = user_data; + + gtk_window_destroy (GTK_WINDOW (dialog)); + + if (response == GTK_RESPONSE_ACCEPT) + { + GError *error = NULL; + + if (!g_file_delete (data->file, NULL, &error)) + error_deleting_file (data->impl, data->file, error); + } + + g_free (data); +} + +static void confirm_delete (GtkFileChooserWidget *impl, + GFile *file, GFileInfo *info) { GtkWindow *toplevel; GtkWidget *dialog; - gint response; - const gchar *name; + const char *name; + ConfirmDeleteData *data; name = g_file_info_get_display_name (info); @@ -1259,11 +1287,15 @@ confirm_delete (GtkFileChooserWidget *impl, if (gtk_window_has_group (toplevel)) gtk_window_group_add_window (gtk_window_get_group (toplevel), GTK_WINDOW (dialog)); - response = gtk_dialog_run (GTK_DIALOG (dialog)); + gtk_widget_show (dialog); - gtk_window_destroy (GTK_WINDOW (dialog)); + data = g_new (ConfirmDeleteData, 1); + data->impl = impl; + data->file = file; - return (response == GTK_RESPONSE_ACCEPT); + g_signal_connect (dialog, "response", + G_CALLBACK (on_confirm_delete_response), + data); } static void @@ -1275,16 +1307,11 @@ delete_selected_cb (GtkTreeModel *model, GtkFileChooserWidget *impl = data; GFile *file; GFileInfo *info; - GError *error = NULL; file = _gtk_file_system_model_get_file (GTK_FILE_SYSTEM_MODEL (model), iter); info = _gtk_file_system_model_get_info (GTK_FILE_SYSTEM_MODEL (model), iter); - if (confirm_delete (impl, info)) - { - if (!g_file_delete (file, NULL, &error)) - error_deleting_file (impl, file, error); - } + confirm_delete (impl, file, info); } static void @@ -5716,17 +5743,43 @@ add_custom_button_to_dialog (GtkDialog *dialog, gtk_dialog_add_action_widget (GTK_DIALOG (dialog), button, response_id); } -/* Presents an overwrite confirmation dialog; returns whether we should accept - * the filename. +/* Every time we request a response explicitly, we need to save the selection to + * the recently-used list, as requesting a response means, “the dialog is confirmed”. */ -static gboolean +static void +request_response_and_add_to_recent_list (GtkFileChooserWidget *impl) +{ + g_signal_emit_by_name (impl, "response-requested"); + add_selection_to_recent_list (impl); +} + +static void +on_confirm_overwrite_response (GtkWidget *dialog, + int response, + gpointer user_data) +{ + GtkFileChooserWidget *impl = user_data; + + if (response == GTK_RESPONSE_ACCEPT) + { + /* Dialog is now going to be closed, so prevent any button/key presses to + * file list (will be restablished on next map()). Fixes data loss bug #2288 */ + impl->browse_files_interaction_frozen = TRUE; + + request_response_and_add_to_recent_list (impl); + } + + gtk_window_destroy (GTK_WINDOW (dialog)); +} + +/* Presents an overwrite confirmation dialog */ +static void confirm_dialog_should_accept_filename (GtkFileChooserWidget *impl, const gchar *file_part, const gchar *folder_display_name) { GtkWindow *toplevel; GtkWidget *dialog; - int response; toplevel = get_toplevel (GTK_WIDGET (impl)); @@ -5748,16 +5801,11 @@ confirm_dialog_should_accept_filename (GtkFileChooserWidget *impl, if (gtk_window_has_group (toplevel)) gtk_window_group_add_window (gtk_window_get_group (toplevel), GTK_WINDOW (dialog)); - response = gtk_dialog_run (GTK_DIALOG (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); - if (response == GTK_RESPONSE_ACCEPT) - /* Dialog is now going to be closed, so prevent any button/key presses to - * file list (will be restablished on next map()). Fixes data loss bug #2288 */ - impl->browse_files_interaction_frozen = TRUE; - - gtk_window_destroy (GTK_WINDOW (dialog)); - - return (response == GTK_RESPONSE_ACCEPT); + g_signal_connect (dialog, "response", + G_CALLBACK (on_confirm_overwrite_response), + impl); } struct GetDisplayNameData @@ -5766,16 +5814,6 @@ struct GetDisplayNameData gchar *file_part; }; -/* Every time we request a response explicitly, we need to save the selection to - * the recently-used list, as requesting a response means, “the dialog is confirmed”. - */ -static void -request_response_and_add_to_recent_list (GtkFileChooserWidget *impl) -{ - g_signal_emit_by_name (impl, "response-requested"); - add_selection_to_recent_list (impl); -} - static void confirmation_confirm_get_info_cb (GCancellable *cancellable, GFileInfo *info, @@ -5783,7 +5821,6 @@ confirmation_confirm_get_info_cb (GCancellable *cancellable, gpointer user_data) { gboolean cancelled = g_cancellable_is_cancelled (cancellable); - gboolean should_respond = FALSE; struct GetDisplayNameData *data = user_data; if (cancellable != data->impl->should_respond_get_info_cancellable) @@ -5795,14 +5832,12 @@ confirmation_confirm_get_info_cb (GCancellable *cancellable, goto out; if (error) - /* Huh? Did the folder disappear? Let the caller deal with it */ - should_respond = TRUE; - else - should_respond = confirm_dialog_should_accept_filename (data->impl, data->file_part, g_file_info_get_display_name (info)); + goto out; + + confirm_dialog_should_accept_filename (data->impl, data->file_part, + g_file_info_get_display_name (info)); set_busy_cursor (data->impl, FALSE); - if (should_respond) - request_response_and_add_to_recent_list (data->impl); out: g_object_unref (data->impl); From 2090dbb27d4dd3afc76bff4631e5b506c5ee0009 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 15:32:38 +0100 Subject: [PATCH 07/13] tests: Remove gtk_dialog_run() Either use the "response" signal for dialogs that are already modal, or use an explicit nested loop for tests that rely on the response id being available in sequence. --- tests/print-editor.c | 78 +++++++++++++++++++++++++---------------- tests/testappchooser.c | 37 +++++++++++++------ tests/testcombochange.c | 35 ++++++++++++------ tests/testdialog.c | 61 ++++++++++++++------------------ tests/testfilechooser.c | 12 ++++--- tests/testwindowsize.c | 21 +++++++---- 6 files changed, 148 insertions(+), 96 deletions(-) diff --git a/tests/print-editor.c b/tests/print-editor.c index 7b8ca15e15..37ea8cf7ca 100644 --- a/tests/print-editor.c +++ b/tests/print-editor.c @@ -496,22 +496,9 @@ activate_preview (GSimpleAction *action, } static void -activate_save_as (GSimpleAction *action, - GVariant *parameter, - gpointer user_data) +on_save_response (GtkWidget *dialog, + int response) { - GtkWidget *dialog; - gint response; - - dialog = gtk_file_chooser_dialog_new ("Select file", - GTK_WINDOW (main_window), - GTK_FILE_CHOOSER_ACTION_SAVE, - "_Cancel", GTK_RESPONSE_CANCEL, - "_Save", GTK_RESPONSE_OK, - NULL); - gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_OK); - response = gtk_dialog_run (GTK_DIALOG (dialog)); - if (response == GTK_RESPONSE_OK) { GFile *save_filename = gtk_file_chooser_get_file (GTK_FILE_CHOOSER (dialog)); @@ -522,6 +509,28 @@ activate_save_as (GSimpleAction *action, gtk_window_destroy (GTK_WINDOW (dialog)); } +static void +activate_save_as (GSimpleAction *action, + GVariant *parameter, + gpointer user_data) +{ + GtkWidget *dialog; + + dialog = gtk_file_chooser_dialog_new ("Select file", + GTK_WINDOW (main_window), + GTK_FILE_CHOOSER_ACTION_SAVE, + "_Cancel", GTK_RESPONSE_CANCEL, + "_Save", GTK_RESPONSE_OK, + NULL); + gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_OK); + gtk_window_set_modal (GTK_WINDOW (dialog), TRUE); + gtk_widget_show (dialog); + + g_signal_connect (dialog, "response", + G_CALLBACK (on_save_response), + NULL); +} + static void activate_save (GSimpleAction *action, GVariant *parameter, @@ -534,22 +543,9 @@ activate_save (GSimpleAction *action, } static void -activate_open (GSimpleAction *action, - GVariant *parameter, - gpointer user_data) +on_open_response (GtkWidget *dialog, + int response) { - GtkWidget *dialog; - gint response; - - dialog = gtk_file_chooser_dialog_new ("Select file", - GTK_WINDOW (main_window), - GTK_FILE_CHOOSER_ACTION_OPEN, - "_Cancel", GTK_RESPONSE_CANCEL, - "_Open", GTK_RESPONSE_OK, - NULL); - gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_OK); - response = gtk_dialog_run (GTK_DIALOG (dialog)); - if (response == GTK_RESPONSE_OK) { GFile *open_filename = gtk_file_chooser_get_file (GTK_FILE_CHOOSER (dialog)); @@ -560,6 +556,28 @@ activate_open (GSimpleAction *action, gtk_window_destroy (GTK_WINDOW (dialog)); } +static void +activate_open (GSimpleAction *action, + GVariant *parameter, + gpointer user_data) +{ + GtkWidget *dialog; + + dialog = gtk_file_chooser_dialog_new ("Select file", + GTK_WINDOW (main_window), + GTK_FILE_CHOOSER_ACTION_OPEN, + "_Cancel", GTK_RESPONSE_CANCEL, + "_Open", GTK_RESPONSE_OK, + NULL); + gtk_dialog_set_default_response (GTK_DIALOG (dialog), GTK_RESPONSE_OK); + gtk_window_set_modal (GTK_WINDOW (dialog), TRUE); + gtk_widget_show (dialog); + + g_signal_connect (dialog, "response", + G_CALLBACK (on_open_response), + NULL); +} + static void activate_new (GSimpleAction *action, GVariant *parameter, diff --git a/tests/testappchooser.c b/tests/testappchooser.c index 281f96e9bb..63ec9bfdee 100644 --- a/tests/testappchooser.c +++ b/tests/testappchooser.c @@ -125,12 +125,32 @@ display_dialog (void) gtk_widget_show (dialog); } +static void +on_open_response (GtkWidget *file_chooser, + int response) +{ + if (response == GTK_RESPONSE_ACCEPT) + { + char *path; + + file = gtk_file_chooser_get_file (GTK_FILE_CHOOSER (file_chooser)); + path = g_file_get_path (file); + + gtk_button_set_label (GTK_BUTTON (file_l), path); + + g_free (path); + } + + gtk_window_destroy (GTK_WINDOW (file_chooser)); + + gtk_widget_set_sensitive (open, TRUE); +} + static void button_clicked (GtkButton *b, gpointer user_data) { GtkWidget *w; - gchar *path; w = gtk_file_chooser_dialog_new ("Select file", GTK_WINDOW (toplevel), @@ -139,16 +159,13 @@ button_clicked (GtkButton *b, "_Open", GTK_RESPONSE_ACCEPT, NULL); - gtk_dialog_run (GTK_DIALOG (w)); - file = gtk_file_chooser_get_file (GTK_FILE_CHOOSER (w)); - path = g_file_get_path (file); - gtk_button_set_label (GTK_BUTTON (file_l), path); + gtk_window_set_modal (GTK_WINDOW (w), TRUE); - gtk_window_destroy (GTK_WINDOW (w)); + g_signal_connect (w, "response", + G_CALLBACK (on_open_response), + NULL); - gtk_widget_set_sensitive (open, TRUE); - - g_free (path); + gtk_window_present (GTK_WINDOW (w)); } static void @@ -180,7 +197,7 @@ main (int argc, char **argv) w1, 0, 0, 1, 1); file_l = gtk_button_new (); - path = g_build_filename (g_get_current_dir (), "apple-red.png", NULL); + path = g_build_filename (GTK_SRCDIR, "apple-red.png", NULL); file = g_file_new_for_path (path); gtk_button_set_label (GTK_BUTTON (file_l), path); g_free (path); diff --git a/tests/testcombochange.c b/tests/testcombochange.c index 29cefc662a..a4a6ec9b56 100644 --- a/tests/testcombochange.c +++ b/tests/testcombochange.c @@ -191,7 +191,7 @@ int main (int argc, char **argv) { GtkWidget *content_area; - GtkWidget *dialog; + GtkWidget *window; GtkWidget *scrolled_window; GtkWidget *hbox; GtkWidget *button_vbox; @@ -206,13 +206,12 @@ main (int argc, char **argv) model = gtk_list_store_new (1, G_TYPE_STRING); contents = g_array_new (FALSE, FALSE, sizeof (char)); - - dialog = gtk_dialog_new_with_buttons ("GtkComboBox model changes", - NULL, 0, - "_Close", GTK_RESPONSE_CLOSE, - NULL); - content_area = gtk_dialog_get_content_area (GTK_DIALOG (dialog)); + window = gtk_window_new (); + gtk_window_set_title (GTK_WINDOW (window), "ComboBox Change"); + + content_area = gtk_box_new (GTK_ORIENTATION_VERTICAL, 12); + gtk_window_set_child (GTK_WINDOW (window), content_area); hbox = gtk_box_new (GTK_ORIENTATION_HORIZONTAL, 12); gtk_box_append (GTK_BOX (content_area), hbox); @@ -243,7 +242,7 @@ main (int argc, char **argv) button_vbox = gtk_box_new (GTK_ORIENTATION_VERTICAL, 8); gtk_box_append (GTK_BOX (hbox), button_vbox); - gtk_window_set_default_size (GTK_WINDOW (dialog), 500, 300); + gtk_window_set_default_size (GTK_WINDOW (window), 500, 300); button = gtk_button_new_with_label ("Insert"); gtk_box_append (GTK_BOX (button_vbox), button); @@ -261,8 +260,24 @@ main (int argc, char **argv) gtk_box_append (GTK_BOX (button_vbox), button); g_signal_connect (button, "clicked", G_CALLBACK (on_animate), NULL); - gtk_widget_show (dialog); - gtk_dialog_run (GTK_DIALOG (dialog)); + GtkWidget *close_button = gtk_button_new_with_mnemonic ("_Close"); + gtk_widget_set_hexpand (close_button, TRUE); + gtk_box_append (GTK_BOX (content_area), close_button); + + gtk_widget_show (window); + + GMainLoop *loop = g_main_loop_new (NULL, FALSE); + + g_signal_connect_swapped (close_button, "clicked", + G_CALLBACK (gtk_window_destroy), + window); + g_signal_connect_swapped (window, "destroy", + G_CALLBACK (g_main_loop_quit), + loop); + + g_main_loop_run (loop); + + g_main_loop_unref (loop); return 0; } diff --git a/tests/testdialog.c b/tests/testdialog.c index c78ec11336..75027b3c9b 100644 --- a/tests/testdialog.c +++ b/tests/testdialog.c @@ -16,8 +16,8 @@ show_message_dialog1 (GtkWindow *parent) gtk_message_dialog_format_secondary_text (GTK_MESSAGE_DIALOG (dialog), "Unhandled error message: SSH program unexpectedly exited"); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -33,8 +33,8 @@ show_message_dialog1a (GtkWindow *parent) GTK_BUTTONS_OK, "The system network services are not compatible with this version.")); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -56,8 +56,8 @@ show_message_dialog2 (GtkWindow *parent) "Empty Wastebasket", GTK_RESPONSE_OK, NULL); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -66,9 +66,8 @@ show_color_chooser (GtkWindow *parent) GtkWidget *dialog; dialog = gtk_color_chooser_dialog_new ("Builtin", parent); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -81,8 +80,8 @@ show_color_chooser_generic (GtkWindow *parent) "transient-for", parent, NULL); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -119,9 +118,8 @@ show_dialog (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -135,9 +133,8 @@ show_dialog_with_header (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -152,9 +149,8 @@ show_dialog_with_buttons (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -169,9 +165,8 @@ show_dialog_with_header_buttons (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -183,9 +178,8 @@ show_dialog_with_header_buttons2 (GtkWindow *parent) builder = gtk_builder_new_from_file ("dialog.ui"); dialog = (GtkWidget *)gtk_builder_get_object (builder, "dialog"); g_object_unref (builder); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } typedef struct { @@ -231,9 +225,8 @@ show_dialog_from_template (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } static void @@ -252,9 +245,8 @@ show_dialog_flex_template (GtkWindow *parent) NULL); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } typedef struct { @@ -305,9 +297,8 @@ show_dialog_from_template_with_header (GtkWindow *parent) add_buttons (dialog); add_content (dialog); - - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_window_present (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", G_CALLBACK (gtk_window_destroy), NULL); } int diff --git a/tests/testfilechooser.c b/tests/testfilechooser.c index 0cb5875ab6..71c0358af3 100644 --- a/tests/testfilechooser.c +++ b/tests/testfilechooser.c @@ -156,8 +156,10 @@ set_current_folder (GtkFileChooser *chooser, GTK_BUTTONS_CLOSE, "Could not set the folder to %s", name); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_widget_show (dialog); + g_signal_connect (dialog, "response", + G_CALLBACK (gtk_window_destroy), + NULL); } g_object_unref (file); } @@ -191,8 +193,10 @@ set_filename (GtkFileChooser *chooser, GTK_BUTTONS_CLOSE, "Could not select %s", name); - gtk_dialog_run (GTK_DIALOG (dialog)); - gtk_window_destroy (GTK_WINDOW (dialog)); + gtk_widget_show (dialog); + g_signal_connect (dialog, "response", + G_CALLBACK (gtk_window_destroy), + NULL); } g_object_unref (file); } diff --git a/tests/testwindowsize.c b/tests/testwindowsize.c index 961843cc05..ae6b2fcaa5 100644 --- a/tests/testwindowsize.c +++ b/tests/testwindowsize.c @@ -65,10 +65,10 @@ show_dialog (void) gtk_widget_realize (dialog); g_signal_connect (gtk_native_get_surface (GTK_NATIVE (dialog)), "size-changed", G_CALLBACK (size_changed_cb), label); - - gtk_dialog_run (GTK_DIALOG (dialog)); - - gtk_window_destroy (GTK_WINDOW (dialog)); + g_signal_connect (dialog, "response", + G_CALLBACK (gtk_window_destroy), + NULL); + gtk_widget_show (dialog); } static void @@ -127,6 +127,16 @@ create_window (void) gtk_grid_attach (GTK_GRID (grid), button, 2, 4, 1, 1); gtk_widget_show (window); + + GMainLoop *loop = g_main_loop_new (NULL, FALSE); + + g_signal_connect_swapped (window, "destroy", + G_CALLBACK (g_main_loop_quit), + loop); + + g_main_loop_run (loop); + + g_main_loop_unref (loop); } int @@ -136,8 +146,5 @@ main (int argc, char *argv[]) create_window (); - while (TRUE) - g_main_context_iteration (NULL, TRUE); - return 0; } From 45eec065009f232c0f6d1d7e68487b2bdf9c936c Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 16:28:00 +0100 Subject: [PATCH 08/13] Drop gtk_dialog_run() from GtkPrintOperationUnix We still provide a blocking API, but we should strongly reconsider it. --- gtk/gtkprintoperation-unix.c | 40 +++++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 7 deletions(-) diff --git a/gtk/gtkprintoperation-unix.c b/gtk/gtkprintoperation-unix.c index ba79ea0bca..fc00ff004e 100644 --- a/gtk/gtkprintoperation-unix.c +++ b/gtk/gtkprintoperation-unix.c @@ -658,6 +658,8 @@ handle_print_response (GtkWidget *dialog, gtk_window_destroy (GTK_WINDOW (pd)); + if (rdata->loop) + g_main_loop_quit (rdata->loop); } @@ -669,7 +671,7 @@ found_printer (GtkPrinter *printer, GtkPrintOperationPrivate *priv = op->priv; GtkPrintSettings *settings = NULL; GtkPageSetup *page_setup = NULL; - + if (rdata->loop) g_main_loop_quit (rdata->loop); @@ -851,7 +853,6 @@ gtk_print_operation_unix_run_dialog (GtkPrintOperation *op, { GtkWidget *pd; PrintResponseData rdata; - gint response; const gchar *printer_name; rdata.op = op; @@ -866,9 +867,19 @@ gtk_print_operation_unix_run_dialog (GtkPrintOperation *op, if (show_dialog) { pd = get_print_dialog (op, parent); + gtk_window_set_modal (GTK_WINDOW (pd), TRUE); - response = gtk_dialog_run (GTK_DIALOG (pd)); - handle_print_response (pd, response, &rdata); + g_signal_connect (pd, "response", + G_CALLBACK (handle_print_response), &rdata); + + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + gtk_window_present (GTK_WINDOW (pd)); + G_GNUC_END_IGNORE_DEPRECATIONS + + rdata.loop = g_main_loop_new (NULL, FALSE); + g_main_loop_run (rdata.loop); + g_main_loop_unref (rdata.loop); + rdata.loop = NULL; } else { @@ -897,6 +908,7 @@ typedef struct GtkPageSetupDoneFunc done_cb; gpointer data; GDestroyNotify destroy; + GMainLoop *loop; } PageSetupResponseData; static void @@ -918,6 +930,9 @@ handle_page_setup_response (GtkWidget *dialog, GtkPageSetupUnixDialog *psd; PageSetupResponseData *rdata = data; + if (rdata->loop) + g_main_loop_quit (rdata->loop); + psd = GTK_PAGE_SETUP_UNIX_DIALOG (dialog); if (response == GTK_RESPONSE_OK) rdata->page_setup = gtk_page_setup_unix_dialog_get_page_setup (psd); @@ -971,18 +986,28 @@ gtk_print_run_page_setup_dialog (GtkWindow *parent, GtkPrintSettings *settings) { GtkWidget *dialog; - gint response; PageSetupResponseData rdata; rdata.page_setup = NULL; rdata.done_cb = NULL; rdata.data = NULL; rdata.destroy = NULL; + rdata.loop = g_main_loop_new (NULL, FALSE); dialog = get_page_setup_dialog (parent, page_setup, settings); - response = gtk_dialog_run (GTK_DIALOG (dialog)); - handle_page_setup_response (dialog, response, &rdata); + + g_signal_connect (dialog, "response", + G_CALLBACK (handle_page_setup_response), + &rdata); + + G_GNUC_BEGIN_IGNORE_DEPRECATIONS + gtk_window_present (GTK_WINDOW (dialog)); + G_GNUC_END_IGNORE_DEPRECATIONS + g_main_loop_run (rdata.loop); + g_main_loop_unref (rdata.loop); + rdata.loop = NULL; + if (rdata.page_setup) return rdata.page_setup; else if (page_setup) @@ -1024,6 +1049,7 @@ gtk_print_run_page_setup_dialog_async (GtkWindow *parent, rdata->done_cb = done_cb; rdata->data = data; rdata->destroy = page_setup_data_free; + rdata->loop = NULL; g_signal_connect (dialog, "response", G_CALLBACK (handle_page_setup_response), rdata); From b8988be4b50069317b07fe13dcd9d950eb7d2a4f Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 17:24:15 +0100 Subject: [PATCH 09/13] Remove unnecessary deprecation pragmas The gtk_window_present() function is not deprecated. --- gtk/gtkprintoperation-unix.c | 8 -------- 1 file changed, 8 deletions(-) diff --git a/gtk/gtkprintoperation-unix.c b/gtk/gtkprintoperation-unix.c index fc00ff004e..f6b2195e17 100644 --- a/gtk/gtkprintoperation-unix.c +++ b/gtk/gtkprintoperation-unix.c @@ -732,9 +732,7 @@ gtk_print_operation_unix_run_dialog_async (GtkPrintOperation *op, g_signal_connect (pd, "response", G_CALLBACK (handle_print_response), rdata); - G_GNUC_BEGIN_IGNORE_DEPRECATIONS gtk_window_present (GTK_WINDOW (pd)); - G_GNUC_END_IGNORE_DEPRECATIONS } else { @@ -872,9 +870,7 @@ gtk_print_operation_unix_run_dialog (GtkPrintOperation *op, g_signal_connect (pd, "response", G_CALLBACK (handle_print_response), &rdata); - G_GNUC_BEGIN_IGNORE_DEPRECATIONS gtk_window_present (GTK_WINDOW (pd)); - G_GNUC_END_IGNORE_DEPRECATIONS rdata.loop = g_main_loop_new (NULL, FALSE); g_main_loop_run (rdata.loop); @@ -1000,9 +996,7 @@ gtk_print_run_page_setup_dialog (GtkWindow *parent, G_CALLBACK (handle_page_setup_response), &rdata); - G_GNUC_BEGIN_IGNORE_DEPRECATIONS gtk_window_present (GTK_WINDOW (dialog)); - G_GNUC_END_IGNORE_DEPRECATIONS g_main_loop_run (rdata.loop); g_main_loop_unref (rdata.loop); @@ -1054,9 +1048,7 @@ gtk_print_run_page_setup_dialog_async (GtkWindow *parent, g_signal_connect (dialog, "response", G_CALLBACK (handle_page_setup_response), rdata); - G_GNUC_BEGIN_IGNORE_DEPRECATIONS gtk_window_present (GTK_WINDOW (dialog)); - G_GNUC_END_IGNORE_DEPRECATIONS } struct _PrinterFinder From 0a6848d70bd49d67034ceae005ce972386f08553 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 18:45:01 +0100 Subject: [PATCH 10/13] Remove gtk_dialog_run() from GtkPrintUnixDialog Replace it with an explicit nested main loop, as we need to block the signal handler currently being emitted depending on the response of the overwrite confirmation dialog. This is a bit of a hack, and the only reason we need it is that the print dialog will load the last used path as the output file name, when printing to a file; this means that, in theory, it would be possible to press Print without selecting a file, and accidentally overwriting an existing file. It would be much simpler if we did not store the last used path, and always explicitly asked the user to select a file; this would avoid destructive actions, and would allow us to rely on the overwrite confirmation dialog right inside the file chooser. --- gtk/gtkprintunixdialog.c | 74 ++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 25 deletions(-) diff --git a/gtk/gtkprintunixdialog.c b/gtk/gtkprintunixdialog.c index eb9d008a04..6b9730f1a0 100644 --- a/gtk/gtkprintunixdialog.c +++ b/gtk/gtkprintunixdialog.c @@ -150,7 +150,7 @@ static void printer_status_cb (GtkPrintBackend *backend, GtkPrintUnixDialog *dialog); static void update_collate_icon (GtkToggleButton *toggle_button, GtkPrintUnixDialog *dialog); -static gboolean error_dialogs (GtkPrintUnixDialog *print_dialog, +static void error_dialogs (GtkPrintUnixDialog *print_dialog, gint print_dialog_response_id, gpointer data); static void emit_ok_response (GtkTreeView *tree_view, @@ -604,31 +604,42 @@ set_busy_cursor (GtkPrintUnixDialog *dialog, gtk_widget_set_cursor (widget, NULL); } +typedef struct { + GMainLoop *loop; + int response; +} ConfirmationData; + +static void +on_confirmation_dialog_response (GtkWidget *dialog, + int response, + gpointer user_data) +{ + ConfirmationData *data = user_data; + + data->response = response; + + g_main_loop_quit (data->loop); + + gtk_window_destroy (GTK_WINDOW (dialog)); +} + /* This function handles error messages before printing. */ -static gboolean +static void error_dialogs (GtkPrintUnixDialog *dialog, gint dialog_response_id, gpointer data) { - GtkPrinterOption *option = NULL; - GtkPrinter *printer = NULL; - GtkWindow *toplevel = NULL; - GFile *file = NULL; - gchar *basename = NULL; - gchar *dirname = NULL; - int response; - if (dialog != NULL && dialog_response_id == GTK_RESPONSE_OK) { - printer = gtk_print_unix_dialog_get_selected_printer (dialog); + GtkPrinter *printer = gtk_print_unix_dialog_get_selected_printer (dialog); if (printer != NULL) { if (dialog->request_details_tag || !gtk_printer_is_accepting_jobs (printer)) { g_signal_stop_emission_by_name (dialog, "response"); - return TRUE; + return; } /* Shows overwrite confirmation dialog in the case of printing @@ -636,18 +647,22 @@ error_dialogs (GtkPrintUnixDialog *dialog, */ if (gtk_printer_is_virtual (printer)) { - option = gtk_printer_option_set_lookup (dialog->options, - "gtk-main-page-custom-input"); + GtkPrinterOption *option = + gtk_printer_option_set_lookup (dialog->options, + "gtk-main-page-custom-input"); if (option != NULL && option->type == GTK_PRINTER_OPTION_TYPE_FILESAVE) { - file = g_file_new_for_uri (option->value); + GFile *file = g_file_new_for_uri (option->value); if (g_file_query_exists (file, NULL)) { - GFile *parent; GtkWidget *message_dialog; + GtkWindow *toplevel; + char *basename; + char *dirname; + GFile *parent; toplevel = get_toplevel (GTK_WIDGET (dialog)); @@ -682,19 +697,29 @@ error_dialogs (GtkPrintUnixDialog *dialog, gtk_window_group_add_window (gtk_window_get_group (toplevel), GTK_WINDOW (message_dialog)); - response = gtk_dialog_run (GTK_DIALOG (message_dialog)); + gtk_window_present (GTK_WINDOW (message_dialog)); - gtk_window_destroy (GTK_WINDOW (message_dialog)); + /* Block on the confirmation dialog until we have a response, + * so that we can stop the "response" signal emission on the + * print dialog + */ + ConfirmationData cdata; + + cdata.loop = g_main_loop_new (NULL, FALSE); + cdata.response = 0; + + g_signal_connect (message_dialog, "response", + G_CALLBACK (on_confirmation_dialog_response), + &cdata); + + g_main_loop_run (cdata.loop); + g_main_loop_unref (cdata.loop); g_free (dirname); g_free (basename); - if (response != GTK_RESPONSE_ACCEPT) - { - g_signal_stop_emission_by_name (dialog, "response"); - g_object_unref (file); - return TRUE; - } + if (cdata.response != GTK_RESPONSE_ACCEPT) + g_signal_stop_emission_by_name (dialog, "response"); } g_object_unref (file); @@ -702,7 +727,6 @@ error_dialogs (GtkPrintUnixDialog *dialog, } } } - return FALSE; } static void From d54b7dec94d5a756453aadcc59f51f509cf1049b Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 18:53:02 +0100 Subject: [PATCH 11/13] Remove gtk_dialog_run() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nested main loops are bad, as they introduce layers of complexity caused by the potential re-entrancy in the case of multiple event sources, like IPC, threads, etc. Additionally, the programming model they provide—stop the world while spinning a new loop—does not conform to the event-driven model employed by GTK. --- docs/reference/gtk/gtk4-sections.txt | 1 - gtk/gtkdialog.c | 172 +-------------------------- gtk/gtkdialog.h | 4 - 3 files changed, 2 insertions(+), 175 deletions(-) diff --git a/docs/reference/gtk/gtk4-sections.txt b/docs/reference/gtk/gtk4-sections.txt index 68c436cf84..1daaa1ad22 100644 --- a/docs/reference/gtk/gtk4-sections.txt +++ b/docs/reference/gtk/gtk4-sections.txt @@ -678,7 +678,6 @@ GtkDialogFlags GtkResponseType gtk_dialog_new gtk_dialog_new_with_buttons -gtk_dialog_run gtk_dialog_response gtk_dialog_add_button gtk_dialog_add_buttons diff --git a/gtk/gtkdialog.c b/gtk/gtkdialog.c index bab86685aa..f5c401311a 100644 --- a/gtk/gtkdialog.c +++ b/gtk/gtkdialog.c @@ -83,12 +83,6 @@ * a dialog receives a delete event, the #GtkDialog::response signal will * be emitted with a response ID of #GTK_RESPONSE_DELETE_EVENT. * - * If you want to block waiting for a dialog to return before returning - * control flow to your code, you can call gtk_dialog_run(). This function - * enters a recursive main loop and waits for the user to respond to the - * dialog, returning the response ID corresponding to the button the user - * clicked. - * * For the simple dialog in the following example, in reality you’d probably * use #GtkMessageDialog to save yourself some effort. But you’d need to * create the dialog contents manually if you had more than a simple message @@ -592,7 +586,6 @@ gtk_dialog_buildable_interface_init (GtkBuildableIface *iface) static gboolean gtk_dialog_close_request (GtkWindow *window) { - /* emit response signal, this will shut down the loop if we are in gtk_dialog_run */ gtk_dialog_response (GTK_DIALOG (window), GTK_RESPONSE_DELETE_EVENT); return GTK_WINDOW_CLASS (gtk_dialog_parent_class)->close_request (window); @@ -1011,9 +1004,8 @@ gtk_dialog_set_default_response (GtkDialog *dialog, * @response_id: response ID * * Emits the #GtkDialog::response signal with the given response ID. - * Used to indicate that the user has responded to the dialog in some way; - * typically either you or gtk_dialog_run() will be monitoring the - * ::response signal and take appropriate action. + * + * Used to indicate that the user has responded to the dialog in some way. **/ void gtk_dialog_response (GtkDialog *dialog, @@ -1027,166 +1019,6 @@ gtk_dialog_response (GtkDialog *dialog, response_id); } -typedef struct -{ - GtkDialog *dialog; - gint response_id; - GMainLoop *loop; - gboolean destroyed; -} RunInfo; - -static void -shutdown_loop (RunInfo *ri) -{ - if (g_main_loop_is_running (ri->loop)) - g_main_loop_quit (ri->loop); -} - -static void -run_unmap_handler (GtkDialog *dialog, gpointer data) -{ - RunInfo *ri = data; - - shutdown_loop (ri); -} - -static void -run_response_handler (GtkDialog *dialog, - gint response_id, - gpointer data) -{ - RunInfo *ri; - - ri = data; - - ri->response_id = response_id; - - shutdown_loop (ri); -} - -static void -run_destroy_handler (GtkDialog *dialog, gpointer data) -{ - RunInfo *ri = data; - - /* shutdown_loop will be called by run_unmap_handler */ - - ri->destroyed = TRUE; -} - -/** - * gtk_dialog_run: - * @dialog: a #GtkDialog - * - * Blocks in a recursive main loop until the @dialog either emits the - * #GtkDialog::response signal, or is destroyed. If the dialog is - * destroyed during the call to gtk_dialog_run(), gtk_dialog_run() returns - * #GTK_RESPONSE_NONE. Otherwise, it returns the response ID from the - * ::response signal emission. - * - * Before entering the recursive main loop, gtk_dialog_run() calls - * gtk_widget_show() on the dialog for you. Note that you still - * need to show any children of the dialog yourself. - * - * During gtk_dialog_run(), the default behavior of delete events - * is disabled; if the dialog receives a delete event, it will not be - * destroyed as windows usually are, and gtk_dialog_run() will return - * #GTK_RESPONSE_DELETE_EVENT. Also, during gtk_dialog_run() the dialog - * will be modal. You can force gtk_dialog_run() to return at any time by - * calling gtk_dialog_response() to emit the ::response signal. Destroying - * the dialog during gtk_dialog_run() is a very bad idea, because your - * post-run code won’t know whether the dialog was destroyed or not. - * - * After gtk_dialog_run() returns, you are responsible for hiding or - * destroying the dialog if you wish to do so. - * - * Typical usage of this function might be: - * |[ - * GtkWidget *dialog = gtk_dialog_new (); - * // Set up dialog... - * - * int result = gtk_dialog_run (GTK_DIALOG (dialog)); - * switch (result) - * { - * case GTK_RESPONSE_ACCEPT: - * // do_application_specific_something (); - * break; - * default: - * // do_nothing_since_dialog_was_cancelled (); - * break; - * } - * gtk_window_destroy (dialog); - * ]| - * - * Note that even though the recursive main loop gives the effect of a - * modal dialog (it prevents the user from interacting with other - * windows in the same window group while the dialog is run), callbacks - * such as timeouts, IO channel watches, DND drops, etc, will - * be triggered during a gtk_dialog_run() call. - * - * Returns: response ID - **/ -gint -gtk_dialog_run (GtkDialog *dialog) -{ - RunInfo ri = { NULL, GTK_RESPONSE_NONE, NULL, FALSE }; - gboolean was_modal; - gboolean was_hide_on_close; - gulong response_handler; - gulong unmap_handler; - gulong destroy_handler; - - g_return_val_if_fail (GTK_IS_DIALOG (dialog), -1); - - g_object_ref (dialog); - - was_modal = gtk_window_get_modal (GTK_WINDOW (dialog)); - gtk_window_set_modal (GTK_WINDOW (dialog), TRUE); - was_hide_on_close = gtk_window_get_hide_on_close (GTK_WINDOW (dialog)); - gtk_window_set_hide_on_close (GTK_WINDOW (dialog), TRUE); - - if (!gtk_widget_get_visible (GTK_WIDGET (dialog))) - gtk_widget_show (GTK_WIDGET (dialog)); - - response_handler = - g_signal_connect (dialog, - "response", - G_CALLBACK (run_response_handler), - &ri); - - unmap_handler = - g_signal_connect (dialog, - "unmap", - G_CALLBACK (run_unmap_handler), - &ri); - - destroy_handler = - g_signal_connect (dialog, - "destroy", - G_CALLBACK (run_destroy_handler), - &ri); - - ri.loop = g_main_loop_new (NULL, FALSE); - g_main_loop_run (ri.loop); - g_main_loop_unref (ri.loop); - - ri.loop = NULL; - - if (!ri.destroyed) - { - gtk_window_set_modal (GTK_WINDOW (dialog), was_modal); - gtk_window_set_hide_on_close (GTK_WINDOW (dialog), was_hide_on_close); - - g_signal_handler_disconnect (dialog, response_handler); - g_signal_handler_disconnect (dialog, unmap_handler); - g_signal_handler_disconnect (dialog, destroy_handler); - } - - g_object_unref (dialog); - - return ri.response_id; -} - /** * gtk_dialog_get_widget_for_response: * @dialog: a #GtkDialog diff --git a/gtk/gtkdialog.h b/gtk/gtkdialog.h index 056253041d..b653017661 100644 --- a/gtk/gtkdialog.h +++ b/gtk/gtkdialog.h @@ -176,10 +176,6 @@ GDK_AVAILABLE_IN_ALL void gtk_dialog_response (GtkDialog *dialog, gint response_id); -/* Returns response_id */ -GDK_AVAILABLE_IN_ALL -gint gtk_dialog_run (GtkDialog *dialog); - GDK_AVAILABLE_IN_ALL GtkWidget * gtk_dialog_get_content_area (GtkDialog *dialog); GDK_AVAILABLE_IN_ALL From e8c4b8338f0f34d09050c3d4096d88498ce4172d Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Thu, 30 Apr 2020 16:08:37 +0100 Subject: [PATCH 12/13] docs: Mention blocking functions in the migration guide Link to how to make a dialog modal, and to the response signal. --- docs/reference/gtk/migrating-3to4.xml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/reference/gtk/migrating-3to4.xml b/docs/reference/gtk/migrating-3to4.xml index ee5bc5ecf9..32fa09d8bb 100644 --- a/docs/reference/gtk/migrating-3to4.xml +++ b/docs/reference/gtk/migrating-3to4.xml @@ -1254,5 +1254,24 @@ emitted. + +
+ Stop using blocking dialog functions + + GtkDialog, GtkNativeDialog, and GtkPrintOperation removed their + blocking API using nested main loops. Nested main loops present + re-entrancy issues and other hard to debug issues when coupled + with other event sources (IPC, accessibility, network operations) + that are not under the toolkit or the application developer's + control. Additionally, "stop-the-world" functions do not fit + the event-driven programming model of GTK. + + + You can replace calls to gtk_dialog_run() + by specifying that the #GtkDialog must be modal using + gtk_window_set_modal() or the %GTK_DIALOG_MODAL flag, and + connecting to the #GtkDialog::response signal. + +
From 717d4abebb1ca3f56ddd81f1ef57eb5166006774 Mon Sep 17 00:00:00 2001 From: Emmanuele Bassi Date: Fri, 1 May 2020 11:08:41 +0100 Subject: [PATCH 13/13] Do not release the GFile prematurely Otherwise we won't be able to access it to get the URI for the GtkPrinterOption. This fixes a regression introduced in commit 5f070ff2333. --- gtk/gtkprinteroptionwidget.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/gtk/gtkprinteroptionwidget.c b/gtk/gtkprinteroptionwidget.c index a866abe85d..a63d614806 100644 --- a/gtk/gtkprinteroptionwidget.c +++ b/gtk/gtkprinteroptionwidget.c @@ -480,8 +480,6 @@ dialog_response_callback (GtkDialog *dialog, g_free (filename_short); g_object_unref (info); } - - g_object_unref (new_location); } gtk_window_destroy (GTK_WINDOW (dialog)); @@ -544,11 +542,10 @@ filesave_choose_cb (GtkWidget *button, } g_signal_connect (dialog, "response", - G_CALLBACK (dialog_response_callback), widget); + G_CALLBACK (dialog_response_callback), + widget); gtk_window_set_modal (GTK_WINDOW (dialog), TRUE); - G_GNUC_BEGIN_IGNORE_DEPRECATIONS gtk_window_present (GTK_WINDOW (dialog)); - G_GNUC_END_IGNORE_DEPRECATIONS } static gchar *