Re: [evolution-patches] New Attachment Bar UI+Patch
- From: Srinivasa Ragavan <sragavan novell com>
- To: Not Zed <notzed ximian com>
- Cc: Evolution Patches List <evolution-patches lists ximian com>
- Subject: Re: [evolution-patches] New Attachment Bar UI+Patch
- Date: Mon, 25 Jul 2005 13:21:19 +0530
Notzed,
i have commited it, since today is the freeze. I would fix any more of
of your comments as bugfix. As of now, i found one random crash thatz
occuring and im figuring out.
-Srini.
On Wed, 2005-07-20 at 17:01 +0530, Srinivasa Ragavan wrote:
> Notzed,
>
> I have reworked on most your comments and given my thoughts below.
>
> On Wed, 2005-07-20 at 12:40 +0800, Not Zed wrote:
> > Its getting there finally ...
> >
> >
> > On Fri, 2005-07-15 at 16:34 +0530, Srinivasa Ragavan wrote:
> > >
> > > Index: em-format-html-display.c
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-format-html-display.c,v
> > > retrieving revision 1.69
> > > diff -u -p -r1.69 em-format-html-display.c
> > > --- em-format-html-display.c 6 Jul 2005 03:56:48 -0000 1.69
> > > +++ em-format-html-display.c 15 Jul 2005 10:42:20 -0000
> > > @@ -45,6 +45,7 @@
> > > #include <gtk/gtkmenuitem.h>
> > > #include <gtk/gtkmain.h>
> > > #include <gtk/gtkdnd.h>
> > > +#include <gtk/gtktoolbutton.h>
> > >
> > > #include <glade/glade.h>
> > >
> > > @@ -87,6 +88,8 @@
> > > #include "em-icon-stream.h"
> > > #include "em-utils.h"
> > > #include "em-popup.h"
> > > +#include "e-attachment.h"
> > > +#include "e-attachment-bar.h"
> >
> > Is this in cvs yet? If not, you should include it so we can test the
> > patch.
> Commited
> >
> > > #define d(x)
> > >
> > > @@ -109,6 +112,8 @@ static void efhd_html_on_url (GtkHTML *h
> > >
> > > static void efhd_attachment_frame(EMFormat *emf, CamelStream *stream,
> > > EMFormatPURI *puri);
> > > static gboolean efhd_attachment_image(EMFormatHTML *efh,
> > > GtkHTMLEmbedded *eb, EMFormatHTMLPObject *pobject);
> > > +static void efhd_message_add_bar(EMFormat *emf, CamelStream *stream,
> > > CamelMimePart *part, const EMFormatHandler *info);
> > > +static void efhd_attachment_bar_refresh (EMFormatHTMLDisplay *efhd);
> > >
> > > struct _attach_puri {
> > > EMFormatPURI puri;
> > > @@ -128,6 +133,11 @@ struct _attach_puri {
> > > int fit_width;
> > > int fit_height;
> > > GtkImage *image;
> > > +
> > > + /* Signed / Encrypted */
> > > + camel_cipher_validity_sign_t sign;
> > > + camel_cipher_validity_encrypt_t encrypt;
> > > +
> > > };
> > >
> > >
> > > @@ -254,6 +264,9 @@ efhd_init(GObject *o)
> > > /* we want to convert url's etc */
> > > efh->text_html_flags |= CAMEL_MIME_FILTER_TOHTML_CONVERT_URLS
> > > | CAMEL_MIME_FILTER_TOHTML_CONVERT_ADDRESSES;
> > > #undef efh
> > > +
> > > + efhd->show_bar = FALSE;
> > > + efhd->files = g_hash_table_new_full (g_str_hash, g_int_equal,
> > > g_free, NULL);
> > > }
> > >
> > > static void
> > > @@ -266,6 +279,9 @@ efhd_finalise(GObject *o)
> > > g_free(efhd->priv->search_text);
> > > g_free(efhd->priv);
> > >
> > > + if (efhd->files)
> > > + g_hash_table_destroy(efhd->files);
> > > +
> > > ((GObjectClass *)efhd_parent)->finalize(o);
> > > }
> > >
> > > @@ -959,6 +975,7 @@ static EMFormatHandler type_builtin_tabl
> > > { "image/pjpeg", (EMFormatFunc)efhd_image },
> > >
> > > { "x-evolution/message/prefix",
> > > (EMFormatFunc)efhd_message_prefix },
> > > + { "x-evolution/message/attachment-bar",
> > > (EMFormatFunc)efhd_message_add_bar },
> >
> > When i said 'x-type' i meant, do what you had before ... but since this
> > makes things simple, see later on in the emformathtml - rename it.
> >
> done it.
> > > };
> > >
> > > static void
> > > @@ -1007,6 +1024,20 @@ static const EMFormatHandler *efhd_find_
> > >
> > > static void efhd_format_clone(EMFormat *emf, CamelFolder *folder,
> > > const char *uid, CamelMimeMessage *msg, EMFormat *src)
> > > {
> > > + EMFormatHTMLDisplay *efhd = (EMFormatHTMLDisplay *) emf;
> > > +
> > > + if (emf != src) {
> > > + if (src)
> > > + ((EMFormatHTMLDisplay *) emf)->show_bar =
> > > ((EMFormatHTMLDisplay *)src)->show_bar;
> > > + else
> > > + ((EMFormatHTMLDisplay *) emf)->show_bar =
> > > FALSE;
> > > + }
> >
> > If you're going to add the alias variable efhd, you should probably use
> > it.
> >
> done.
> > > +
> > > + efhd->attachment_bar = NULL;
> > > + if (efhd->files)
> > > + g_hash_table_destroy(efhd->files);
> > > + efhd->files = g_hash_table_new_full (g_str_hash, g_int_equal,
> > > g_free, NULL);
> > > +
> >
> > Umm, g_str_hash needs to use g_str_equal, not g_int_equal.
> >
> done.
> > > ((EMFormatClass *)efhd_parent)->format_clone(emf, folder, uid,
> > > msg, src);
> > > }
> > >
> > > @@ -1370,6 +1401,8 @@ efhd_attachment_image(EMFormatHTML *efh,
> > > static gboolean
> > > efhd_attachment_button(EMFormatHTML *efh, GtkHTMLEmbedded *eb,
> > > EMFormatHTMLPObject *pobject)
> > > {
> > > + EMFormatHTMLDisplay *efhd = (EMFormatHTMLDisplay *)efh;
> > > + EAttachment *new;
> > > struct _attach_puri *info;
> > > GtkWidget *hbox, *w, *button, *mainbox;
> > > char *simple_type;
> > > @@ -1386,6 +1419,42 @@ efhd_attachment_button(EMFormatHTML *efh
> > > g_assert(info != NULL);
> > > g_assert(info->forward == NULL);
> > >
> > > + if (!
> > > camel_content_type_is(camel_medium_get_content_object((CamelMedium
> > > *)info->puri.part)->mime_type, "message", "rfc822")) {
> >
> > Messages are also attachments, why are you removing them from the list?
> >
> okie. Taken care.
> > > + const char *file =
> > > camel_mime_part_get_filename(info->puri.part);
> > > + char *tmp, *new_file = NULL;
> > > +
> > > + new = e_attachment_new_from_mime_part
> > > (info->puri.part);
> > > +
> > > + if (!file) {
> > > + file = "attachment.dat";
> > > + camel_mime_part_set_filename(new->body, file);
> >
> > > + }
> > > +
> > > + tmp = g_hash_table_lookup (efhd->files, file);
> > > + if (tmp) {
> >
> > > + guint count = GPOINTER_TO_UINT(tmp);
> > > + char *ext;
> > > + if ((ext = strrchr(file, '.')))
> > > + new_file = g_strdup_printf("%.*s(%d)%
> > > s", ext-file, file, count++, ext);
> > > + else
> > > + new_file = g_strdup_printf("%s(%d)",
> > > file, count++);
> >
> > You can't use .* to cut off a string, it works on the number of
> > characters not the number of bytes. So it breaks for multi-byte strings
> > like utf8, which this is all using.
> >
> done.
> > > + g_hash_table_insert (efhd->files,
> > > g_strdup(file), GUINT_TO_POINTER(count));
> > > + camel_mime_part_set_filename(new->body,
> > > new_file);
> > > +
> > > + g_free(new_file);
> > > + } else {
> > > + g_hash_table_insert (efhd->files,
> > > g_strdup(file), GUINT_TO_POINTER(1));
> > > + }
> > > +
> > > + /* Store the status of encryption / signature on the
> > > attachment for emblem display */
> > > + new->sign = info->sign;
> > > + new->encrypt = info->encrypt;
> >
> > I'm not sure this will work properly, the validity is calculated at
> > different points, not before the attachment is displayed though.
> >
> I tried it worked. Any way i have added a fixme.
> > > + /* Add the attachment to the bar.*/
> > > + e_attachment_bar_add_attachment
> > > (E_ATTACHMENT_BAR(efhd->attachment_bar), new);
> > > + efhd_attachment_bar_refresh (efhd);
> >
> > I think the attachment bar should have a changed signal, and the refresh
> > thing should hook off that. The refresh thing should only update the
> > label.
> >
> Done.
> > > + }
> > > +
> > > mainbox = gtk_hbox_new(FALSE, 0);
> > >
> > > button = gtk_button_new();
> > > @@ -1644,6 +1713,248 @@ type_ok:
> > > }
> > >
> > > static void
> > > +attachment_bar_arrow_clicked(GtkWidget *w, EMFormatHTMLDisplay *efhd)
> > > +{
> > > +
> > > + efhd->show_bar = !efhd->show_bar;
> > > +
> > > + if (efhd->show_bar) {
> > > + gtk_widget_show(efhd->attachment_box);
> > > + gtk_arrow_set((GtkArrow
> > > *)gtk_tool_button_get_icon_widget((GtkToolButton *)w), GTK_ARROW_DOWN,
> > > GTK_SHADOW_NONE);
> > > + } else {
> > > + gtk_widget_hide(efhd->attachment_box);
> > > + gtk_arrow_set((GtkArrow
> > > *)gtk_tool_button_get_icon_widget((GtkToolButton *)w),
> > > GTK_ARROW_RIGHT, GTK_SHADOW_NONE);
> > > + }
> >
> > As i said last time, use the same mechanism as is used elsewhere, hide
> > the arrows, dont create new ones.
> >
> > (why do i have to repeat myself so often?).
> >
> sorry about that. I missed out.
> > > +}
> > > +
> > > +static void
> > > +attachments_save_all_clicked(GtkWidget *w, EMFormatHTMLDisplay *efhd)
> > > +{
> > > + GSList *attachment_parts;
> > > +
> > > + attachment_parts =
> > > e_attachment_bar_get_attachment_part_list(E_ATTACHMENT_BAR(efhd->attachment_bar));
> >
> >
> > What is wrong with "e_attachment_bar_get_parts()"?
> >
> > Such a huge long name for no reason.
> >
> changed.
> > > + em_utils_save_parts(w, _("Select folder to save all
> > > attachments..."), attachment_parts);
> > > +}
> > > +
> > > +static void
> > > +efhd_bar_popup_position(GtkMenu *menu, int *x, int *y, gboolean
> > > *push_in, gpointer user_data)
> > > +{
> > > + EAttachmentBar *bar = user_data;
> > > + GnomeIconList *icon_list = user_data;
> > > + GList *selection;
> > > + GnomeCanvasPixbuf *image;
> > > +
> > > + gdk_window_get_origin (((GtkWidget*) bar)->window, x, y);
> > > +
> > > + selection = gnome_icon_list_get_selection (icon_list);
> > > + if (selection == NULL)
> > > + return;
> > > +
> > > + image = gnome_icon_list_get_icon_pixbuf_item (icon_list,
> > > (gint)selection->data);
> > > + if (image == NULL)
> > > + return;
> > > +
> > > + /* Put menu to the center of icon. */
> > > + *x += (int)(image->item.x1 + image->item.x2) / 2;
> > > + *y += (int)(image->item.y1 + image->item.y2) / 2;
> > > +}
> > > +
> > > +static void
> > > +efhd_bar_save_selected(EPopup *ep, EPopupItem *item,
> > > EMFormatHTMLDisplay *efhd)
> > > +{
> > > + GSList *attachment_parts, *tmp;
> > > + GSList *parts = NULL;
> > > +
> > > + attachment_parts =
> > > e_attachment_bar_get_attachment(E_ATTACHMENT_BAR(efhd->attachment_bar), -1);
> >
> > again strange api here. why not get_selected (hmm, i hear an echo).
> >
> done.
> > > +
> > > + for (tmp = attachment_parts; tmp; tmp=tmp->next)
> > > + parts = g_slist_prepend(parts, ((EAttachment
> > > *)tmp->data)->body);
> > > +
> > > + parts = g_slist_reverse(parts);
> > > + em_utils_save_parts(efhd->attachment_bar, _("Select folder to
> > > save selected attachments..."), parts);
> > > +
> > > + g_slist_foreach(attachment_parts, (GFunc)g_object_unref,
> > > NULL);
> > > + g_slist_free (attachment_parts);
> > > +}
> > > +
> > > +static EPopupItem efhd_bar_menu_items[] = {
> > > + { E_POPUP_BAR, "05.display", },
> > > + { E_POPUP_ITEM, "05.display.01", N_("Save Selected..."),
> > > efhd_bar_save_selected, NULL, NULL, EM_POPUP_ATTACHMENTS_MULTIPLE},
> > > +};
> > > +
> > > +static gboolean
> > > +efhd_bar_button_press_event(EAttachmentBar *bar, GdkEventButton
> > > *event, EMFormat *emf)
> > > +{
> > > + GtkMenu *menu;
> > > + GSList *list=NULL;
> > > + EPopupTarget *target;
> > > + EMPopup *emp;
> > > + GSList *menus = NULL;
> > > + int i;
> > > +
> > > + if (event && event->button != 3)
> > > + return FALSE;
> > > +
> > > + /** @HookPoint-EMPopup: Attachment Bar Context Menu
> > > + * @Id: org.gnome.evolution.mail.attachments.popup
> > > + * @Class: org.gnome.evolution.mail.popup:1.0
> > > + * @Target: EMPopupTargetPart
> > > + *
> > > + * This is the drop-down menu shown when a user clicks on the
> > > attachment bar
> > > + * when attachments are selected.
> > > + */
> > > + emp =
> > > em_popup_new("org.gnome.evolution.mail.attachments.popup");
> > > +
> > > + /* Add something like save-selected, foward selected
> > > attachments in a mail etc....*/
> > > + list = e_attachment_bar_get_attachment (bar, -1);
> > > +
> > > + /* Lets not propagate any more the r-click which is intended
> > > to us*/
> > > + if ( g_slist_length (list) == 0)
> > > + return TRUE;
> >
> > > + target = (EPopupTarget *)em_popup_target_new_attachments(emp,
> > > list);
> > > + for (i=0; i<2; i++)
> > > + menus = g_slist_prepend(menus,
> > > &efhd_bar_menu_items[i]);
> > > + e_popup_add_items((EPopup *)emp, menus, NULL,
> > > efhd_menu_items_free, emf);
> > > +
> > > + ((EMPopupTargetPart *)target)->target.widget = (GtkWidget
> > > *)bar;
> > > + menu = e_popup_create_menu_once((EPopup *)emp, (EPopupTarget
> > > *)target, 0);
> > > + if (event)
> > > + gtk_menu_popup(menu, NULL, NULL, NULL, NULL,
> > > event->button, event->time);
> > > + else
> > > + gtk_menu_popup(menu, NULL, NULL,
> > > (GtkMenuPositionFunc)efhd_bar_popup_position, bar, 0,
> > > gtk_get_current_event_time());
> > > +
> > > + return TRUE;
> > > +
> > > +}
> >
> > ^^ style, be careful of extra whitespace.
> >
> okie.
> > > +
> > > +static gboolean
> > > +efhd_bar_popup_menu_event (EAttachmentBar *bar, EMFormat *emf)
> > > +{
> > > + return efhd_bar_button_press_event(bar, NULL, emf);
> > > +}
> > > +
> > > +static void
> > > +efhd_attachment_bar_refresh (EMFormatHTMLDisplay *efhd)
> > > +{
> >
> > > + PangoFontMetrics *metrics;
> > > + PangoContext *context;
> > > + int width, height, bar_width, bar_height, nattachments;
> > > +
> > > + if (!efhd->attachment_bar)
> > > + return;
> > > +
> > > + context = gtk_widget_get_pango_context ((GtkWidget *)
> > > efhd->attachment_bar);
> > > + metrics = pango_context_get_metrics (context, ((GtkWidget *)
> > > efhd->attachment_bar)->style->font_desc, pango_context_get_language
> > > (context));
> > > + width = PANGO_PIXELS
> > > (pango_font_metrics_get_approximate_char_width (metrics)) * 15;
> > > + /* This should be *2, but the icon list creates too much space
> > > above ... */
> > > + height = PANGO_PIXELS (pango_font_metrics_get_ascent (metrics)
> > > + pango_font_metrics_get_descent (metrics)) * 3;
> > > + pango_font_metrics_unref (metrics);
> > > +
> > > + gtk_widget_get_size_request (efhd->attachment_bar, &bar_width,
> > > &bar_height);
> > > +
> > > + nattachments = e_attachment_bar_get_num_attachments
> > > (E_ATTACHMENT_BAR(efhd->attachment_bar));
> > > + if (nattachments) {
> > > + int per_col, rows;
> > > + char *txt;
> > > + per_col = bar_width / width;
> > > + rows = nattachments / per_col;
> > > +
> > > + /* FIXME: Do the calculation better. Often it goes
> > > more */
> > > + gtk_widget_set_size_request (efhd->attachment_bar,
> > > bar_width, (rows+1) * 84 /* FIXME: Use enum/defines */);
> >
> > All of this resizing stuff should be internal to the attachment bar.
> >
> done.
> > > + /* Cant i put in the number of attachments here ?*/
> > > + txt = g_strdup_printf(ngettext("%d Attachment", "%d
> > > Attachments", nattachments), nattachments);
> > > + gtk_label_set_text ((GtkLabel *)efhd->label, txt);
> > > + g_free (txt);
> > > +
> > > + /* Enable the expander button and the save all
> > > button.*/
> > > + gtk_widget_set_sensitive (efhd->arrow, TRUE);
> > > + gtk_widget_set_sensitive (efhd->save, TRUE);
> > > + }
> > > +}
> > > +
> > > +static gboolean
> > > +efhd_add_bar(EMFormatHTML *efh, GtkHTMLEmbedded *eb,
> > > EMFormatHTMLPObject *pobject)
> > > +{
> > > + EMFormatHTMLDisplay *efhd = (EMFormatHTMLDisplay *)efh;
> > > + GtkWidget *hbox1, *hbox2, *vbox, *txt, *image;
> > > + int width, height;
> > > + GtkTargetEntry drag_types[] = {
> > > + { "text/uri-list", 0, 0 },
> > > + };
> > > +
> > > + efhd->attachment_bar = e_attachment_bar_new(NULL);
> > > +
> > > + if (efhd->show_bar)
> > > + efhd->arrow = (GtkWidget
> > > *)gtk_tool_button_new(gtk_arrow_new (GTK_ARROW_DOWN, GTK_SHADOW_NONE),
> > > NULL);
> > > + else
> > > + efhd->arrow = (GtkWidget
> > > *)gtk_tool_button_new(gtk_arrow_new (GTK_ARROW_RIGHT,
> > > GTK_SHADOW_NONE), NULL);
> > > +
> > > + efhd->label = gtk_label_new(_("No Attachment"));
> > > + efhd->save = gtk_button_new();
> > > + image = gtk_image_new_from_stock ("gtk-save",
> > > GTK_ICON_SIZE_BUTTON);
> > > + txt = gtk_label_new(_("Save All"));
> > > + hbox1 = gtk_hbox_new(FALSE, 0);
> > > + gtk_box_pack_start((GtkBox *)hbox1, image, FALSE, FALSE, 2);
> > > + gtk_box_pack_start((GtkBox *)hbox1, txt, FALSE, FALSE, 0);
> > > +
> > > + gtk_container_add((GtkContainer *)efhd->save, hbox1);
> > > +
> > > + gtk_widget_set_sensitive(efhd->arrow, FALSE);
> > > + gtk_widget_set_sensitive(efhd->save, FALSE);
> > > +
> > > + hbox2 = gtk_hbox_new (FALSE, 0);
> > > + gtk_box_pack_start ((GtkBox *)hbox2, efhd->arrow, FALSE,
> > > FALSE, 0);
> > > + gtk_box_pack_start ((GtkBox *)hbox2, efhd->label, FALSE,
> > > FALSE, 2);
> > > + gtk_box_pack_start ((GtkBox *)hbox2, efhd->save, FALSE, FALSE,
> > > 2);
> > > +
> > > + GTK_WIDGET_SET_FLAGS (efhd->attachment_bar, GTK_CAN_FOCUS);
> >
> > Shouldn't the attachment bar be setting this flag itself?
> >
> done.
> > > + efhd->attachment_box = gtk_hbox_new (FALSE, 0);
> > > + gtk_box_pack_start ((GtkBox *)efhd->attachment_box,
> > > efhd->attachment_bar, TRUE, TRUE, 0);
> > > +
> > > + gtk_widget_get_size_request(efhd->attachment_bar, &width,
> > > &height);
> > > +
> > > + /* FIXME: What if the text is more?. Should we reduce the text
> > > with appending ...?
> > > + * or resize the bar? How to figure out that, it needs more
> > > space? */
> > > + gtk_widget_set_size_request (efhd->attachment_bar,
> > > + ((GtkWidget
> > > *)efh->html)->parent->allocation.width - /* FIXME */36,
> > > + 84 /* FIXME: Default show only
> > > one row, Dont hardcode size*/);
> > > +
> > > + vbox = gtk_vbox_new (FALSE, 0);
> > > + gtk_box_pack_start ((GtkBox *)vbox, hbox2, FALSE, FALSE, 2);
> > > + gtk_box_pack_start ((GtkBox *)vbox, efhd->attachment_box,
> > > TRUE, TRUE, 2);
> > > +
> > > + gtk_container_add ((GtkContainer *)eb, vbox);
> > > + gtk_widget_show_all ((GtkWidget *)eb);
> > > +
> > > + if (!efhd->show_bar)
> > > + gtk_widget_hide (efhd->attachment_box);
> > > +
> > > + g_signal_connect (efhd->arrow, "clicked",
> > > G_CALLBACK(attachment_bar_arrow_clicked), efh);
> > > + g_signal_connect (efhd->attachment_bar, "button_press_event",
> > > G_CALLBACK(efhd_bar_button_press_event), efhd);
> > > + g_signal_connect (efhd->attachment_bar, "popup-menu",
> > > G_CALLBACK(efhd_bar_popup_menu_event), efhd);
> >
> > > + gtk_drag_source_set(efhd->attachment_bar, GDK_BUTTON1_MASK,
> > > drag_types, G_N_ELEMENTS(drag_types), GDK_ACTION_COPY);
> >
> > Why is this still doing dnd stuff?
> >
> missed out.
> > > + g_signal_connect (efhd->save, "clicked",
> > > G_CALLBACK(attachments_save_all_clicked), efh);
> > > +
> > > + return TRUE;
> > > +}
> > > +
> > > +static void
> > > +efhd_message_add_bar(EMFormat *emf, CamelStream *stream,
> > > CamelMimePart *part, const EMFormatHandler *info)
> > > +{
> > > + EMFormatHTMLDisplay *efhd = (EMFormatHTMLDisplay *) emf;
> > > + const char *classid = "attachment-bar";
> > > +
> > > + if (efhd->attachment_bar)
> > > + return;
> >
> > > + em_format_html_add_pobject((EMFormatHTML *)emf,
> > > sizeof(EMFormatHTMLPObject), classid, part, efhd_add_bar);
> > > + camel_stream_printf(stream, "<td><object classid=\"%s
> > > \"></object></td>", classid);
> > > +}
> > > +
> > > +static void
> > > efhd_format_attachment(EMFormat *emf, CamelStream *stream,
> > > CamelMimePart *part, const char *mime_type, const EMFormatHandler
> > > *handle)
> > > {
> > > char *classid, *text, *html;
> > > @@ -1655,6 +1966,11 @@ efhd_format_attachment(EMFormat *emf, Ca
> > > info->handle = handle;
> > > info->shown = em_format_is_inline(emf, info->puri.part_id,
> > > info->puri.part, handle);
> > > info->snoop_mime_type = emf->snoop_mime_type;
> > > +
> > > + if (emf->valid) {
> > > + info->sign = emf->valid->sign.status;
> > > + info->encrypt = emf->valid->encrypt.status;
> > > + }
> > >
> > > camel_stream_write_string(stream,
> > > EM_FORMAT_HTML_VPAD
> > > Index: em-format-html-display.h
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-format-html-display.h,v
> > > retrieving revision 1.5
> > > diff -u -p -r1.5 em-format-html-display.h
> > > --- em-format-html-display.h 16 May 2005 07:53:53 -0000 1.5
> > > +++ em-format-html-display.h 15 Jul 2005 10:42:20 -0000
> > > @@ -22,6 +22,15 @@ struct _EMFormatHTMLDisplay {
> > >
> > > unsigned int animate:1;
> > > unsigned int caret_mode:1;
> > > +
> > > + /* for Attachment bar */
> > > + GtkWidget *attachment_bar;
> > > + GtkWidget *attachment_box;
> > > + GtkWidget *label;
> > > + GtkWidget *arrow;
> > > + GtkWidget *save;
> > > + gboolean show_bar;
> > > + GHashTable *files;
> >
> > Put this all in private.
> >
> done
> > > };
> > >
> > > #define EM_FORMAT_HTML_DISPLAY_SEARCH_PRIMARY (0)
> > > Index: em-format-html.c
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-format-html.c,v
> > > retrieving revision 1.80
> > > diff -u -p -r1.80 em-format-html.c
> > > --- em-format-html.c 1 Jul 2005 03:29:23 -0000 1.80
> > > +++ em-format-html.c 15 Jul 2005 10:42:20 -0000
> > > @@ -1830,6 +1830,8 @@ efh_format_headers(EMFormatHTML *efh, Ca
> > >
> > > static void efh_format_message(EMFormat *emf, CamelStream *stream,
> > > CamelMimePart *part, const EMFormatHandler *info)
> > > {
> > > + const EMFormatHandler *handle;
> > > +
> > > /* TODO: make this validity stuff a method */
> > > EMFormatHTML *efh = (EMFormatHTML *) emf;
> > > CamelCipherValidity *save = emf->valid, *save_parent =
> > > emf->valid_parent;
> > > @@ -1842,6 +1844,10 @@ static void efh_format_message(EMFormat
> > >
> > > if (!efh->hide_headers)
> > > efh_format_headers(efh, stream, (CamelMedium *)part);
> > > +
> > > + handle = em_format_find_handler(emf,
> > > "x-evolution/message/attachment-bar");
> > > + if (handle)
> > > + handle->handler(emf, stream, part, handle);
> >
> > Ok i guess this is simple. But dont call it attachment-bar,
> > em-format-html knows nothing about attachment bars, that is a purely ui
> > element. Call it
> >
> > x-evolution/message/post-header
> >
> done.
> >
> > > camel_stream_printf(stream, EM_FORMAT_HTML_VPAD);
> > > em_format_part(emf, stream, part);
> > > Index: em-popup.c
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-popup.c,v
> > > retrieving revision 1.36
> > > diff -u -p -r1.36 em-popup.c
> > > --- em-popup.c 17 Jun 2005 15:20:29 -0000 1.36
> > > +++ em-popup.c 15 Jul 2005 10:42:21 -0000
> > > @@ -64,6 +64,7 @@
> > > #include <gconf/gconf-client.h>
> > >
> > > #include <e-util/e-util.h>
> > > +#include "e-attachment.h"
> > >
> > > static void emp_standard_menu_factory(EPopup *emp, void *data);
> > >
> > > @@ -365,8 +366,17 @@ em_popup_target_new_attachments(EMPopup
> > > t->attachments = attachments;
> > > if (len > 0)
> > > mask &= ~ EM_POPUP_ATTACHMENTS_MANY;
> > > - if (len == 1)
> > > + if (len == 1) {
> > > + char *mime =
> > > camel_data_wrapper_get_mime_type((CamelDataWrapper *)((EAttachment *)
> > > attachments->data)->body);
> > > +
> > > + if (!strncasecmp(mime, "image/", 6))
> >
> > You can't use strncasecmp for ascii text. Use g_ascii_strncasecmp.
> >
> > (i is a particularly problematic character).
> >
> > Besides, dont use this at all, use camel_content_type_is() on the
> > relevent structure.
> >
> okie. I saw a similar function and wrote it. Probably it also should be
> changed then(em_popup_target_new_part).
>
> > > + mask &= ~ EM_POPUP_ATTACHMENTS_IMAGE;
> > > + g_free(mime);
> > > +
> > > mask &= ~ EM_POPUP_ATTACHMENTS_ONE;
> > > + }
> > > + if (len > 1)
> > > + mask &= ~ EM_POPUP_ATTACHMENTS_MULTIPLE;
> > > t->target.mask = mask;
> > >
> > > return t;
> > > @@ -377,26 +387,43 @@ em_popup_target_new_attachments(EMPopup
> > > static void
> > > emp_part_popup_saveas(EPopup *ep, EPopupItem *item, void *data)
> > > {
> > > - EMPopupTargetPart *t = (EMPopupTargetPart *)ep->target;
> > > + EPopupTarget *t = ep->target;
> > > + CamelMimePart *part = NULL;
> > >
> > > - em_utils_save_part(ep->target->widget, _("Save As..."),
> > > t->part);
> > > + /* If it is of type EM_POPUP_TARGET_ATTACHMENTS, we can assume
> > > the length is one. */
> > > + if (t->type == EM_POPUP_TARGET_ATTACHMENTS)
> > > + part = ((EAttachment *) ((EMPopupTargetAttachments *)
> > > t)->attachments->data)->body;
> > > + else if (t->type == EM_POPUP_TARGET_PART)
> > > + part = ((EMPopupTargetPart *) t)->part;
> > > + else
> > > + return;
> > > +
> > > + em_utils_save_part(ep->target->widget, _("Save As..."), part);
> >
> > Dont make this function so complicated, just make a new one for
> > attachment_saveas.
> >
> notzed, i dont think it is so compliacted to duplicate it. Just gets the
> part based on the type. I dont think its useful to duplicate to two
> functions.
> > > }
> > >
> > > static void
> > > emp_part_popup_set_background(EPopup *ep, EPopupItem *item, void
> > > *data)
> > > {
> > > - EMPopupTargetPart *t = (EMPopupTargetPart *)ep->target;
> > > + EPopupTarget *t = ep->target;
> > > GConfClient *gconf;
> > > char *str, *filename, *path, *extension;
> > > unsigned int i=1;
> > > + CamelMimePart *part = NULL;
> > > +
> > > + if (t->type == EM_POPUP_TARGET_ATTACHMENTS)
> > > + part = ((EAttachment *) ((EMPopupTargetAttachments *)
> > > t)->attachments->data)->body;
> > > + else if (t->type == EM_POPUP_TARGET_PART)
> > > + part = ((EMPopupTargetPart *) t)->part;
> > > + else
> > > + return;
> >
> > Same here.
> >
> >
> > > - filename = g_strdup(camel_mime_part_get_filename(t->part));
> > > + filename = g_strdup(camel_mime_part_get_filename(part));
> > >
> > > /* if filename is blank, create a default filename based on
> > > MIME type */
> > > if (!filename || !filename[0]) {
> > > CamelContentType *ct;
> > >
> > > - ct = camel_mime_part_get_content_type(t->part);
> > > + ct = camel_mime_part_get_content_type(part);
> > > g_free (filename);
> > > filename = g_strdup_printf (_("untitled_image.%s"),
> > > ct->subtype);
> > > }
> > > @@ -420,7 +447,7 @@ emp_part_popup_set_background(EPopup *ep
> > >
> > > g_free(filename);
> > >
> > > - if (em_utils_save_part_to_file(ep->target->widget, path,
> > > t->part)) {
> > > + if (em_utils_save_part_to_file(ep->target->widget, path,
> > > part)) {
> > > gconf = gconf_client_get_default();
> > >
> > > /* if the filename hasn't changed, blank the filename
> > > before
> > > @@ -501,6 +528,11 @@ static EMPopupItem emp_standard_object_p
> > > { E_POPUP_ITEM, "20.part.00", N_("_Forward"),
> > > emp_part_popup_forward, NULL, "stock_mail-forward",
> > > EM_POPUP_PART_MESSAGE },
> > > };
> > >
> > > +static EMPopupItem emp_attachment_object_popups[] = {
> > > + { E_POPUP_ITEM, "00.part.00", N_("_Save As..."),
> > > emp_part_popup_saveas, NULL, "stock_save-as", 0 },
> > > + { E_POPUP_ITEM, "00.part.10", N_("Set as _Background"),
> > > emp_part_popup_set_background, NULL, NULL,
> > > EM_POPUP_ATTACHMENTS_IMAGE },
> > > +};
> > > +
> > > static const EPopupItem emp_standard_part_apps_bar = { E_POPUP_BAR,
> > > "99.object" };
> > >
> > > /*
> > > **********************************************************************
> > > */
> > > @@ -563,9 +595,17 @@ static void
> > > emp_apps_open_in(EPopup *ep, EPopupItem *item, void *data)
> > > {
> > > char *path;
> > > - EMPopupTargetPart *target = (EMPopupTargetPart *)ep->target;
> > > + EPopupTarget *target = ep->target;
> > > + CamelMimePart *part;
> > > +
> > > + if (target->type == EM_POPUP_TARGET_ATTACHMENTS)
> > > + part = ((EAttachment *) ((EMPopupTargetAttachments *)
> > > target)->attachments->data)->body;
> > > + else if (target->type == EM_POPUP_TARGET_PART)
> > > + part = ((EMPopupTargetPart *) target)->part;
> > > + else
> > > + return;
> >
> > you can't get an unknown target type here.
> >
> > besides, again dont make this function generic, unless you add another
> > layer and reuse it. it just makes the code more complicated.
> >
> >
> > > - path = em_utils_temp_save_part(target->target.widget,
> > > target->part);
> > > + path = em_utils_temp_save_part(target->widget, part);
> > > if (path) {
> > > GnomeVFSMimeApplication *app = item->user_data;
> > > char *uri;
> > > @@ -680,6 +720,75 @@ emp_standard_menu_factory(EPopup *emp, v
> > > items = emp_standard_object_popups;
> > > len = LEN(emp_standard_object_popups);
> > > break; }
> > > + case EM_POPUP_TARGET_ATTACHMENTS: {
> > > + EMPopupTargetAttachments *t =
> > > (EMPopupTargetAttachments *)emp->target;
> > > + GSList *list = t->attachments;
> > > + GList *apps;
> > > + EAttachment *attachment;
> > > + char *mime_type;
> > > +
> > > + if (g_slist_length(list) != 1) {
> > > + items = NULL;
> > > + len = 0;
> > > + break;
> > > + }
> > > +
> > > + /* Only one attachment selected */
> > > + attachment = list->data;
> > > + mime_type =
> > > camel_data_wrapper_get_mime_type((CamelDataWrapper
> > > *)attachment->body);
> > > + apps = gnome_vfs_mime_get_all_applications(mime_type);
> > > +
> > > + /* FIXME: use the snoop_part stuff from em-format.c */
> > > + if (apps == NULL && strcmp(mime_type,
> > > "application/octet-stream") == 0) {
> > > + const char *filename, *name_type;
> > > +
> > > + filename =
> > > camel_mime_part_get_filename(attachment->body);
> > > +
> > > + if (filename) {
> > > + /* GNOME-VFS will misidentify TNEF
> > > attachments as MPEG */
> > > + if (!strcmp (filename, "winmail.dat"))
> > > + name_type =
> > > "application/vnd.ms-tnef";
> > > + else
> > > + name_type =
> > > gnome_vfs_mime_type_from_name(filename);
> > > + if (name_type)
> > > + apps =
> > > gnome_vfs_mime_get_all_applications(name_type);
> > > + }
> > > + }
> > > +
> > > + if (apps) {
> > > + GString *label = g_string_new("");
> > > + GSList *open_menus = NULL;
> > > + GList *l;
> > > +
> > > + menus = g_slist_prepend(menus, (void
> > > *)&emp_standard_part_apps_bar);
> > > +
> > > + for (l = apps, i = 0; l; l = l->next, i++) {
> > > + GnomeVFSMimeApplication *app =
> > > l->data;
> > > + EPopupItem *item;
> > > +
> > > + if (app->requires_terminal)
> > > + continue;
> > > +
> > > + item = g_malloc0(sizeof(*item));
> > > + item->type = E_POPUP_ITEM;
> > > + item->path =
> > > g_strdup_printf("99.object.%02d", i);
> > > + item->label = g_strdup_printf(_("Open
> > > in %s..."), app->name);
> > > + item->activate = emp_apps_open_in;
> > > + item->user_data = app;
> > > +
> > > + open_menus =
> > > g_slist_prepend(open_menus, item);
> > > + }
> > > +
> > > + if (open_menus)
> > > + e_popup_add_items(emp, open_menus,
> > > NULL, emp_apps_popup_free, NULL);
> > > +
> > > + g_string_free(label, TRUE);
> > > + g_list_free(apps);
> > > + }
> > > +
> > > + items = emp_attachment_object_popups;
> > > + len = LEN(emp_attachment_object_popups);
> > > + break; }
> >
> > Ahh so here you just copy all the code ... strange, after all the weird
> > code re-arragments above.
> >
> > Separate out the 'open in' logic, and re-use it for both cases.
> >
> OKie. restructured and reused the code.
> > > default:
> > > items = NULL;
> > > len = 0;
> > > @@ -769,6 +878,8 @@ static const EPopupHookTargetMask emph_f
> > > static const EPopupHookTargetMask emph_attachments_masks[] = {
> > > { "one", EM_POPUP_ATTACHMENTS_ONE },
> > > { "many", EM_POPUP_ATTACHMENTS_MANY },
> > > + { "multiple", EM_POPUP_ATTACHMENTS_MULTIPLE },
> > > + { "image", EM_POPUP_ATTACHMENTS_IMAGE },
> > > { 0 }
> > > };
> > >
> > > Index: em-popup.h
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-popup.h,v
> > > retrieving revision 1.11
> > > diff -u -p -r1.11 em-popup.h
> > > --- em-popup.h 24 Jan 2005 14:46:24 -0000 1.11
> > > +++ em-popup.h 15 Jul 2005 10:42:21 -0000
> > > @@ -161,6 +161,8 @@ enum _em_popup_target_folder_t {
> > > enum _em_popup_target_attachments_t {
> > > EM_POPUP_ATTACHMENTS_ONE = 1<<0, /* only 1 selected */
> > > EM_POPUP_ATTACHMENTS_MANY = 1<<1, /* one or more selected */
> > > + EM_POPUP_ATTACHMENTS_MULTIPLE = 1<<2, /* More than 1 selected
> > > */
> > > + EM_POPUP_ATTACHMENTS_IMAGE = 1<<3, /* Image selected */
> > > };
> > >
> > > typedef struct _EMPopupTargetSelect EMPopupTargetSelect;
> > > Index: em-utils.c
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-utils.c,v
> > > retrieving revision 1.61
> > > diff -u -p -r1.61 em-utils.c
> > > --- em-utils.c 17 Jun 2005 15:20:29 -0000 1.61
> > > +++ em-utils.c 15 Jul 2005 10:42:21 -0000
> > > @@ -300,7 +300,7 @@ em_utils_edit_filters (GtkWidget *parent
> > > /* Saving messages... */
> > >
> > > static GtkWidget *
> > > -emu_get_save_filesel (GtkWidget *parent, const char *title, const
> > > char *name)
> > > +emu_get_save_filesel (GtkWidget *parent, const char *title, const
> > > char *name, GtkFileChooserAction action)
> > > {
> > > GtkWidget *filesel;
> > > const char *dir;
> > > @@ -310,7 +310,7 @@ emu_get_save_filesel (GtkWidget *parent,
> > > #ifdef USE_GTKFILECHOOSER
> > > filesel = gtk_file_chooser_dialog_new (title,
> > > NULL,
> > > -
> > > GTK_FILE_CHOOSER_ACTION_SAVE,
> > > + action,
> > > GTK_STOCK_CANCEL,
> > > GTK_RESPONSE_CANCEL,
> > > GTK_STOCK_SAVE,
> > > GTK_RESPONSE_OK,
> > > NULL);
> > > @@ -439,12 +439,71 @@ em_utils_save_part(GtkWidget *parent, co
> > > }
> > > }
> > >
> > > - filesel = emu_get_save_filesel(parent, prompt, name);
> > > + filesel = emu_get_save_filesel(parent, prompt, name,
> > > GTK_FILE_CHOOSER_ACTION_SAVE);
> > > camel_object_ref(part);
> > > g_signal_connect (filesel, "response", G_CALLBACK
> > > (emu_save_part_response), part);
> > > gtk_widget_show (filesel);
> > > }
> > >
> > > +static void
> > > +emu_save_parts_response (GtkWidget *filesel, int response, GSList
> > > *parts)
> > > +{
> > > + char *path = NULL;
> > > + GSList *selected;
> > > + if (response == GTK_RESPONSE_OK) {
> > > +#ifdef USE_GTKFILECHOOSER
> > > + path = gtk_file_chooser_get_current_folder
> > > (GTK_FILE_CHOOSER (filesel));
> > > +#else
> > > + path = gtk_file_selection_get_filename
> > > (GTK_FILE_SELECTION (filesel));
> > > +#endif
> > > +
> > > + emu_update_save_path(path);
> > > +
> > > + for ( selected = parts; selected != NULL; selected =
> > > selected->next) {
> > > + const char *file_name;
> > > + char *safe_name = NULL;
> > > + char *file_path;
> > > + CamelMimePart *part = selected->data;
> > > +
> > > + file_name =
> > > camel_mime_part_get_filename(part);
> > > + if (file_name == NULL) {
> > > + if (CAMEL_IS_MIME_MESSAGE(part)) {
> > > + file_name =
> > > camel_mime_message_get_subject((CamelMimeMessage *)part);
> > > + if (file_name == NULL)
> > > + file_name =
> > > _("message");
> > > + } else {
> > > + file_name = _("attachment");
> > > + }
> > > + } else {
> > > + safe_name = g_strdup(file_name);
> > > + e_filename_make_safe(safe_name);
> > > + file_name = safe_name;
> > > + }
> > > +
> > > + file_path = g_build_filename (path, file_name,
> > > NULL);
> > > + mail_save_part(part, file_path, NULL, NULL);
> > > + g_free (file_path);
> > > + g_free (safe_name);
> > > + }
> > > +
> > > + g_free (path);
> > > + }
> > > +
> > > + g_slist_free (parts);
> > > + gtk_widget_destroy((GtkWidget *)filesel);
> > > +}
> > > +
> > > +void
> > > +em_utils_save_parts (GtkWidget *parent, const char *prompt, GSList *
> > > parts)
> > > +{
> > > + GtkWidget *filesel;
> > > +
> > > + filesel = emu_get_save_filesel (parent, prompt, NULL,
> > > GTK_FILE_CHOOSER_ACTION_SELECT_FOLDER);
> > > + g_signal_connect (filesel, "response", G_CALLBACK
> > > (emu_save_parts_response), parts);
> > > + gtk_widget_show (filesel);
> > > +}
> > > +
> > > +
> > > /**
> > > * em_utils_save_part_to_file:
> > > * @parent: parent window
> > > @@ -542,7 +601,7 @@ em_utils_save_messages (GtkWidget *paren
> > > g_return_if_fail (CAMEL_IS_FOLDER (folder));
> > > g_return_if_fail (uids != NULL);
> > >
> > > - filesel = emu_get_save_filesel(parent, _("Save Message..."),
> > > NULL);
> > > + filesel = emu_get_save_filesel(parent, _("Save Message..."),
> > > NULL, GTK_FILE_CHOOSER_ACTION_SAVE);
> > > camel_object_ref(folder);
> > >
> > > data = g_malloc(sizeof(struct _save_messages_data));
> > > Index: em-utils.h
> > > ===================================================================
> > > RCS file: /cvs/gnome/evolution/mail/em-utils.h,v
> > > retrieving revision 1.17
> > > diff -u -p -r1.17 em-utils.h
> > > --- em-utils.h 16 May 2005 07:53:53 -0000 1.17
> > > +++ em-utils.h 15 Jul 2005 10:42:21 -0000
> > > @@ -77,6 +77,7 @@ void em_utils_selection_set_urilist(stru
> > > void em_utils_selection_get_urilist(struct _GtkSelectionData *data,
> > > struct _CamelFolder *folder);
> > >
> > > char *em_utils_temp_save_part(struct _GtkWidget *parent, struct
> > > _CamelMimePart *part);
> > > +void em_utils_save_parts (struct _GtkWidget *parent, const char
> > > *prompt, GSList * parts);
> > >
> > > gboolean em_utils_folder_is_drafts(struct _CamelFolder *folder, const
> > > char *uri);
> > > gboolean em_utils_folder_is_sent(struct _CamelFolder *folder, const
> > > char *uri);
> >
> > _______________________________________________
> > evolution-patches mailing list
> > evolution-patches lists ximian com
> > http://lists.ximian.com/mailman/listinfo/evolution-patches
> _______________________________________________
> evolution-patches mailing list
> evolution-patches lists ximian com
> http://lists.ximian.com/mailman/listinfo/evolution-patches
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
