Merge lp:~mitya57/indicator-applet/unparent-label into lp:indicator-applet

[Dmitry Shachnev]

Fix assertion error which prevented indicator-applet-appmenu from functioning.

This fixes indicator-applet-appmenu package, before this any attempt to add menu to panel was failing with:

 CRITICAL: Gtk - gtk_box_pack: assertion 'gtk_widget_get_parent (child) == NULL' failed

Fixed by calling gtk_widget_unparent() before calling gtk_box_pack_start().

[Alberts Muktupāvels (muktupavels)]

Assertion is fixed, at least I am no longer getting that error. Also there is no empty labels anymore. Appmenu indicator works with all apps now.

But would not be better to check if widget has parent before trying to unparent?
if (gtk_widget_get_parent (GTK_WIDGET (entry->label)) != NULL)
    gtk_widget_unparent (GTK_WIDGET (entry->label));

Maybe you know how to fix this warning too?
WARNING: GLib-GObject - g_object_disconnect: invalid signal spec "signal::show"

review: Approve (testing)

[Dmitry Shachnev (mitya57)]

Alberts, thanks for quick testing.

1) gtk_widget_unparent has this check at the beginning:

  if (priv->parent == NULL)
    return;

... so that is not necessary.

2) I have no idea, for some reason connect succeeds but disconnect prints this warning. Maybe Lars has an idea.

[Charles Kerr (charlesk)]

Dmitry, thanks for the patch! Though, the GTK+ docs say gtk_widget_unparent() should only be used by GtkContainer subclasses. Maybe this patch should be using gtk_container_remove() instead?

Alberts, https://bugs.launchpad.net/indicator-applet/+bug/1284840

[Dmitry Shachnev (mitya57)]

Charles: Thanks for your review.

Actually, I don't know what the parent is. In my case, that label is created using GTK_LABEL macro in indicator-appmenu:

http://bazaar.launchpad.net/~indicator-applet-developers/indicator-appmenu/trunk.14.04/view/head:/src/window-menu-dbusmenu.c#L718

… and then transported over D-Bus, so I am not really sure that the parent is a GtkContainer.

I have also noticed that warning in Gtk docs, but I thought that we can pretend indicator-applet *is* a container implementation :)

[Dmitry Shachnev (mitya57)]

Hi Charles,

Can you please approve this MP? Or propose an alternative fix to the bug?

[Lars Karlitski (larsu)]

As I said on irc, I don't like this patch very much because entry->label not having a parent might point to a bigger issue. Also, I wonder why entry->image doesn't have the same issue. I've looked into this briefly, but haven't found the cause of the issue. It doesn't help that I can't seem to reproduce it...

I'm okay with landing this if it helps. Calling gtk_widget_unparent() shouldn't harm.

review: Approve

[Dmitry Shachnev (mitya57)]

Thanks Lars!

To reproduce it, just login into gnome-flashback session and add indicator-applet-appmenu to the panel.

The menu items will be in place (so you can click on them), but won't have any text.

=== modified file 'src/applet-main.c'
--- src/applet-main.c	2013-09-19 21:56:33 +0000
+++ src/applet-main.c	2014-02-25 16:08:06 +0000
@@ -404,7 +404,9 @@
         break;
       default:
         break;
-    }    
+    }
+    /* gtk_box_pack requires that the widget has no parent */
+    gtk_widget_unparent(GTK_WIDGET(entry->label));
     gtk_box_pack_start(GTK_BOX(box), GTK_WIDGET(entry->label), FALSE, FALSE, 1);
   }
   gtk_container_add(GTK_CONTAINER(menuitem), box);