Re: [evolution-patches] New Attachment Bar UI+Patch




On Wed, 2005-07-13 at 15:10 +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    13 Jul 2005 09:18:43 -0000
> @@ -30,6 +30,7 @@
>  #include <gtkhtml/gtkhtml-embedded.h>
>  #include <gtkhtml/gtkhtml-search.h>
>  
> +#include <gtk/gtk.h>

Jeff already commented on this.  Why do you think all the other crap is
below this?  For fun?

>  #include <gtk/gtkeventbox.h>
>  #include <gtk/gtkvbox.h>
>  #include <gtk/gtkhbox.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"
>  
>  #define d(x)
>  
> @@ -109,6 +112,7 @@ 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);
> +void efhd_add_attachment_bar(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part);

Uh, must be static.

>  
>  struct _attach_puri {
>         EMFormatPURI puri;
> @@ -136,6 +140,7 @@ static void efhd_iframe_created(GtkHTML 
>    static gboolean efhd_object_requested(GtkHTML *html,
> GtkHTMLEmbedded *eb, EMFormatHTMLDisplay *efh);*/
>  
>  static void efhd_message_prefix(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part, EMFormatHandler *info);
> +static void efhd_format_message(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part, const EMFormatHandler *info);
>  
>  static const EMFormatHandler *efhd_find_handler(EMFormat *emf, const
> char *mime_type);
>  static void efhd_format_clone(EMFormat *, CamelFolder *folder, const
> char *, CamelMimeMessage *msg, EMFormat *);
> @@ -959,6 +964,7 @@ static EMFormatHandler type_builtin_tabl
>         { "image/pjpeg", (EMFormatFunc)efhd_image },
>  
>         { "x-evolution/message/prefix",
> (EMFormatFunc)efhd_message_prefix },
> +       { "x-evolution/message/rfc822",
> (EMFormatFunc)efhd_format_message },

This isn't a plugin, this is the wrong place to put it.


>  };
>  
>  static void
> @@ -1080,6 +1086,36 @@ static void efhd_message_prefix(EMFormat
>         camel_stream_printf(stream, "</td></tr></table>");
>  }
>  
> +static void efhd_format_message(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part, const EMFormatHandler *info)
> +{

This is for formatting messages, and message ATTACHMENTS.  Don't do
this.

It should go in the format_clone code instead, or something.

> +       /* TODO: make this validity stuff a method */
> +       EMFormatHTML *efh = (EMFormatHTML *) emf;
> +       CamelCipherValidity *save = emf->valid, *save_parent =
> emf->valid_parent;
> +
> +       emf->valid = NULL;
> +       emf->valid_parent = NULL;
> +
> +       if (emf->message != (CamelMimeMessage *)part)
> +               camel_stream_printf(stream, "<blockquote>\n");
> +
> +       if (!efh->hide_headers)
> +               em_format_html_format_headers(efh, stream,
> (CamelMedium *)part);
> +       
> +       if (!emf->attachment_bar)
> +               efhd_add_attachment_bar (emf, stream, part);
> +       
> +       camel_stream_printf(stream, EM_FORMAT_HTML_VPAD);
> +       em_format_part(emf, stream, part);
> +
> +       if (emf->message != (CamelMimeMessage *)part)
> +               camel_stream_printf(stream, "</blockquote>\n");
> +
> +       camel_cipher_validity_free(emf->valid);
> +
> +       emf->valid = save;
> +       emf->valid_parent = save_parent;
> +}
> +
>  /* TODO: if these aren't going to do anything should remove */
>  static void efhd_format_error(EMFormat *emf, CamelStream *stream,
> const char *txt)
>  {
> @@ -1644,6 +1680,326 @@ type_ok:
>  }
>  
>  static void
> +attachment_bar_arrow_clicked(GtkWidget *w, EMFormat *emf)
> +{
> +       if (emf->show_bar) {
> +               gtk_widget_hide(emf->attachment_bar);
> +               gtk_arrow_set((GtkArrow
> *)gtk_tool_button_get_icon_widget((GtkToolButton *)w),
> GTK_ARROW_RIGHT, GTK_SHADOW_NONE);
> +       } else {
> +               gtk_widget_show(emf->attachment_bar);
> +               gtk_arrow_set((GtkArrow
> *)gtk_tool_button_get_icon_widget((GtkToolButton *)w), GTK_ARROW_DOWN,
> GTK_SHADOW_NONE);
> +       }

Why not do this the same way the other code does it?  Use a hbox and
hide/show as appropriate.

> +       emf->show_bar = !emf->show_bar;
> +}

Umm, also, instead of making your life difficult by using reverse logic
pre-change, please toggle show_bar before this.

> +static void
> +attachments_save_all_clicked(GtkWidget *w, EMFormat *emf)
> +{
> +       GSList *attachment_parts;
> +
> +       attachment_parts =
> e_attachment_bar_get_attachment_part_list(E_ATTACHMENT_BAR(emf->attachment_bar));
> +       em_utils_save_selected_parts(w, _("Select folder to save all
> attachments..."), attachment_parts);
> +}

Please fix the e-attachment-bar api as per other comments before using
it here.

> +static void
> +eab_popup_position(GtkMenu *menu, int *x, int *y, gboolean *push_in,
> gpointer user_data)

WTF is eab?  Stick to the proper namespace.

> +{
> +       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
> +eab_drag_data_get(EAttachmentBar *bar, GdkDragContext *drag,
> GtkSelectionData *data, guint info, guint time, EMFormat *emf)

Again, stick to the namespace.

Why is the dnd stuff even here?  It should be handled by the attachment
bar itself.

> +{
> +       GnomeIconList *icon_list = GNOME_ICON_LIST(bar);
> +       char *path;
> +       GList *selected = NULL;
> +       GSList *attachments = NULL, *tmp;
> +       gchar **uris;
> +       int length, i=0;
> +
> +       if (info)
> +               return;
> +
> +       selected = gnome_icon_list_get_selection(icon_list);
> +       length = g_list_length (selected);
> +
> +       uris = g_malloc0(sizeof(bar) * (length+1));
> +
> +       attachments = e_attachment_bar_get_attachment (bar, -1);

Um,m fix the attachment bar api, it should have a function to get
selected attachments.

How odd, you get the selection, but then do nothing with it, and just
process every attachment.

> +       for (tmp = attachments; tmp; tmp = tmp->next,i++) {
> +               EAttachment *attachment = tmp->data;
> +               char *uri;
> +
> +               uri = g_object_get_data((GObject *)attachment,
> "e-drag-uri");
> +               if (uri) {
> +                       uris[i] = uri;
> +                       continue;
> +               }
> +               path = em_utils_temp_save_part(emf->attachment_bar,
> attachment->body);
> +               if (path == NULL)
> +                       return;
> +               uri = g_strdup_printf("file://%s\r\n", path);
> +               g_free(path);
> +               g_object_set_data_full((GObject *)attachment,
> "e-drag-uri", uri, g_free);
> +               uris[i] = uri;
> +       }
> +       uris[i]=0;
> +       gtk_selection_data_set_uris(data, uris);
> +       
> +       g_slist_foreach(attachments, (GFunc)g_object_unref, NULL);
> +       g_free (uris);
> +
> +       return;
> +}
> +
> +static void
> +eab_save_selected(EPopup *ep, EPopupItem *item, EMFormat *emf)

Namespace.

> +{
> +       GSList *attachment_parts, *tmp;
> +       GSList *parts = NULL;
> +
> +       attachment_parts =
> e_attachment_bar_get_attachment(E_ATTACHMENT_BAR(emf->attachment_bar),
> -1);
> +       
> +       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_selected_parts(emf->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 eab_menu_items[] = {
> +       { E_POPUP_BAR, "05.display", },
> +       { E_POPUP_ITEM, "05.display.01", N_("Save Selected"),
> eab_save_selected, },
> +};
> +
> +static gboolean
> +eab_button_release_event(EAttachmentBar *bar, GdkEventButton *event,
> EMFormat *emf)
> +{

all the dnd stuff belongs in the e-attachment-bar.

> +       GnomeIconList *icon_list = GNOME_ICON_LIST(bar);
> +       GList *selected;
> +       int length;
> +       GtkTargetEntry drag_types[] = {
> +               { "text/uri-list", 0, 0 },
> +       };      
> +
> +       if (event && event->button != 1)
> +               return FALSE;
> +
> +       selected = gnome_icon_list_get_selection(icon_list);
> +       length = g_list_length (selected);
> +
> +       if (length)
> +               gtk_drag_source_set(emf->attachment_bar,
> GDK_BUTTON1_MASK, drag_types,
> sizeof(drag_types)/sizeof(drag_types[0]), GDK_ACTION_COPY);
> +       else
> +               gtk_drag_source_unset(emf->attachment_bar);
> +       return FALSE;
> +}
> +
> +static gboolean
> +eab_button_press_event(EAttachmentBar *bar, GdkEventButton *event,
> EMFormat *emf)
> +{
> +       GnomeIconList *icon_list = GNOME_ICON_LIST(bar);
> +       GtkMenu *menu;
> +       GSList *list=NULL;
> +       EAttachment *attachment;
> +       GList *selected = NULL, *tmp;
> +       EPopupTarget *target;
> +       EMPopup *emp;
> +       GSList *menus = NULL;
> +       int length, icon_number;
> +       gboolean take_selected = FALSE;
> +       GtkTargetEntry drag_types[] = {
> +               { "text/uri-list", 0, 0 },
> +       };      
> +
> +       selected = gnome_icon_list_get_selection(icon_list);
> +       length = g_list_length(selected);
> +       
> +       if (event) {
> +               icon_number = gnome_icon_list_get_icon_at(icon_list,
> event->x, event->y);
> +               if (icon_number < 0) { 
> +                       /* When nothing is selected deselect all*/
> +                       gnome_icon_list_unselect_all(icon_list);
> +                       length = 0;
> +                       selected = NULL;
> +               }
> +       
> +               if (event->button == 1) {
> +                       /* If something is selected, then allow drag
> or else help to select */
> +                       if (length)
> +                               gtk_drag_source_set(emf->attachment_bar, GDK_BUTTON1_MASK, drag_types, sizeof(drag_types)/sizeof(drag_types[0]), GDK_ACTION_COPY);
> +                       else
> +                               gtk_drag_source_unset(emf->attachment_bar);
> +                       return FALSE;
> +               }
> +       
> +               /* If not r-click dont progress any more.*/
> +               if (event->button != 3)
> +                       return FALSE;   
> +
> +               /* When a r-click on something, if it is in the
> already selected list, consider a r-click of multiple things
> +                * or deselect all and select only this for r-click 
> +                */
> +               if (icon_number >= 0) {
> +                       for (tmp = selected; tmp; tmp = tmp->next) {
> +                               if (GPOINTER_TO_INT(tmp->data) ==
> icon_number)
> +                                       take_selected = TRUE;
> +                       }
> +       
> +                       if (!take_selected) {
> +                               gnome_icon_list_unselect_all(icon_list);
> +                               gnome_icon_list_select_icon(icon_list,
> icon_number);
> +                               selected =
> gnome_icon_list_get_selection(icon_list);
> +                               length = g_list_length(selected);
> +                       }
> +               }
> +       } 
> +       
> +       if (length == 0)
> +               return TRUE;
> +
> +       /** @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");
> +
> +       if (length == 1) {
> +               list = e_attachment_bar_get_attachment (bar,
> GPOINTER_TO_INT(selected->data));
> +               attachment = list->data;
> +               target = (EPopupTarget *)em_popup_target_new_part(emp,
> attachment->body, NULL);
> +               g_object_unref (attachment);
> +               g_slist_free (list);
> +               list = NULL;

You shouldn't use a different target here, use the attachments target.

> +       } else {
> +               int i;
> +               /* Add something like save-selected, foward selected
> attachments in a mail etc....*/
> +               list = e_attachment_bar_get_attachment (bar, -1);
> +               target = (EPopupTarget
> *)em_popup_target_new_attachments(emp, list);
> +               for (i=0; i<2; i++)
> +                       menus = g_slist_prepend(menus,
> &eab_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)eab_popup_position, bar, 0,
> gtk_get_current_event_time());
> +
> +       return TRUE;
> +
> +}

I'm not sure if it even belongs here, or on the e-attachment-bar more
directly.

> +static gboolean
> +eab_popup_menu_event (EAttachmentBar *bar, EMFormat *emf) {
> +       return eab_button_press_event(bar, NULL, emf);
> +}
> +
> +
> +static gboolean
> +efhd_add_bar(EMFormatHTML *efh, GtkHTMLEmbedded *eb,
> EMFormatHTMLPObject *pobject)
> +{
> +       EMFormat *emf = (EMFormat *)efh;
> +       GtkWidget *hbox2, *hbox3, *vbox ;
> +       int width, height;
> +       GtkTargetEntry drag_types[] = {
> +               { "text/uri-list", 0, 0 },
> +       };      
> +       
> +       hbox2 = gtk_hbox_new (FALSE, 0);
> +
> +       gtk_box_pack_start ((GtkBox *)hbox2, emf->arrow, FALSE, FALSE,
> 0);
> +       gtk_box_pack_start ((GtkBox *)hbox2, emf->label, FALSE, FALSE,
> 2);
> +       gtk_box_pack_start ((GtkBox *)hbox2, emf->save, FALSE, FALSE,
> 2);
> +
> +
> +       gtk_widget_get_size_request(emf->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 (emf->attachment_bar, 
> +                                       ((GtkWidget
> *)efh->html)->parent->allocation.width - /* FIXME */36,
> +                                       84 /* FIXME: Default show only
> one row, Dont hardcode size*/);
> +        GTK_WIDGET_SET_FLAGS (emf->attachment_bar, GTK_CAN_FOCUS);
> +
> +       hbox3 = gtk_hbox_new (FALSE, 0);
> +       gtk_box_pack_start ((GtkBox *)hbox3, emf->attachment_bar,
> TRUE, TRUE, 0);
> +       
> +       vbox = gtk_vbox_new (FALSE, 0);
> +       gtk_box_pack_start ((GtkBox *)vbox, hbox2, FALSE, FALSE, 2);
> +       gtk_box_pack_start ((GtkBox *)vbox, hbox3, TRUE, TRUE, 2);
> +
> +       gtk_container_add ((GtkContainer *)eb, vbox);
> +       gtk_widget_show_all ((GtkWidget *)eb);
> +       if (!emf->show_bar)
> +               gtk_widget_hide (emf->attachment_bar);
> +
> +       g_signal_connect (emf->arrow, "clicked",
> G_CALLBACK(attachment_bar_arrow_clicked), efh);
> +       g_signal_connect (emf->attachment_bar, "button_press_event",
> G_CALLBACK(eab_button_press_event), emf);
> +       g_signal_connect (emf->attachment_bar, "button_release_event",
> G_CALLBACK(eab_button_release_event), emf);      
> +       g_signal_connect (emf->attachment_bar, "popup-menu",
> G_CALLBACK(eab_popup_menu_event), emf);
> +       gtk_drag_source_set(emf->attachment_bar, GDK_BUTTON1_MASK,
> drag_types, sizeof(drag_types)/sizeof(drag_types[0]),
> GDK_ACTION_COPY);      
> +       g_signal_connect(emf->attachment_bar, "drag-data-get",
> G_CALLBACK(eab_drag_data_get), emf);     
> +       g_signal_connect (emf->save, "clicked",
> G_CALLBACK(attachments_save_all_clicked), efh);
> +
> +       return TRUE;
> +}
> +
> +void
> +efhd_add_attachment_bar(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part)
> +{
> +       GtkWidget *image, *txt, *hbox1;
> +       const char *classid = "attachment-bar";
> +
> +       emf->attachment_bar = e_attachment_bar_new(NULL);
> +       if (emf->show_bar)
> +               emf->arrow = (GtkWidget
> *)gtk_tool_button_new(gtk_arrow_new (GTK_ARROW_DOWN, GTK_SHADOW_NONE),
> NULL);
> +       else            
> +               emf->arrow = (GtkWidget
> *)gtk_tool_button_new(gtk_arrow_new (GTK_ARROW_RIGHT,
> GTK_SHADOW_NONE), NULL);
> +
> +       emf->label = gtk_label_new("No Attachment");

English text needs translating.

> +       emf->save = gtk_button_new();

Why do you need to keep this pointer around?

> +       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 *)emf->save, hbox1);
> +
> +       gtk_widget_set_sensitive(emf->arrow, FALSE);
> +       gtk_widget_set_sensitive(emf->save, FALSE);
> +       

^^ This is all wrong anyway.  You can't call ANY GTK METHODS from a
formatter.

> +       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 +2011,45 @@ 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;
> +
> +       /* FIXME: Find how to get all leaf nodes. This gets me only
> the level if disposition is inline. What about disposition attachment
> */
> +       if (!
> camel_content_type_is(camel_medium_get_content_object((CamelMedium
> *)part)->mime_type, "message", "rfc822") &&
> em_format_is_attachment(emf, part))

WTF is this test for?  _Everything_ that comes here is ALREADY an
attachment.

> +       {
> +               EAttachment *new;
> +               const char *file = camel_mime_part_get_filename(part);
> +               char *tmp, *new_file = NULL;
> +
> +               /* FIXME: Attachments with out filenames?, How to
> handle them. */

Easy, you mustn't use the filename as the key.

> +               if (file) {
> +                       tmp = g_hash_table_lookup (emf->files, file);
> +                       if (tmp) {
> +                               guint count = GPOINTER_TO_UINT(tmp);
> +                               gchar** tokens;
> +                               tokens = g_strsplit(file, ".", 2);
> +                               new_file = g_strdup_printf("%s(%d).%
> s", tokens[0], count++, tokens[1]);
> +                               g_hash_table_insert (emf->files,
> g_strdup(file), GUINT_TO_POINTER(count));
> +                               g_strfreev(tokens);

If you want to get a .extension you have to use strrchr, g_strsplit wont
handle multiple .'s.

Style needs fixing anyway.

> +                               /* Set it to part, so that its same
> everywhere and easy to relate wrt filenames*/
> +                               camel_mime_part_set_filename(part,
> new_file);

No, dont set the filename on the part.

> +                               g_free(new_file);
> +               
> +                       } else {
> +                               g_hash_table_insert (emf->files,
> g_strdup(file), GUINT_TO_POINTER(1));
> +                       }
> +       
> +                       new = e_attachment_new_from_mime_part (part);
> +                       /* Store the status of encryption / signature
> on the attachment for emblem display */
> +                       if (emf->valid) {
> +                               if (emf->valid->sign.status)
> +                                       new->sign =
> emf->valid->sign.status;
> +                               if (emf->valid->encrypt.status)
> +                                       new->encrypt =
> emf->valid->encrypt.status;
> +                       }
> +               
> +                       /* Add the attachment to the bar.*/
> +                       e_attachment_bar_add_attachment
> (E_ATTACHMENT_BAR(emf->attachment_bar), new);

This runs in another thread, cna't call gtk+ stuff.

> +               }
> +       }
>  
>         camel_stream_write_string(stream,
>                                   EM_FORMAT_HTML_VPAD
> 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    13 Jul 2005 09:18:43 -0000
> @@ -70,6 +70,7 @@
>  #include "em-format-html.h"
>  #include "em-html-stream.h"
>  #include "em-utils.h"
> +#include "e-attachment-bar.h"

NO NO No.  em-format-html.c contains NO UI stuff.  NONE.  This is a
purely UI feature, it cannot go anywhere in em-format-html, anywhere.
All the dependent code must be removed from this file.

>  
>  #define d(x)
>  
> @@ -1270,6 +1271,46 @@ static char *efh_format_desc(struct _mai
>         return g_strdup(_("Formatting message"));
>  }
>  
> +static void
> +efh_attachment_bar_done (EMFormat *emf)
> +{
> +        PangoFontMetrics *metrics;
> +        PangoContext *context;
> +       int width, height, bar_width, bar_height, nattachments;
> +
> +       if (!emf->attachment_bar)
> +               return;
> +
> +        context = gtk_widget_get_pango_context ((GtkWidget *)
> emf->attachment_bar);
> +        metrics = pango_context_get_metrics (context, ((GtkWidget *)
> emf->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 (emf->attachment_bar, &bar_width,
> &bar_height);
> +       
> +       nattachments = e_attachment_bar_get_num_attachments
> (E_ATTACHMENT_BAR(emf->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 (emf->attachment_bar,
> bar_width, (rows+1) * 84 /* FIXME: Use enum/defines */);
> +               
> +               /* 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 *)emf->label, txt);
> +               g_free (txt);
> +
> +               /* Enable the expander button and the save all
> button.*/
> +               gtk_widget_set_sensitive (emf->arrow, TRUE);
> +               gtk_widget_set_sensitive (emf->save, TRUE);
> +       }
> +}
> +
>  static void efh_format_do(struct _mail_msg *mm)
>  {
>         struct _format_msg *m = (struct _format_msg *)mm;
> @@ -1366,6 +1407,9 @@ static void efh_format_done(struct _mail
>         m->format->load_http_now = FALSE;
>         m->format->priv->format_id = -1;
>         g_signal_emit_by_name(m->format, "complete");
> +
> +       /* Resize the bar and set the number of attachments */
> +       efh_attachment_bar_done((EMFormat *)m->format);
>  }
>  
>  static void efh_format_free(struct _mail_msg *mm)
> @@ -1827,6 +1871,13 @@ efh_format_headers(EMFormatHTML *efh, Ca
>                 camel_stream_printf (stream,
> "</tr></table>\n</font>\n");
>         }
>  }
> +
> +void
> +em_format_html_format_headers (EMFormatHTML *efh, CamelStream
> *stream, CamelMedium *part)
> +{
> +       efh_format_headers(efh, stream, part);
> +}
> +
>  
>  static void efh_format_message(EMFormat *emf, CamelStream *stream,
> CamelMimePart *part, const EMFormatHandler *info)
>  {
> Index: em-format.c
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-format.c,v
> retrieving revision 1.48
> diff -u -p -r1.48 em-format.c
> --- em-format.c 1 Jul 2005 03:29:23 -0000       1.48
> +++ em-format.c 13 Jul 2005 09:18:44 -0000
> @@ -121,6 +121,8 @@ emf_init(GObject *o)
>         e_dlist_init(&emf->header_list);
>         em_format_default_headers(emf);
>         emf->part_id = g_string_new("");
> +       emf->show_bar = FALSE;
> +       emf->files = g_hash_table_new_full (g_str_hash, g_int_equal,
> g_free, NULL);

Ok, so em-format is even LESS UI related than em-format-html.

NONE of this should be here at all.

>  }
>  
>  static void
> @@ -139,6 +141,8 @@ emf_finalise(GObject *o)
>         g_free(emf->charset);
>         g_free (emf->default_charset);
>         g_string_free(emf->part_id, TRUE);
> +       if (emf->files)
> +               g_hash_table_destroy(emf->files);

That means all this stuff too.

>         /* FIXME: check pending jobs */
>         
> @@ -648,8 +652,17 @@ emf_format_clone(EMFormat *emf, CamelFol
>                         em_format_clear_headers(emf);
>                         for (h = (struct _EMFormatHeader
> *)emfsource->header_list.head; h->next; h = h->next)
>                                 em_format_add_header(emf, h->name,
> h->flags);
> +                       emf->show_bar = emfsource->show_bar;
> +               } else {
> +                       emf->show_bar = FALSE;
>                 }
>         }
> +
> +       /* Reset the attachment bar */
> +       emf->attachment_bar = NULL;
> +       if (emf->files)
> +               g_hash_table_destroy(emf->files);
> +       emf->files = g_hash_table_new_full (g_str_hash, g_int_equal,
> g_free, NULL);
>  
>         /* what a mess */
>         if (folder != emf->folder) {
> Index: em-format.h
> ===================================================================
> RCS file: /cvs/gnome/evolution/mail/em-format.h,v
> retrieving revision 1.16
> diff -u -p -r1.16 em-format.h
> --- em-format.h 19 May 2005 06:06:35 -0000      1.16
> +++ em-format.h 13 Jul 2005 09:18:44 -0000
> @@ -28,6 +28,7 @@
>  #define _EM_FORMAT_H
>  
>  #include <glib-object.h>
> +#include <gtk/gtk.h>

No, this file cannot include gtk.

>  #include "libedataserver/e-msgport.h"
>  
>  struct _CamelStream;
> @@ -226,6 +227,14 @@ struct _EMFormat {
>         em_format_mode_t mode;  /* source/headers/etc */
>         char *charset;          /* charset override */
>         char *default_charset;  /* charset fallback */
> +
> +       /* for Attachment bar */
> +       GtkWidget *attachment_bar;
> +       GtkWidget *label;
> +       GtkWidget *arrow;
> +       GtkWidget *save;
> +       gboolean  show_bar;
> +       GHashTable *files;

Absolutely NONE of this belongs anywhere near this object.  It should be
blatantly obvious from the rest of the code that no widgets belong here.
Blatantly.  This was covered in detail in the talk I gave in bangalore
too, so you cannot claim ignorance from poorly documented code.

>  };
>  
>  struct _EMFormatClass {
> 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  13 Jul 2005 09:18:44 -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,64 @@ 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_selected_part_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 *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");

None of this code handles duplicate filenames.  None of this code
'safen's up the filenames.  e.g. I could write an attachment which
overwrites any aribtrary file on the filesystme without the user even
being prompted for the name.  Massive security problem.

> +                                }
> +                        }
> +                file_path = g_strconcat (path, "/", file_name, NULL);

Use g_build_filename.

> +                mail_save_part(part, file_path, NULL, NULL);
> +                g_free (file_path);
> +                }
> +
> +        g_free (path);
> +        }
> +       
> +       g_slist_free (parts);

> +        gtk_widget_destroy((GtkWidget *)filesel);
> +}
> +
> +void
> +em_utils_save_selected_parts (GtkWidget *parent, const char *prompt,
> GSList * parts)
> +{
> +        const char *name;

Where is name used?

> +        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_selected_part_response), parts);
> +        gtk_widget_show (filesel);
> +}
> +
> +
>  /**
>   * em_utils_save_part_to_file:
>   * @parent: parent window
> @@ -542,7 +594,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  13 Jul 2005 09:18:44 -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_selected_parts (struct _GtkWidget *parent, const
> char *prompt, GSList * parts);


Naff name, try em_utils_save_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
> 




[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]