Jeff,
Would you please review the patch again?
Thanks!
Harry
Harry Lu wrote:
Anna, Jeff
and others,
Thanks for the review. I modified my patch and attached it here.
For the first problem, since we are using GtkFileSelection widget of
GTK to let user select directory, it is very hard for us to change its
behavior. You have to double click to make the selection work. This is
also the normal behavior for the some File selection dialogs. At least
I am used to it. Anyway, I can hide the selection text entry so that
user can only see the selection label. So the selection label will be
the selection user should refer to. You can apply the patch to see the
result. If you don't like it, I can show the entry again.
For the second, yes, my fault. English is not my native language, so
sometimes I make mistakes:( I had changed the menu item and the dialog
title according to your suggestions.
For the third, I agree with you :)
Jeff, would you please review the patch again?
Thanks!
Harry
anna ximian com wrote:
Hi Harry, Ettore and others,
Thank you for your work on this patch! I have read many, many bug
reports
from people who want to use this feature, and I am happy that we will
be
able to offer it to them soon. Nice work.
For the most part, I think that this patch seems good from a UI
perspective. There are a few little things that bother me. They are:
1. On my machine, it is difficult to tell when a directory has been
selected. Look at the following screenshot:
http://primates.ximian.com/~anna/save-all.png . Have I succeeded in
selecting the "Documents" directory? I cannot tell. If I press the "OK"
button, what will happen? Will my attachments be saved in "Desktop", or
in
"Documents" ?
I'd really like it if we could make the selected directory -- in this
case, "Documents", appear in the text entry beneath the "Selection"
label.
Yes, I know it is a directroy, not a file, and that file names usually
go
in the "Selection" entry. To be clear, in this case, that would mean
that
"Documents" would be shown in the text entry. What do you think? I
think
if it is possible for the name of the selected directory to be shown
there, then, that would be helpful in eliminating user confusion about
precisely where her files will be saved.
2. Cosmetic Stuff: The plural form of the singular noun "attachment" is
"attachments"; you tended to use "attachment", which is incorrect. The
labels you used in your patch should be updated as follows: the name of
the context menu item should be "Save all attachments..." (note that
"all"
is not capitalized).
The title of the file selector should be shortened a bit, to be more
consistent with the window title naming conventions used in the rest of
Evolution; "Select Directory for Attachments" would be more
appropriate.
What do you think of that? Just to let you know what the HIG says on
the
matter, see
http://developer.gnome.org/projects/gup/hig/1.0/windows.html#window-props-titles
. The gist of the guidelines is that window titles should distinguish
the
window from other windows, but should not provide unnecessary
information.
3. As for making other places in the UI for this option, I agree with
you;
figuring out the places where we should make this available is another
bug. Specifically, it is this bug:
http://bugzilla.ximian.com/show_bug.cgi?id=47048 . Let's discuss the
possibilities for how to present these options there.
Thank you,
Anna
_______________________________________________
Evolution-patches mailing list
Evolution-patches lists ximian com
http://lists.ximian.com/mailman/listinfo/evolution-patches
Index: evolution/mail/ChangeLog
===================================================================
RCS file: /cvs/gnome/evolution/mail/ChangeLog,v
retrieving revision 1.2780
diff -u -r1.2780 ChangeLog
--- evolution/mail/ChangeLog 28 Jul 2003 16:09:14 -0000 1.2780
+++ evolution/mail/ChangeLog 30 Jul 2003 05:42:50 -0000
@@ -1,3 +1,18 @@
+2003-07-28 Harry Lu <harry lu sun com>
+
+ *Fix for bug #6951
+
+ * mail-display.c (launch_cb): Bypass the new added menu item.
+ (save_all_parts_cb): New function. Do the real save-all work.
+ (save_all_parts): New function. Get the directory to save to.
+ (save_all_cb): New function. The call-back function for the new
+ added menu item.
+ (pixmap_press): Add the new menu item "Save All Attachment...".
+ (ptr_array_free_notify): A simple wrapper function to free the
+ pointer array.
+ (do_attachment_header): Save attachment pointer in an array for
+ "Save All Attachment" use.
+
2003-07-28 Jeffrey Stedfast <fejj ximian com>
* mail-crypto.c: Removed smime functions as they were stale.
Index: evolution/mail/mail-display.c
===================================================================
RCS file: /cvs/gnome/evolution/mail/mail-display.c,v
retrieving revision 1.287
diff -u -r1.287 mail-display.c
--- evolution/mail/mail-display.c 23 Jul 2003 15:01:26 -0000 1.287
+++ evolution/mail/mail-display.c 30 Jul 2003 05:42:56 -0000
@@ -375,9 +375,11 @@
/* Yum. Too bad EPopupMenu doesn't allow per-item closures. */
children = gtk_container_get_children (GTK_CONTAINER (widget->parent));
- g_return_if_fail (children != NULL && children->next != NULL && children->next->next != NULL);
-
- for (c = children->next->next, apps = handler->applications; c && apps; c = c->next, apps = apps->next) {
+ /* We need to bypass the first 2 menu items */
+ g_return_if_fail (children != NULL && children->next != NULL
+ && children->next->next != NULL && children->next->next->next != NULL);
+
+ for (c = children->next->next->next, apps = handler->applications; c && apps; c = c->next, apps = apps->next) {
if (c->data == widget)
break;
}
@@ -435,7 +437,107 @@
mail_display_queue_redisplay (md);
}
-static gboolean
+static void
+save_all_parts_cb (GtkWidget *widget, gpointer user_data)
+{
+ GtkFileSelection *dir_select = (GtkFileSelection *)
+ gtk_widget_get_ancestor (widget, GTK_TYPE_FILE_SELECTION);
+ const char *filename;
+ char *save_filename, *dir;
+ struct stat st;
+ int i;
+ GPtrArray *attachment_array;
+ CamelMimePart *part;
+ GConfClient *gconf;
+
+ gtk_widget_hide (GTK_WIDGET (dir_select));
+
+ /* Get the selected directory name */
+ filename = gtk_file_selection_get_filename (dir_select);
+ if (stat (filename, &st) == -1 || !S_ISDIR (st.st_mode)) {
+ GtkWidget *dialog;
+
+ dialog = gtk_message_dialog_new (NULL, 0, GTK_MESSAGE_ERROR, GTK_BUTTONS_CLOSE,
+ _("%s is not a valid directory name."), filename);
+
+ /* FIXME: this should be async */
+ gtk_dialog_run ((GtkDialog *) dialog);
+ gtk_widget_destroy (dialog);
+ gtk_widget_destroy (GTK_WIDGET (dir_select));
+ return;
+ } else {
+ dir = g_strdup (filename);
+ }
+
+ /* Now save the attachment one by one */
+ attachment_array = (GPtrArray *)user_data;
+ for (i = 0; i < attachment_array->len; i++) {
+ part = g_ptr_array_index (attachment_array, i);
+ save_filename = make_safe_filename (dir, part);
+ write_data_to_file (part, save_filename, FALSE);
+ g_free (save_filename);
+ }
+
+ /* preserve the pathname */
+ gconf = mail_config_get_gconf_client ();
+ gconf_client_set_string (gconf, "/apps/evolution/mail/save_dir", dir, NULL);
+ g_free (dir);
+
+ gtk_widget_destroy (GTK_WIDGET (dir_select));
+}
+
+static void
+save_all_parts (GPtrArray *attachment_array)
+{
+ GtkFileSelection *dir_select;
+ char *dir, *home, *dir2;
+ GConfClient *gconf;
+
+ g_return_if_fail (attachment_array != NULL);
+
+ home = getenv ("HOME");
+ gconf = mail_config_get_gconf_client ();
+ dir = gconf_client_get_string (gconf, "/apps/evolution/mail/save_dir", NULL);
+ dir = dir ? dir : (home ? g_strdup (home) : g_strdup (""));
+
+ /* Make sure dir2 has a '/' as its tail */
+ dir2 = g_strdup_printf ("%s/", dir);
+ g_free (dir);
+
+ dir_select = GTK_FILE_SELECTION (
+ gtk_file_selection_new (_("Select Directory for Attachments")));
+ gtk_file_selection_set_filename (dir_select, dir2);
+ gtk_widget_set_sensitive (dir_select->file_list, FALSE);
+ gtk_widget_hide (dir_select->selection_entry);
+ g_free (dir2);
+
+ g_signal_connect (dir_select->ok_button, "clicked",
+ G_CALLBACK (save_all_parts_cb), attachment_array);
+ g_signal_connect_swapped (dir_select->cancel_button,
+ "clicked",
+ G_CALLBACK (gtk_widget_destroy),
+ dir_select);
+
+ gtk_widget_show (GTK_WIDGET (dir_select));
+}
+
+static void
+save_all_cb (GtkWidget *widget, gpointer user_data)
+{
+ MailDisplay *md = g_object_get_data (user_data, "MailDisplay");
+ GPtrArray *attachment_array;
+
+ if (md == NULL) {
+ g_warning ("No MailDisplay!");
+ return;
+ }
+
+ attachment_array = g_datalist_get_data (md->data, "attachment_array");
+ save_all_parts (attachment_array);
+}
+
+
+static gboolean
button_press (GtkWidget *widget, GdkEvent *event, CamelMimePart *part)
{
MailDisplay *md;
@@ -475,12 +577,14 @@
EPopupMenu *menu;
GtkMenu *gtk_menu;
EPopupMenu save_item = E_POPUP_ITEM (N_("Save Attachment..."), G_CALLBACK (save_cb), 0);
+ EPopupMenu save_all_item = E_POPUP_ITEM (N_("Save all attachments..."), G_CALLBACK (save_all_cb), 0);
EPopupMenu view_item = E_POPUP_ITEM (N_("View Inline"), G_CALLBACK (inline_cb), 2);
EPopupMenu open_item = E_POPUP_ITEM (N_("Open in %s..."), G_CALLBACK (launch_cb), 1);
MailDisplay *md;
CamelMimePart *part;
MailMimeHandler *handler;
int mask = 0, i, nitems;
+ int current_item = 0;
if (event->type == GDK_BUTTON_PRESS) {
#ifdef USE_OLD_DISPLAY_STYLE
@@ -508,17 +612,23 @@
handler = mail_lookup_handler (g_object_get_data ((GObject *) widget, "mime_type"));
if (handler && handler->applications)
- nitems = g_list_length (handler->applications) + 2;
+ nitems = g_list_length (handler->applications) + 3;
else
- nitems = 3;
+ nitems = 4;
menu = g_new0 (EPopupMenu, nitems + 1);
/* Save item */
- memcpy (&menu[0], &save_item, sizeof (menu[0]));
- menu[0].name = _(menu[0].name);
-
+ memcpy (&menu[current_item], &save_item, sizeof (menu[current_item]));
+ menu[current_item].name = g_strdup (_(menu[current_item].name));
+ current_item++;
+
+ /* Save All item */
+ memcpy (&menu[current_item], &save_all_item, sizeof (menu[current_item]));
+ menu[current_item].name = g_strdup (_(menu[current_item].name));
+ current_item++;
+
/* Inline view item */
- memcpy (&menu[1], &view_item, sizeof (menu[1]));
+ memcpy (&menu[current_item], &view_item, sizeof (menu[current_item]));
if (handler && handler->builtin) {
md = g_object_get_data ((GObject *) widget, "MailDisplay");
@@ -536,15 +646,16 @@
name = prop->v._u.value_string;
else
name = "bonobo";
- menu[1].name = g_strdup_printf (_("View Inline (via %s)"), name);
+ menu[current_item].name = g_strdup_printf (_("View Inline (via %s)"), name);
} else
- menu[1].name = g_strdup (_(menu[1].name));
+ menu[current_item].name = g_strdup (_(menu[current_item].name));
} else
- menu[1].name = g_strdup (_("Hide"));
+ menu[current_item].name = g_strdup (_("Hide"));
} else {
- menu[1].name = g_strdup (_(menu[1].name));
+ menu[current_item].name = g_strdup (_(menu[current_item].name));
mask |= 2;
}
+ current_item++;
/* External views */
if (handler && handler->applications) {
@@ -553,14 +664,15 @@
int i;
apps = handler->applications;
- for (i = 2; i < nitems; i++, apps = apps->next) {
+ for (i = current_item; i < nitems; i++, apps = apps->next) {
app = apps->data;
memcpy (&menu[i], &open_item, sizeof (menu[i]));
menu[i].name = g_strdup_printf (_(menu[i].name), app->name);
+ current_item++;
}
} else {
- memcpy (&menu[2], &open_item, sizeof (menu[2]));
- menu[2].name = g_strdup_printf (_(menu[2].name), _("External Viewer"));
+ memcpy (&menu[current_item], &open_item, sizeof (menu[current_item]));
+ menu[current_item].name = g_strdup_printf (_(menu[current_item].name), _("External Viewer"));
mask |= 1;
}
@@ -983,6 +1095,12 @@
}
}
+/* This is a wrapper function */
+void ptr_array_free_notify (gpointer array)
+{
+ g_ptr_array_free ((GPtrArray *) array, TRUE);
+}
+
static gboolean
do_attachment_header (GtkHTML *html, GtkHTMLEmbedded *eb,
CamelMimePart *part, MailDisplay *md)
@@ -990,6 +1108,7 @@
GtkWidget *button, *mainbox, *hbox, *arrow, *popup;
MailMimeHandler *handler;
struct _PixbufLoader *pbl;
+ GPtrArray *attachment_array;
pbl = g_new0 (struct _PixbufLoader, 1);
if (strncasecmp (eb->type, "image/", 6) == 0) {
@@ -1059,6 +1178,19 @@
g_object_set_data ((GObject *) popup, "MailDisplay", md);
g_object_set_data ((GObject *) popup, "CamelMimePart", part);
g_object_set_data_full ((GObject *) popup, "mime_type", g_strdup (eb->type), (GDestroyNotify) g_free);
+
+ /* Save attachment pointer in an array for "save all attachment" use */
+ attachment_array = g_datalist_get_data (md->data, "attachment_array");
+ if (!attachment_array) {
+ attachment_array = g_ptr_array_new ();
+ g_datalist_set_data_full (md->data, "attachment_array",
+ attachment_array, (GDestroyNotify) ptr_array_free_notify);
+ }
+ /* Since the attachment pointer might have been added to the array before,
+ remove it first anyway to avoide duplication */
+ g_ptr_array_remove (attachment_array, part);
+ g_ptr_array_add (attachment_array, part);
+
g_signal_connect (popup, "button_press_event", G_CALLBACK (pixmap_press), md->scroll);
g_signal_connect (popup, "key_press_event", G_CALLBACK (pixmap_press), md->scroll);
|
