Re: [evolution-patches] Patch for bug 45826, please review

[Not Zed <notzed ximian com>]

A few comments, (also, remember a changelog).


+static void
+popup_menu_placement_callback(GtkMenu *menu, gint *x, gint *y, gboolean
*push_in, gpointer user_data)
+{
+       EMsgComposerAttachmentBar *bar;
+       GnomeIconList *icon_list;
+       GList *selection;
+       GnomeCanvasPixbuf *image;
+       GList *first_item;
+
+       g_return_if_fail (user_data != NULL);

This one isn't needed, as you know what you're calling it with.

+       bar = E_MSG_COMPOSER_ATTACHMENT_BAR (user_data);
+       icon_list = GNOME_ICON_LIST (user_data);
+
+       selection = gnome_icon_list_get_selection (icon_list);
+       g_return_if_fail (selection != NULL);

Since this is probably a possible normal-case and not a coding error,
this should just be a return.

+       first_item = g_list_first (selection);
+       g_return_if_fail (first_item != NULL);

All of this is completely redundant, first_item will be set to
selection, so first_item doesn't need to exist.

+       gdk_window_get_origin (((GtkWidget*) bar)->window, x, y);

This should probably come first, so x and y are always set to something.

+       image = gnome_icon_list_get_icon_pixbuf_item (icon_list,
(gint)first_item->data);
+       g_return_if_fail (image != NULL);

If its possible to have a null icon, this should just be a return, or
probably an if.

+       /* Put menu to the center of icon */
+       *x += (gint)(image->item.x1 + image->item.x2)/2;
+       *y += (gint)(image->item.y1 + image->item.y2)/2;
+
+}

On Fri, 2003-07-04 at 21:17, Yiming Cao wrote:
> Hi,
> 
> This patch fix bug 45862, "shift+f10 should activate popup menu when
> focus on attachment". Now in the composer, if you press S-F10 when the
> attachments are focused, popup menu will be available. The menu comes at
> the center point of the first focused attachment.