From 3b3ad3d6cc8b34dd67f0431b16c1cb5a63135951 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 2 May 2022 14:36:26 -0700 Subject: [PATCH 1/4] widget: add gtk_widget_set_action_parent() This adds a new function to the 4.8 ABI that allows setting an action parent for a widget. The action parent affects the action muxing so that instead of using the widgets direct ancestor, an alternate widget's action muxer may be used. You may not set an action parent for a widget that is a direct descendant of your widget as that would cause cycles in action resolution. You might find this API useful for situations where you want menus in headerbars to route through action muxers for the current document as well as toolbars or sidebars. Fixes #4860 --- gtk/gtkactionmuxer.c | 18 +++++++++++ gtk/gtkwidget.c | 69 ++++++++++++++++++++++++++++++++++++++++-- gtk/gtkwidget.h | 3 ++ gtk/gtkwidgetprivate.h | 1 + 4 files changed, 89 insertions(+), 2 deletions(-) diff --git a/gtk/gtkactionmuxer.c b/gtk/gtkactionmuxer.c index 06660ddf89d..ce7d66889e0 100644 --- a/gtk/gtkactionmuxer.c +++ b/gtk/gtkactionmuxer.c @@ -1356,6 +1356,23 @@ gtk_action_muxer_get_parent (GtkActionMuxer *muxer) return muxer->parent; } +static gboolean +muxer_will_cycle (GtkActionMuxer *muxer, + GtkActionMuxer *parent) +{ + GtkActionMuxer *ancestor; + + for (ancestor = parent; + ancestor != NULL; + ancestor = gtk_action_muxer_get_parent (ancestor)) + { + if (ancestor == muxer) + return TRUE; + } + + return FALSE; +} + /*< private > * gtk_action_muxer_set_parent: * @muxer: a `GtkActionMuxer` @@ -1369,6 +1386,7 @@ gtk_action_muxer_set_parent (GtkActionMuxer *muxer, { g_return_if_fail (GTK_IS_ACTION_MUXER (muxer)); g_return_if_fail (parent == NULL || GTK_IS_ACTION_MUXER (parent)); + g_return_if_fail (parent == NULL || !muxer_will_cycle (muxer, parent)); if (muxer->parent == parent) return; diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c index afe7e9224e8..f3c51db2683 100644 --- a/gtk/gtkwidget.c +++ b/gtk/gtkwidget.c @@ -7390,6 +7390,8 @@ gtk_widget_dispose (GObject *object) GSList *sizegroups; GtkATContext *at_context; + g_clear_object (&priv->action_parent); + if (priv->muxer != NULL) g_object_run_dispose (G_OBJECT (priv->muxer)); @@ -10808,13 +10810,20 @@ void _gtk_widget_update_parent_muxer (GtkWidget *widget) { GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget); + GtkActionMuxer *parent_muxer; GtkWidget *child; if (priv->muxer == NULL) return; - gtk_action_muxer_set_parent (priv->muxer, - gtk_widget_get_parent_muxer (widget, FALSE)); + if (priv->action_parent != NULL && + !gtk_widget_is_ancestor (priv->action_parent, widget)) + parent_muxer = _gtk_widget_get_action_muxer (priv->action_parent, FALSE); + else + parent_muxer = gtk_widget_get_parent_muxer (widget, FALSE); + + gtk_action_muxer_set_parent (priv->muxer, parent_muxer); + for (child = gtk_widget_get_first_child (widget); child != NULL; child = gtk_widget_get_next_sibling (child)) @@ -12934,3 +12943,59 @@ gtk_widget_set_active_state (GtkWidget *widget, gtk_widget_unset_state_flags (widget, GTK_STATE_FLAG_ACTIVE); } } + +/** + * gtk_widget_set_action_parent: + * @widget: a [class@Gtk.Widget] + * @action_parent: (nullable): a [class@Gtk.Widget] + * + * Sets the action parent for @widget. + * + * Actions will resolve through @action_parent for @widget and all of + * it's descendants unless otherwise specified with + * [method@Gtk.Widget.set_action_parent]. + * + * Setting an action parent can be useful when you want actions within + * a menu or toolbar to resolve through a document widget. + * + * To unset an action parent, use `NULL` for @action_parent and the widget + * will resume using the parent widget as the action parent. + * + * It is a programming error to set an action parent which will cause a + * cycle to occur. + * + * Since: 4.8 + */ +void +gtk_widget_set_action_parent (GtkWidget *widget, + GtkWidget *action_parent) +{ + GtkWidgetPrivate *priv = gtk_widget_get_instance_private (widget); + GtkActionMuxer *muxer; + GtkActionMuxer *parent_muxer; + + g_return_if_fail (GTK_IS_WIDGET (widget)); + g_return_if_fail (action_parent != widget); + g_return_if_fail (!action_parent || GTK_IS_WIDGET (action_parent)); + g_return_if_fail (!action_parent || !gtk_widget_is_ancestor (action_parent, widget)); + + muxer = _gtk_widget_get_action_muxer (widget, FALSE); + + if (action_parent == NULL) + { + if (muxer != NULL) + { + parent_muxer = gtk_widget_get_parent_muxer (widget, FALSE); + gtk_action_muxer_set_parent (muxer, parent_muxer); + } + } + else + { + if (muxer == NULL) + muxer = _gtk_widget_get_action_muxer (widget, TRUE); + parent_muxer = _gtk_widget_get_action_muxer (action_parent, TRUE); + gtk_action_muxer_set_parent (muxer, parent_muxer); + } + + g_set_object (&priv->action_parent, action_parent); +} diff --git a/gtk/gtkwidget.h b/gtk/gtkwidget.h index 0788fcdd817..1a9793f2969 100644 --- a/gtk/gtkwidget.h +++ b/gtk/gtkwidget.h @@ -924,6 +924,9 @@ char ** gtk_widget_get_css_classes (GtkWidget *widget); GDK_AVAILABLE_IN_ALL void gtk_widget_set_css_classes (GtkWidget *widget, const char **classes); +GDK_AVAILABLE_IN_4_8 +void gtk_widget_set_action_parent (GtkWidget *widget, + GtkWidget *action_parent); diff --git a/gtk/gtkwidgetprivate.h b/gtk/gtkwidgetprivate.h index e1e336e6e65..b8f17c8f235 100644 --- a/gtk/gtkwidgetprivate.h +++ b/gtk/gtkwidgetprivate.h @@ -185,6 +185,7 @@ struct _GtkWidgetPrivate GtkListListModel *children_observer; GtkListListModel *controller_observer; GtkActionMuxer *muxer; + GtkWidget *action_parent; GtkWidget *focus_child; -- GitLab From f9c2768313472e248bea902546af076519f2921b Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 2 May 2022 14:55:55 -0700 Subject: [PATCH 2/4] actionmuxer: set handler ids initially to zero These were getting created with possible non-zero values and then inserted into a hashtable where the readers may not know the state of the group. Ensure those values are set to zero until we assign them below. --- gtk/gtkactionmuxer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtk/gtkactionmuxer.c b/gtk/gtkactionmuxer.c index ce7d66889e0..6cbf7421dad 100644 --- a/gtk/gtkactionmuxer.c +++ b/gtk/gtkactionmuxer.c @@ -1269,7 +1269,7 @@ gtk_action_muxer_insert (GtkActionMuxer *muxer, if (!muxer->groups) muxer->groups = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, gtk_action_muxer_free_group); - group = g_slice_new (Group); + group = g_slice_new0 (Group); group->muxer = muxer; group->group = g_object_ref (action_group); group->prefix = g_strdup (prefix); -- GitLab From 3e76026ab462c219334d96a70076b7fdc3a7a9a2 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 2 May 2022 15:33:34 -0700 Subject: [PATCH 3/4] actionmuxer: check for observer before unregistering This can happen if the group can be resolved even when doing the initial registration of an action as observer will not yet be in the GSList of watchers (and therefore has no weak references). Fixes a warning like the following: g_object_weak_unref: couldn't find weak ref --- gtk/gtkactionmuxer.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/gtk/gtkactionmuxer.c b/gtk/gtkactionmuxer.c index 6cbf7421dad..46414e228e6 100644 --- a/gtk/gtkactionmuxer.c +++ b/gtk/gtkactionmuxer.c @@ -991,13 +991,15 @@ gtk_action_muxer_unregister_observer (GtkActionObservable *observable, GtkActionObserver *observer) { GtkActionMuxer *muxer = GTK_ACTION_MUXER (observable); - Action *action; + Action *action = find_observers (muxer, name); - action = find_observers (muxer, name); if (action) { - g_object_weak_unref (G_OBJECT (observer), gtk_action_muxer_weak_notify, action); - gtk_action_muxer_unregister_internal (action, observer); + if (g_slist_find (action->watchers, observer) != NULL) + { + g_object_weak_unref (G_OBJECT (observer), gtk_action_muxer_weak_notify, action); + gtk_action_muxer_unregister_internal (action, observer); + } } } -- GitLab From b584250777b12f3905e52e0878a9c9140fc6e198 Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Mon, 2 May 2022 15:33:57 -0700 Subject: [PATCH 4/4] actionmuxer: add test for action parent This tests that the infrastructure for action muxer parents is working by activating an action on a widget for which the action is defined on a widget not in the ancestry. --- testsuite/gtk/action.c | 106 ++++++++++++++++++++++++++++++++++++++ testsuite/gtk/meson.build | 2 +- 2 files changed, 107 insertions(+), 1 deletion(-) diff --git a/testsuite/gtk/action.c b/testsuite/gtk/action.c index 95bb071d4b9..7024f353411 100644 --- a/testsuite/gtk/action.c +++ b/testsuite/gtk/action.c @@ -15,6 +15,8 @@ */ #include +#include "gtkwidgetprivate.h" + static void activate (GSimpleAction *action, GVariant *parameter, @@ -717,6 +719,109 @@ test_enabled (void) g_object_unref (g_object_ref_sink (text)); } +static void +test_action_parent_action1 (GSimpleAction *action, + GVariant *param, + gpointer user_data) +{ + guint *count = user_data; + (*count)++; +} + +static void +test_action_parent_action2 (GSimpleAction *action, + GVariant *param, + gpointer user_data) +{ + g_simple_action_set_state (action, param); +} + +static void +test_action_parent (void) +{ + static const GActionEntry test_actions[] = { + { "action1", test_action_parent_action1 }, + { "action2", test_action_parent_action2, "s", "'initial'", test_action_parent_action2 }, + }; + GSimpleActionGroup *group; + GAction *action2; + GtkWidget *window; + GtkWidget *header; + GtkWidget *content; + GtkWidget *label1; + GtkWidget *label2; + GVariant *state = NULL; + const GVariantType *state_type = NULL; + GtkActionMuxer *muxer; + guint count = 0; + + window = g_object_new (GTK_TYPE_WINDOW, NULL); + g_object_ref_sink (window); + + header = g_object_new (GTK_TYPE_BUTTON, NULL); + content = g_object_new (GTK_TYPE_BOX, NULL); + label1 = g_object_new (GTK_TYPE_LABEL, NULL); + label2 = g_object_new (GTK_TYPE_LABEL, NULL); + gtk_box_append (GTK_BOX (content), label1); + gtk_box_append (GTK_BOX (content), label2); + gtk_window_set_titlebar (GTK_WINDOW (window), header); + gtk_window_set_child (GTK_WINDOW (window), content); + group = g_simple_action_group_new (); + g_action_map_add_action_entries (G_ACTION_MAP (group), + test_actions, + G_N_ELEMENTS (test_actions), + &count); + action2 = g_action_map_lookup_action (G_ACTION_MAP (group), "action2"); + + gtk_widget_insert_action_group (content, "test", G_ACTION_GROUP (group)); + gtk_widget_activate_action (label1, "test.action1", NULL); + g_assert_cmpint (count, ==, 1); + + gtk_widget_activate_action (header, "test.action1", NULL); + g_assert_cmpint (count, ==, 1); + + gtk_widget_set_action_parent (header, label1); + gtk_widget_activate_action (header, "test.action1", NULL); + g_assert_cmpint (count, ==, 2); + + gtk_widget_activate_action (header, "test.action2", "s", "changed"); + g_assert_cmpstr ("changed", ==, g_variant_get_string (g_action_get_state (action2), NULL)); + muxer = _gtk_widget_get_action_muxer (header, FALSE); + gtk_action_muxer_query_action (muxer, "test.action2", NULL, NULL, &state_type, NULL, &state); + g_assert_nonnull (state_type); + g_assert_nonnull (state); + g_assert_cmpstr ((const char *)state_type, ==, "s"); + g_assert_cmpstr ("changed", ==, g_variant_get_string (state, NULL)); + g_variant_unref (state); + + gtk_widget_set_action_parent (header, label2); + gtk_widget_activate_action (header, "test.action1", NULL); + g_assert_cmpint (count, ==, 3); + + gtk_widget_set_action_parent (header, NULL); + gtk_widget_activate_action (header, "test.action1", NULL); + gtk_widget_activate_action (header, "test.action2", "s", "third"); + g_assert_cmpint (count, ==, 3); + g_assert_cmpstr ("changed", ==, g_variant_get_string (g_action_get_state (action2), NULL)); + + gtk_widget_set_action_parent (label2, header); + gtk_widget_activate_action (label2, "test.action1", NULL); + g_assert_cmpint (count, ==, 3); + + gtk_widget_insert_action_group (content, "test", NULL); + gtk_widget_activate_action (label1, "test.action1", NULL); + gtk_widget_activate_action (header, "test.action1", NULL); + g_assert_cmpint (count, ==, 3); + + gtk_widget_activate_action (label2, "test.action2", "s", "third"); + g_assert_cmpstr ("changed", ==, g_variant_get_string (g_action_get_state (action2), NULL)); + + gtk_window_destroy (GTK_WINDOW (window)); + + g_assert_finalize_object (group); + g_assert_finalize_object (window); +} + int main (int argc, char *argv[]) @@ -732,6 +837,7 @@ main (int argc, g_test_add_func ("/action/overlap2", test_overlap2); g_test_add_func ("/action/introspection", test_introspection); g_test_add_func ("/action/enabled", test_enabled); + g_test_add_func ("/action/action_parent", test_action_parent); return g_test_run(); } diff --git a/testsuite/gtk/meson.build b/testsuite/gtk/meson.build index 3324664cac6..da513ef22b3 100644 --- a/testsuite/gtk/meson.build +++ b/testsuite/gtk/meson.build @@ -23,7 +23,6 @@ tests = [ { 'name': 'accel' }, # sadly, mesons xfail support seems busted # { 'name': 'accessor-apis' }, - { 'name': 'action' }, { 'name': 'adjustment' }, { 'name': 'bitset' }, { @@ -105,6 +104,7 @@ tests = [ # Tests that test private apis and therefore are linked against libgtk-4.a internal_tests = [ + { 'name': 'action' }, { 'name': 'bitmask' }, { 'name': 'composetable', -- GitLab