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: Thu, 14 Jul 2005 20:17:42 +0530
Notzed,
Few doubts and clarifications wrt to thoughts.
On Thu, 2005-07-14 at 11:53 +0800, Not Zed wrote:
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?
I didnt knew it. Wont repeat here after.
> #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.
Fine notzed. Ill remove this. Im unaware how to display the message in between the first header and the message body. Adding to format_clone, moves it to top of the message and it may look wierd. Another idea you explained on the irc, which i didnt get clearly, like u said 'add some internal x/ type' means that adding internal mime type and a handler and ill call it from efh_format_message after the header and add the pobject for the bar?
Okie, i understand the threads in EMFormatClass better now. Here after in my code i wont add any gtk calls in the formatter thread.
> + /* 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.
Sure will do that in the next patch.
> + 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.
Thanks i missed out such a silly stuff.
> +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.
Notzed, ill make sure that ill fix it once i complete it. The UI freeze is approaching, and i
wanted this to go without breaking the freeze, since we are already in the announce period.
The api's ill defenitely cleanup and use a ideal one. Hope you got me.
> +static void
> +eab_popup_position(GtkMenu *menu, int *x, int *y, gboolean *push_in,
> gpointer usedata)
WTF is eab? Stick to the proper namespace.
Sorry for that.
> +{
> + 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.
Okie. I'll move it.
> +{
> + 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.
Sorry really missed out the logic here. I have this code to e-attachment-bar.
> + 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.
Okie. Changed to attachments target. But this reqd me to write EM_TARGET_ATTACHMENT_IMAGE
and do some handling in em-popup.c, similiar to the normal image/save as/open with. I have made
duplication, but will check the type of target and get the part wrt it.
> + } 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 = "">
> +
> + 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.
Okie. Ill add it in a new pobject? or do it in the existing pobject where we add a button for attachment?
> + em_format_html_add_pobject((EMFormatHTML *)emf,
> sizeof(EMFormatHTMLPObject), classid, part, efhd_add_bar);
> + camel_stream_printf(stream, "<td><object classid="">
> \"></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.
sorry didnt look close at it. Ill have check for msg rfc822 only here
> + {
> + 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.
But then how to know, whether there was a similiar file? I thought ill add a dummy
name "attachment.dat" or something and proceeed. I dont know how to do without
using file as a key. You are a expert here, would love to know your ideas on it.
> + 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.
Sure.
> + /* 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.
Okie. So i can set it to the part of the attachment rit. That shouldnt be a problem.
But then i have to change the describe part where it says the old file name.
I think it would be better to tell same file name every where. Or again do you
have a better idea
> + 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.
Sure will do all this in a pobject. But do i have to create a new pobject or do it in the existing pobject.
> + }
> + }
>
> 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.
Will remove it.
>
> #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.
Sure will remove it and move all this EMFormatHTMLDisplay.
> }
>
> 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.
Okie.
> #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.
Is it like, i have to e_filename_safe*? to be safer?
> + }
> + }
> + 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?
Sorry missed out.
> + 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 = "" _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()
will change it.
> 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
-Srini.
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
