From e8dcac5e5e1da9eb69210cffde00f896858793ef Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Mon, 28 Mar 2011 21:54:55 -0500 Subject: Don't unref a variant we didn't have a ref to on error --- libdbusmenu-gtk/menuitem.c | 1 - 1 file changed, 1 deletion(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/menuitem.c b/libdbusmenu-gtk/menuitem.c index ca2bc3e..f02e171 100644 --- a/libdbusmenu-gtk/menuitem.c +++ b/libdbusmenu-gtk/menuitem.c @@ -287,7 +287,6 @@ dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * ke if (g_variant_n_children(wrapper) != 1) { g_warning("Unable to parse shortcut, too many keys"); - g_variant_unref(wrapper); return; } -- cgit v1.2.3 From 980cc3a10a166cd99f9369712489f849732a63e0 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Mon, 28 Mar 2011 22:06:44 -0500 Subject: Putting in some protections from NULL parameters. --- libdbusmenu-gtk/menuitem.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/menuitem.c b/libdbusmenu-gtk/menuitem.c index f02e171..f6c50b6 100644 --- a/libdbusmenu-gtk/menuitem.c +++ b/libdbusmenu-gtk/menuitem.c @@ -275,6 +275,17 @@ dbusmenu_menuitem_property_set_shortcut_menuitem (DbusmenuMenuitem * menuitem, c void dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * key, GdkModifierType * modifier) { + guint dummykey; + GdkModifierType dummymodifier; + + if (key == NULL) { + key = &dummykey; + } + + if (modifier == NULL) { + modifier = &dummymodifier; + } + *key = 0; *modifier = 0; -- cgit v1.2.3 From de83592f2bb7a825b4164b68e9a70499416aeb4b Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Mon, 28 Mar 2011 22:11:25 -0500 Subject: Use loop instead of next --- libdbusmenu-gtk/menuitem.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/menuitem.c b/libdbusmenu-gtk/menuitem.c index f6c50b6..9987cef 100644 --- a/libdbusmenu-gtk/menuitem.c +++ b/libdbusmenu-gtk/menuitem.c @@ -305,7 +305,7 @@ dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * ke g_variant_iter_init(&iter, g_variant_get_child_value(wrapper, 0)); gchar * string; - while(g_variant_iter_next(&iter, "s", &string)) { + while(g_variant_iter_loop(&iter, "s", &string)) { if (g_strcmp0(string, DBUSMENU_MENUITEM_SHORTCUT_CONTROL) == 0) { *modifier |= GDK_CONTROL_MASK; } else if (g_strcmp0(string, DBUSMENU_MENUITEM_SHORTCUT_ALT) == 0) { @@ -318,8 +318,6 @@ dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * ke GdkModifierType tempmod; gtk_accelerator_parse(string, key, &tempmod); } - - g_free(string); } return; -- cgit v1.2.3 From ad638a2aed595ae3695c0068e3447c811fa91207 Mon Sep 17 00:00:00 2001 From: Ted Gould Date: Tue, 29 Mar 2011 08:46:45 -0500 Subject: Split out into another variable --- libdbusmenu-gtk/menuitem.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/menuitem.c b/libdbusmenu-gtk/menuitem.c index 9987cef..0f511bc 100644 --- a/libdbusmenu-gtk/menuitem.c +++ b/libdbusmenu-gtk/menuitem.c @@ -302,7 +302,8 @@ dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * ke } GVariantIter iter; - g_variant_iter_init(&iter, g_variant_get_child_value(wrapper, 0)); + GVariant * child = g_variant_get_child_value(wrapper, 0); + g_variant_iter_init(&iter, child); gchar * string; while(g_variant_iter_loop(&iter, "s", &string)) { @@ -320,6 +321,8 @@ dbusmenu_menuitem_property_get_shortcut (DbusmenuMenuitem * menuitem, guint * ke } } + g_variant_unref(child); + return; } -- cgit v1.2.3 From ccf2e684c6a2f8115dbe9544f43412986acaee79 Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 30 Mar 2011 13:20:52 +0100 Subject: Revert the last commit and handle the same problem in the parser instead --- libdbusmenu-gtk/parser.c | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index a7f90a2..9fc8f53 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -454,7 +454,7 @@ construct_dbusmenu_for_widget (GtkWidget * widget) gboolean visible = FALSE; gboolean sensitive = FALSE; - if (GTK_IS_SEPARATOR_MENU_ITEM (widget)) + if (GTK_IS_SEPARATOR_MENU_ITEM (widget) || !find_menu_label (widget)) { dbusmenu_menuitem_property_set (mi, "type", @@ -891,6 +891,26 @@ widget_notify_cb (GtkWidget *widget, } else if (pspec->name == g_intern_static_string ("label")) { + if (!g_strcmp0 (dbusmenu_menuitem_property_get (child, DBUSMENU_MENUITEM_PROP_TYPE), "separator")) + { + /* GtkMenuItem's can start life as a separator if they have no child + * GtkLabel. In this case, we need to convert the DbusmenuMenuitem from + * a separator to a normal menuitem if the application adds a label + */ + GtkWidget *label = find_menu_label (widget); + /* This should never fail */ + g_return_if_fail (label != NULL); + + dbusmenu_menuitem_property_remove (child, DBUSMENU_MENUITEM_PROP_TYPE); + ParserData *pdata = g_object_get_data (G_OBJECT (widget), PARSER_DATA); + pdata->label = label; + g_signal_connect (G_OBJECT (label), + "notify", + G_CALLBACK (label_notify_cb), + child); + g_object_add_weak_pointer(G_OBJECT (label), (gpointer*)&pdata->label); + } + dbusmenu_menuitem_property_set (child, DBUSMENU_MENUITEM_PROP_LABEL, g_value_get_string (&prop_value)); -- cgit v1.2.3 From f4bfda45834ee6f3983be3e3d7fd9c0e8fb7512e Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 30 Mar 2011 13:31:33 +0100 Subject: Remove the now unneeded null pointer check on label in construct_dbusmenu_for_widget. Also, don't use a strcmp in widget_notify_cb for checking if the menuitem is a separator. Just do a null pointer check on pdata->label instead --- libdbusmenu-gtk/parser.c | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 9fc8f53..fc9a3fe 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -512,21 +512,18 @@ construct_dbusmenu_for_widget (GtkWidget * widget) GtkWidget *label = find_menu_label (widget); - if (label) - { - // Sometimes, an app will directly find and modify the label - // (like empathy), so watch the label especially for that. - gchar * text = sanitize_label (GTK_LABEL (label)); - dbusmenu_menuitem_property_set (mi, "label", text); - g_free (text); - - pdata->label = label; - g_signal_connect (G_OBJECT (label), - "notify", - G_CALLBACK (label_notify_cb), - mi); - g_object_add_weak_pointer(G_OBJECT (label), (gpointer*)&pdata->label); - } + // Sometimes, an app will directly find and modify the label + // (like empathy), so watch the label especially for that. + gchar * text = sanitize_label (GTK_LABEL (label)); + dbusmenu_menuitem_property_set (mi, "label", text); + g_free (text); + + pdata->label = label; + g_signal_connect (G_OBJECT (label), + "notify", + G_CALLBACK (label_notify_cb), + mi); + g_object_add_weak_pointer(G_OBJECT (label), (gpointer*)&pdata->label); if (GTK_IS_ACTIVATABLE (widget)) { @@ -891,7 +888,8 @@ widget_notify_cb (GtkWidget *widget, } else if (pspec->name == g_intern_static_string ("label")) { - if (!g_strcmp0 (dbusmenu_menuitem_property_get (child, DBUSMENU_MENUITEM_PROP_TYPE), "separator")) + ParserData *pdata = g_object_get_data (G_OBJECT (widget), PARSER_DATA); + if (!pdata->label) { /* GtkMenuItem's can start life as a separator if they have no child * GtkLabel. In this case, we need to convert the DbusmenuMenuitem from @@ -902,7 +900,6 @@ widget_notify_cb (GtkWidget *widget, g_return_if_fail (label != NULL); dbusmenu_menuitem_property_remove (child, DBUSMENU_MENUITEM_PROP_TYPE); - ParserData *pdata = g_object_get_data (G_OBJECT (widget), PARSER_DATA); pdata->label = label; g_signal_connect (G_OBJECT (label), "notify", -- cgit v1.2.3 From 25b78133a2075185fe2d94925f446927c1b85b9a Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 30 Mar 2011 19:43:02 +0100 Subject: - Don't change the type of existing menu items in the server. This isn't handled in the client too well - Handle a GtkMenuItem's GtkLabel being removed too --- libdbusmenu-gtk/parser.c | 74 ++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 62 insertions(+), 12 deletions(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index fc9a3fe..037c362 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -756,12 +756,43 @@ find_menu_label (GtkWidget *widget) return label; } +static gboolean +recreate_menu_item_in_idle_cb (gpointer data) +{ + DbusmenuMenuitem * child = (DbusmenuMenuitem *)data; + DbusmenuMenuitem * parent = dbusmenu_menuitem_get_parent (child); + g_object_unref (child); + if (parent == NULL) + { + return FALSE; + } + ParserData * pdata = g_object_get_data (G_OBJECT (child), PARSER_DATA); + /* Keep a pointer to the GtkMenuItem, as pdata->widget might be + * invalidated when we delete the DbusmenuMenuitem + */ + GtkWidget * menuitem = pdata->widget; + + dbusmenu_menuitem_child_delete (parent, child); + + RecurseContext recurse = {0}; + recurse.toplevel = gtk_widget_get_toplevel(menuitem); + recurse.parent = parent; + + parse_menu_structure_helper(menuitem, &recurse); + + return FALSE; +} + static void label_notify_cb (GtkWidget *widget, GParamSpec *pspec, gpointer data) { DbusmenuMenuitem *child = (DbusmenuMenuitem *)data; + GValue prop_value = {0}; + + g_value_init (&prop_value, pspec->value_type); + g_object_get_property (G_OBJECT (widget), pspec->name, &prop_value); if (pspec->name == g_intern_static_string ("label")) { @@ -771,6 +802,24 @@ label_notify_cb (GtkWidget *widget, text); g_free (text); } + else if (pspec->name == g_intern_static_string ("parent")) + { + if (GTK_WIDGET (g_value_get_object (&prop_value)) == NULL) + { + /* This label is being removed from its GtkMenuItem. The + * menuitem becomes a separator now. As the client doesn't handle + * changing types so well, we remove the current DbusmenuMenuitem + * and add a new one. + * + * Note, we have to defer this to idle, as we are called before + * bin->child member of our old parent is invalidated. If we go ahead + * and call parse_menu_structure_helper now, the GtkMenuItem will + * still appear to have a label and we never convert it to a separator + */ + g_object_ref (child); + g_idle_add ((GSourceFunc)recreate_menu_item_in_idle_cb, child); + } + } } static void @@ -888,24 +937,25 @@ widget_notify_cb (GtkWidget *widget, } else if (pspec->name == g_intern_static_string ("label")) { - ParserData *pdata = g_object_get_data (G_OBJECT (widget), PARSER_DATA); + ParserData *pdata = g_object_get_data (G_OBJECT (child), PARSER_DATA); if (!pdata->label) { /* GtkMenuItem's can start life as a separator if they have no child * GtkLabel. In this case, we need to convert the DbusmenuMenuitem from - * a separator to a normal menuitem if the application adds a label + * a separator to a normal menuitem if the application adds a label. + * As changing types isn't handled too well by the client, we delete + * this menuitem for now and then recreate it */ - GtkWidget *label = find_menu_label (widget); - /* This should never fail */ - g_return_if_fail (label != NULL); + DbusmenuMenuitem * parent = dbusmenu_menuitem_get_parent (child); + dbusmenu_menuitem_child_delete (parent, child); - dbusmenu_menuitem_property_remove (child, DBUSMENU_MENUITEM_PROP_TYPE); - pdata->label = label; - g_signal_connect (G_OBJECT (label), - "notify", - G_CALLBACK (label_notify_cb), - child); - g_object_add_weak_pointer(G_OBJECT (label), (gpointer*)&pdata->label); + RecurseContext recurse = {0}; + recurse.toplevel = gtk_widget_get_toplevel(widget); + recurse.parent = parent; + + parse_menu_structure_helper(widget, &recurse); + + return; } dbusmenu_menuitem_property_set (child, -- cgit v1.2.3 From 44fb71ec9b70ad3d2dc7bcb6d35e3f6c7e69370e Mon Sep 17 00:00:00 2001 From: Chris Coulson Date: Wed, 30 Mar 2011 19:58:47 +0100 Subject: Remove some code duplication introduced in this branch --- libdbusmenu-gtk/parser.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'libdbusmenu-gtk') diff --git a/libdbusmenu-gtk/parser.c b/libdbusmenu-gtk/parser.c index 037c362..e68c286 100644 --- a/libdbusmenu-gtk/parser.c +++ b/libdbusmenu-gtk/parser.c @@ -756,15 +756,13 @@ find_menu_label (GtkWidget *widget) return label; } -static gboolean -recreate_menu_item_in_idle_cb (gpointer data) +static void +recreate_menu_item (DbusmenuMenuitem * parent, DbusmenuMenuitem * child) { - DbusmenuMenuitem * child = (DbusmenuMenuitem *)data; - DbusmenuMenuitem * parent = dbusmenu_menuitem_get_parent (child); - g_object_unref (child); if (parent == NULL) { - return FALSE; + /* We need a parent */ + return; } ParserData * pdata = g_object_get_data (G_OBJECT (child), PARSER_DATA); /* Keep a pointer to the GtkMenuItem, as pdata->widget might be @@ -779,7 +777,15 @@ recreate_menu_item_in_idle_cb (gpointer data) recurse.parent = parent; parse_menu_structure_helper(menuitem, &recurse); +} +static gboolean +recreate_menu_item_in_idle_cb (gpointer data) +{ + DbusmenuMenuitem * child = (DbusmenuMenuitem *)data; + DbusmenuMenuitem * parent = dbusmenu_menuitem_get_parent (child); + g_object_unref (child); + recreate_menu_item (parent, child); return FALSE; } @@ -947,14 +953,7 @@ widget_notify_cb (GtkWidget *widget, * this menuitem for now and then recreate it */ DbusmenuMenuitem * parent = dbusmenu_menuitem_get_parent (child); - dbusmenu_menuitem_child_delete (parent, child); - - RecurseContext recurse = {0}; - recurse.toplevel = gtk_widget_get_toplevel(widget); - recurse.parent = parent; - - parse_menu_structure_helper(widget, &recurse); - + recreate_menu_item (parent, child); return; } -- cgit v1.2.3