← Back to team overview

ayatana-commits team mailing list archive

[Merge] lp:~ted/indicator-messages/shortcut-duplication-duplication into lp:indicator-messages

 

Ted Gould has proposed merging lp:~ted/indicator-messages/shortcut-duplication-duplication into lp:indicator-messages.

Requested reviews:
  Indicator Applet Developers (indicator-applet-developers)


Goal here is to remove the extra duplications of the shortcuts in the
menus.  This is caused by them not getting deleted, which was caused by
them not getting found.  So instead of searching, now we're being
explicit about adding and deleting them.
-- 
https://code.launchpad.net/~ted/indicator-messages/shortcut-duplication-duplication/+merge/23570
Your team ayatana-commits is subscribed to branch lp:indicator-messages.
=== modified file 'configure.ac'
--- configure.ac	2010-03-25 15:36:41 +0000
+++ configure.ac	2010-04-16 18:20:31 +0000
@@ -4,7 +4,7 @@
 AC_PREREQ(2.53)
 
 AM_CONFIG_HEADER(config.h)
-AM_INIT_AUTOMAKE(indicator-messages, 0.3.5)
+AM_INIT_AUTOMAKE(indicator-messages, 0.3.6)
 
 AM_MAINTAINER_MODE
 

=== modified file 'src/app-menu-item.c'
--- src/app-menu-item.c	2010-03-31 03:25:19 +0000
+++ src/app-menu-item.c	2010-04-16 18:20:31 +0000
@@ -36,7 +36,8 @@
 enum {
 	COUNT_CHANGED,
 	NAME_CHANGED,
-	SHORTCUTS_CHANGED,
+	SHORTCUT_ADDED,
+	SHORTCUT_REMOVED,
 	LAST_SIGNAL
 };
 
@@ -100,13 +101,20 @@
 	                                      NULL, NULL,
 	                                      g_cclosure_marshal_VOID__STRING,
 	                                      G_TYPE_NONE, 1, G_TYPE_STRING);
-	signals[SHORTCUTS_CHANGED] =  g_signal_new(APP_MENU_ITEM_SIGNAL_SHORTCUTS_CHANGED,
-	                                      G_TYPE_FROM_CLASS(klass),
-	                                      G_SIGNAL_RUN_LAST,
-	                                      G_STRUCT_OFFSET (AppMenuItemClass, shortcuts_changed),
-	                                      NULL, NULL,
-	                                      g_cclosure_marshal_VOID__VOID,
-	                                      G_TYPE_NONE, 0, G_TYPE_NONE);
+	signals[SHORTCUT_ADDED] =  g_signal_new(APP_MENU_ITEM_SIGNAL_SHORTCUT_ADDED,
+	                                      G_TYPE_FROM_CLASS(klass),
+	                                      G_SIGNAL_RUN_LAST,
+	                                      G_STRUCT_OFFSET (AppMenuItemClass, shortcut_added),
+	                                      NULL, NULL,
+	                                      g_cclosure_marshal_VOID__OBJECT,
+	                                      G_TYPE_NONE, 1, G_TYPE_OBJECT);
+	signals[SHORTCUT_REMOVED] =  g_signal_new(APP_MENU_ITEM_SIGNAL_SHORTCUT_REMOVED,
+	                                      G_TYPE_FROM_CLASS(klass),
+	                                      G_SIGNAL_RUN_LAST,
+	                                      G_STRUCT_OFFSET (AppMenuItemClass, shortcut_removed),
+	                                      NULL, NULL,
+	                                      g_cclosure_marshal_VOID__OBJECT,
+	                                      G_TYPE_NONE, 1, G_TYPE_OBJECT);
 
 	return;
 }
@@ -137,6 +145,7 @@
 static void
 func_unref (gpointer data, gpointer user_data)
 {
+	g_signal_emit(user_data, signals[SHORTCUT_REMOVED], 0, data, TRUE);
 	g_object_unref(G_OBJECT(data));
 	return;
 }
@@ -155,10 +164,9 @@
 	}
 
 	if (priv->shortcuts != NULL) {
-		g_list_foreach(priv->shortcuts, func_unref, NULL);
+		g_list_foreach(priv->shortcuts, func_unref, object);
 		g_list_free(priv->shortcuts);
 		priv->shortcuts = NULL;
-		g_signal_emit(object, signals[SHORTCUTS_CHANGED], 0, TRUE);
 	}
 
 	if (priv->root != NULL) {
@@ -351,7 +359,7 @@
 
 	priv->shortcuts = g_list_insert(priv->shortcuts, mip, position);
 
-	g_signal_emit(G_OBJECT(data), signals[SHORTCUTS_CHANGED], 0, TRUE);
+	g_signal_emit(G_OBJECT(data), signals[SHORTCUT_ADDED], 0, mip, TRUE);
 	return;
 }
 
@@ -377,10 +385,10 @@
 
 	if (pitems != NULL) {
 		DbusmenuMenuitemProxy * mip = DBUSMENU_MENUITEM_PROXY(pitems->data);
+		priv->shortcuts = g_list_remove(priv->shortcuts, mip);
+
+		g_signal_emit(G_OBJECT(data), signals[SHORTCUT_REMOVED], 0, mip, TRUE);
 		g_object_unref(mip);
-		priv->shortcuts = g_list_remove(priv->shortcuts, mip);
-
-		g_signal_emit(G_OBJECT(data), signals[SHORTCUTS_CHANGED], 0, TRUE);
 	}
 
 	return;
@@ -406,7 +414,7 @@
 	if (mip != NULL) {
 		priv->shortcuts = g_list_remove(priv->shortcuts, mip);
 		priv->shortcuts = g_list_insert(priv->shortcuts, mip, newpos);
-		g_signal_emit(G_OBJECT(data), signals[SHORTCUTS_CHANGED], 0, TRUE);
+		g_signal_emit(G_OBJECT(data), signals[SHORTCUT_ADDED], 0, NULL, TRUE);
 	}
 
 	return;
@@ -420,12 +428,10 @@
 	g_debug("Root Changed");
 	AppMenuItem * self = APP_MENU_ITEM(data);
 	AppMenuItemPrivate * priv = APP_MENU_ITEM_GET_PRIVATE(self);
-	gboolean change_time = FALSE;
 
 	if (priv->root != NULL) {
 		if (dbusmenu_menuitem_get_children(DBUSMENU_MENUITEM(priv->root)) != NULL) {
-			change_time = TRUE;
-			g_list_foreach(priv->shortcuts, func_unref, NULL);
+			g_list_foreach(priv->shortcuts, func_unref, data);
 			g_list_free(priv->shortcuts);
 			priv->shortcuts = NULL;
 		}
@@ -436,29 +442,28 @@
 	/* We need to proxy the new root across to the old
 	   world of indicator land. */
 	priv->root = newroot;
-	g_object_ref(priv->root);
-	g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_ADDED,   G_CALLBACK(child_added_cb),   self);
-	g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_REMOVED, G_CALLBACK(child_removed_cb), self);
-	g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_MOVED,   G_CALLBACK(child_moved_cb),   self);
-
-	/* See if we have any menuitems to worry about,
-	   otherwise we'll just move along. */
-	GList * children = dbusmenu_menuitem_get_children(DBUSMENU_MENUITEM(priv->root));
-	if (children != NULL) {
-		change_time = TRUE;
-		g_debug("\tProcessing %d children", g_list_length(children));
-		while (children != NULL) {
-			DbusmenuMenuitemProxy * mip = dbusmenu_menuitem_proxy_new(DBUSMENU_MENUITEM(children->data));
-			dbusmenu_menuitem_property_set(DBUSMENU_MENUITEM(mip), DBUSMENU_MENUITEM_PROP_ICON_NAME, DBUSMENU_MENUITEM_ICON_NAME_BLANK);
-			priv->shortcuts = g_list_append(priv->shortcuts, mip);
-			children = g_list_next(children);
+
+	if (priv->root != NULL) {
+		g_object_ref(priv->root);
+		g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_ADDED,   G_CALLBACK(child_added_cb),   self);
+		g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_REMOVED, G_CALLBACK(child_removed_cb), self);
+		g_signal_connect(G_OBJECT(priv->root), DBUSMENU_MENUITEM_SIGNAL_CHILD_MOVED,   G_CALLBACK(child_moved_cb),   self);
+
+		/* See if we have any menuitems to worry about,
+		   otherwise we'll just move along. */
+		GList * children = dbusmenu_menuitem_get_children(DBUSMENU_MENUITEM(priv->root));
+		if (children != NULL) {
+			g_debug("\tProcessing %d children", g_list_length(children));
+			while (children != NULL) {
+				DbusmenuMenuitemProxy * mip = dbusmenu_menuitem_proxy_new(DBUSMENU_MENUITEM(children->data));
+				dbusmenu_menuitem_property_set(DBUSMENU_MENUITEM(mip), DBUSMENU_MENUITEM_PROP_ICON_NAME, DBUSMENU_MENUITEM_ICON_NAME_BLANK);
+				priv->shortcuts = g_list_append(priv->shortcuts, mip);
+				g_signal_emit(G_OBJECT(self), signals[SHORTCUT_ADDED], 0, mip, TRUE);
+				children = g_list_next(children);
+			}
 		}
 	}
 
-	if (change_time) {
-		g_signal_emit(G_OBJECT(self), signals[SHORTCUTS_CHANGED], 0, TRUE);
-	}
-
 	return;
 }
 

=== modified file 'src/app-menu-item.h'
--- src/app-menu-item.h	2010-02-17 21:01:13 +0000
+++ src/app-menu-item.h	2010-04-16 18:20:31 +0000
@@ -39,7 +39,8 @@
 
 #define APP_MENU_ITEM_SIGNAL_COUNT_CHANGED     "count-changed"
 #define APP_MENU_ITEM_SIGNAL_NAME_CHANGED      "name-changed"
-#define APP_MENU_ITEM_SIGNAL_SHORTCUTS_CHANGED "shortcuts-changed"
+#define APP_MENU_ITEM_SIGNAL_SHORTCUT_ADDED    "shortcut-added"
+#define APP_MENU_ITEM_SIGNAL_SHORTCUT_REMOVED  "shortcut-removed"
 
 typedef struct _AppMenuItem      AppMenuItem;
 typedef struct _AppMenuItemClass AppMenuItemClass;
@@ -49,7 +50,8 @@
 
 	void (* count_changed) (guint count);
 	void (* name_changed) (gchar * name);
-	void (* shortcuts_changed) (void);
+	void (* shortcut_added) (DbusmenuMenuitem * mi);
+	void (* shortcut_removed) (DbusmenuMenuitem * mi);
 };
 
 struct _AppMenuItem {

=== modified file 'src/messages-service.c'
--- src/messages-service.c	2010-03-31 14:54:24 +0000
+++ src/messages-service.c	2010-04-16 18:20:31 +0000
@@ -55,7 +55,8 @@
 #define DESKTOP_FILE_GROUP        "Messaging Menu"
 #define DESKTOP_FILE_KEY_DESKTOP  "DesktopFile"
 
-static void server_shortcuts_changed (AppMenuItem * appitem, gpointer data);
+static void server_shortcut_added (AppMenuItem * appitem, DbusmenuMenuitem * mi, gpointer data);
+static void server_shortcut_removed (AppMenuItem * appitem, DbusmenuMenuitem * mi, gpointer data);
 static void server_count_changed (AppMenuItem * appitem, guint count, gpointer data);
 static void server_name_changed (AppMenuItem * appitem, gchar * name, gpointer data);
 static void im_time_changed (ImMenuItem * imitem, glong seconds, gpointer data);
@@ -582,7 +583,8 @@
 	/* Connect the signals up to the menu item */
 	g_signal_connect(G_OBJECT(menuitem), APP_MENU_ITEM_SIGNAL_COUNT_CHANGED, G_CALLBACK(server_count_changed), sl_item);
 	g_signal_connect(G_OBJECT(menuitem), APP_MENU_ITEM_SIGNAL_NAME_CHANGED,  G_CALLBACK(server_name_changed),  menushell);
-	g_signal_connect(G_OBJECT(menuitem), APP_MENU_ITEM_SIGNAL_SHORTCUTS_CHANGED,  G_CALLBACK(server_shortcuts_changed),  menushell);
+	g_signal_connect(G_OBJECT(menuitem), APP_MENU_ITEM_SIGNAL_SHORTCUT_ADDED,  G_CALLBACK(server_shortcut_added),  menushell);
+	g_signal_connect(G_OBJECT(menuitem), APP_MENU_ITEM_SIGNAL_SHORTCUT_REMOVED,  G_CALLBACK(server_shortcut_removed),  menushell);
 
 	/* Put our new menu item in, with the separator behind it.
 	   resort_menu will take care of whether it should be hidden
@@ -605,47 +607,26 @@
 	return;
 }
 
-/* The shortcuts have changed, let's just remove them and put
-   the back. */
+/* Server shortcut has been added */
 static void
-server_shortcuts_changed (AppMenuItem * appitem, gpointer data)
+server_shortcut_added (AppMenuItem * appitem, DbusmenuMenuitem * mi, gpointer data)
 {
-	g_debug("Application Shortcuts changed");
+	g_debug("Application Shortcut added: %s", mi != NULL ? dbusmenu_menuitem_property_get(mi, DBUSMENU_MENUITEM_PROP_LABEL) : "none");
 	DbusmenuMenuitem * shell = DBUSMENU_MENUITEM(data);
-	gboolean appitemfound = FALSE;
-	GList * children = dbusmenu_menuitem_get_children(shell);
-	GList * removelist = NULL;
-
-	while (children != NULL) {
-		if (!appitemfound && children->data != appitem) {
-			children = g_list_next(children);
-			continue;
-		}
-		appitemfound = TRUE;
-
-		if (!DBUSMENU_IS_MENUITEM_PROXY(children->data)) {
-			break;
-		}
-
-		removelist = g_list_prepend(removelist, children->data);
-	}
-
-	GList * removeitem;
-	for (removeitem = removelist; removeitem != NULL; removeitem = g_list_next(removeitem)) {
-		dbusmenu_menuitem_child_delete(shell, DBUSMENU_MENUITEM(removeitem->data));
-	}
-	g_list_free(removeitem);
-
-	GList * shortcuts = app_menu_item_get_items(appitem);
-	while (shortcuts != NULL) {
-		DbusmenuMenuitem * mi = DBUSMENU_MENUITEM(shortcuts->data);
-		g_debug("\tAdding shortcut: %s", dbusmenu_menuitem_property_get(mi, DBUSMENU_MENUITEM_PROP_LABEL));
+	if (mi != NULL) {
 		dbusmenu_menuitem_child_append(shell, mi);
-		shortcuts = g_list_next(shortcuts);
 	}
-
 	resort_menu(shell);
+	return;
+}
 
+/* Server shortcut has been removed */
+static void
+server_shortcut_removed (AppMenuItem * appitem, DbusmenuMenuitem * mi, gpointer data)
+{
+	g_debug("Application Shortcut removed: %s", mi != NULL ? dbusmenu_menuitem_property_get(mi, DBUSMENU_MENUITEM_PROP_LABEL) : "none");
+	DbusmenuMenuitem * shell = DBUSMENU_MENUITEM(data);
+	dbusmenu_menuitem_child_delete(shell, mi);
 	return;
 }
 
@@ -756,16 +737,31 @@
 
 	/* If there is a menu item, let's get rid of it. */
 	if (sltp->menuitem != NULL) {
-		dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(sltp->menuitem), DBUSMENU_MENUITEM_PROP_VISIBLE, TRUE);
+		/* If there are shortcuts remove them */
+		GList * shortcuts = app_menu_item_get_items(sltp->menuitem);
+		while (shortcuts != NULL) {
+			g_debug("\tRemoving shortcut: %s", dbusmenu_menuitem_property_get(DBUSMENU_MENUITEM(shortcuts->data), DBUSMENU_MENUITEM_PROP_LABEL));
+			dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(shortcuts->data), DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
+			dbusmenu_menuitem_child_delete(DBUSMENU_MENUITEM(data), DBUSMENU_MENUITEM(shortcuts->data));
+			shortcuts = g_list_next(shortcuts);
+		}
+
+		g_debug("\tRemoving item");
+		dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(sltp->menuitem), DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
 		dbusmenu_menuitem_child_delete(DBUSMENU_MENUITEM(data), DBUSMENU_MENUITEM(sltp->menuitem));
 		g_object_unref(G_OBJECT(sltp->menuitem));
+	} else {
+		g_debug("\tNo menuitem");
 	}
-
+	
 	/* If there is a separator, let's get rid of it. */
 	if (sltp->separator != NULL) {
+		g_debug("\tRemoving separator");
 		dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(sltp->separator), DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
 		dbusmenu_menuitem_child_delete(DBUSMENU_MENUITEM(data), DBUSMENU_MENUITEM(sltp->separator));
 		g_object_unref(G_OBJECT(sltp->separator));
+	} else {
+		g_debug("\tNo separator");
 	}
 
 	if (sltp->attention) {
@@ -1263,6 +1259,14 @@
 	g_list_free(li->appdiritems);
 
 	if (li->menuitem != NULL) {
+		/* If there are shortcuts remove them */
+		GList * shortcuts = launcher_menu_item_get_items(li->menuitem);
+		while (shortcuts != NULL) {
+			dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(shortcuts->data), DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
+			dbusmenu_menuitem_child_delete(DBUSMENU_MENUITEM(data), DBUSMENU_MENUITEM(shortcuts->data));
+			shortcuts = g_list_next(shortcuts);
+		}
+
 		dbusmenu_menuitem_property_set_bool(DBUSMENU_MENUITEM(li->menuitem), DBUSMENU_MENUITEM_PROP_VISIBLE, FALSE);
 		dbusmenu_menuitem_child_delete(root_menuitem, DBUSMENU_MENUITEM(li->menuitem));
 		g_object_unref(G_OBJECT(li->menuitem));


Follow ups