ayatana-commits team mailing list archive

[Michael Terry <michael.terry@xxxxxxxxxxxxx>]

Michael Terry has proposed merging lp:~mterry/indicator-appmenu/cleaner-remove into lp:indicator-appmenu.

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

For more details, see:
https://code.launchpad.net/~mterry/indicator-appmenu/cleaner-remove/+merge/48364

So when I was in the indicator-appmenu code, I noticed a TODO in the bit that removes entries from the window menu code.  It currently just deleted the last entry in the list instead of the requested entry.

I haven't found code that hits this bug, so I'm not sure how important this is, but since I was there, I whipped up a little searcher function and used it there and elsewhere to grab an entry from the internal list.

Didn't seem to make things worse, though it also didn't make things noticeably better.
-- 
https://code.launchpad.net/~mterry/indicator-appmenu/cleaner-remove/+merge/48364
Your team ayatana-commits is subscribed to branch lp:indicator-appmenu.

=== modified file 'src/window-menus.c'
--- src/window-menus.c	2011-01-26 22:21:20 +0000
+++ src/window-menus.c	2011-02-02 19:14:52 +0000
@@ -338,6 +338,26 @@
 	return;
 }
 
+static IndicatorObjectEntry *
+get_entry(WindowMenus *wm, DbusmenuMenuitem * item, guint *index)
+{
+	WindowMenusPrivate * priv = WINDOW_MENUS_GET_PRIVATE(wm);
+
+	guint position = 0;
+	for (position = 0; position < priv->entries->len; ++position) {
+		WMEntry * entry = g_array_index(priv->entries, WMEntry *, position);
+		if (entry->mi == item) {
+			if (index != NULL) {
+				*index = position;
+			}
+			return &entry->ioentry;
+		}
+	}
+
+	/* Not found */
+	return NULL;
+}
+
 /* Called when a menu item wants to be displayed.  We need to see if
    it's one of our root items and pass it up if so. */
 static void
@@ -350,33 +370,12 @@
 		return;
 	}
 
-	GList * children = dbusmenu_menuitem_get_children(priv->root);
-	guint position = 0;
-	GList * child;
-
-	for (child = children; child != NULL; position++, child = g_list_next(child)) {
-		DbusmenuMenuitem * childmi = DBUSMENU_MENUITEM(child->data);
-
-		/* We're only putting items with children on the panel, so
-		   they're the only one with entires. */
-		if (dbusmenu_menuitem_get_children(childmi) == NULL) {
-			position--;
-			continue;
-		}
-
-		if (childmi == item) {
-			break;
-		}
-	}
-
-	/* Not found */
-	if (child == NULL) {
+	IndicatorObjectEntry * entry = get_entry(WINDOW_MENUS(user_data), item, NULL);
+	if (entry == NULL) {
+		/* Not found */
 		return;
 	}
 
-	g_return_if_fail(position < priv->entries->len);
-
-	IndicatorObjectEntry * entry = g_array_index(priv->entries, IndicatorObjectEntry *, position);
 	g_signal_emit(G_OBJECT(user_data), signals[SHOW_MENU], 0, entry, timestamp, TRUE);
 
 	return;
@@ -715,9 +714,14 @@
 		return;
 	}
 	
-	/* TODO: find the menuitem */
-	IndicatorObjectEntry * entry = g_array_index(priv->entries, IndicatorObjectEntry *, priv->entries->len - 1);
-	g_array_remove_index(priv->entries, priv->entries->len - 1);
+	guint position;
+	IndicatorObjectEntry * entry = get_entry(WINDOW_MENUS(user_data), oldentry, &position);
+	if (entry == NULL) {
+		/* Not found */
+		return;
+	}
+
+	g_array_remove_index(priv->entries, position);
 
 	g_signal_emit(G_OBJECT(user_data), signals[ENTRY_REMOVED], 0, entry, TRUE);