[geary] Re-implement alt text filename fallback when saving inline images.

[Michael Gratton <mjog src gnome org>]

commit 9f5def0d0fb634d2a01fcbe9f55a590aec843654
Author: Michael James Gratton <mike vee net>
Date:   Thu Feb 16 08:58:58 2017 +1100

    Re-implement alt text filename fallback when saving inline images.
    
    This restores some old behaviour after teh WK2 port.
    
    * src/client/conversation-viewer/conversation-message.vala
      (ConversationMessage): Add alt_text param to ::save_image
      signal. Replace ::set_action_param_string with
      ::set_action_param_string so we can pass a (url, alt_text) tuple for
      the save_image tuple and update call sites. Set this tuple for the
      action when enabling it, and pass the two values to the ::save_image
      signal handler when activated.
    
    * src/engine/api/geary-attachment.vala (Attachment::get_safe_file_name):
      Add alt_file_name param method, use that when constructing a file
      name. Add doc comment, test.
    
    * src/client/application/geary-controller.vala: Updated to pass alt text
      from save_imge signal emission all thr way through to
      get_safe_file_name.

 src/client/application/geary-controller.vala       |   14 ++++---
 .../conversation-viewer/conversation-message.vala  |   36 +++++++++++++------
 src/engine/api/geary-attachment.vala               |    7 +++-
 test/engine/api/geary-attachment-test.vala         |   37 ++++++++++++++++----
 4 files changed, 69 insertions(+), 25 deletions(-)
---
diff --git a/src/client/application/geary-controller.vala b/src/client/application/geary-controller.vala
index c8d637b..240c51f 100644
--- a/src/client/application/geary-controller.vala
+++ b/src/client/application/geary-controller.vala
@@ -2051,8 +2051,9 @@ public class GearyController : Geary.BaseObject {
         }
     }
 
-    private async void save_attachment_to_file(Geary.Attachment attachment) {
-        string file_name = yield attachment.get_safe_file_name();
+    private async void save_attachment_to_file(Geary.Attachment attachment,
+                                               string? alt_text) {
+        string file_name = yield attachment.get_safe_file_name(alt_text);
         try {
             yield this.prompt_save_buffer(
                 file_name, new Geary.Memory.FileBuffer(attachment.file, true)
@@ -2770,8 +2771,8 @@ public class GearyController : Geary.BaseObject {
                             open_uri(link);
                         }
                     });
-                mview.save_image.connect((filename, buf) => {
-                    on_save_image_extended(view, filename, buf);
+                mview.save_image.connect((url, alt_text, buf) => {
+                        on_save_image_extended(view, url, alt_text, buf);
                 });
                 mview.search_activated.connect((op, value) => {
                         string search = op + ":" + value;
@@ -2991,7 +2992,7 @@ public class GearyController : Geary.BaseObject {
 
     private void on_save_attachments(Gee.Collection<Geary.Attachment> attachments) {
         if (attachments.size == 1) {
-            this.save_attachment_to_file.begin(attachments.to_array()[0]);
+            this.save_attachment_to_file.begin(attachments.to_array()[0], null);
         } else {
             this.save_attachments_to_file.begin(attachments);
         }
@@ -2999,6 +3000,7 @@ public class GearyController : Geary.BaseObject {
 
     private void on_save_image_extended(ConversationEmail view,
                                         string url,
+                                        string? alt_text,
                                         Geary.Memory.Buffer resource_buf) {
         // This is going to be either an inline image, or a remote
         // image, so either treat it as an attachment ot assume we'll
@@ -3014,7 +3016,7 @@ public class GearyController : Geary.BaseObject {
                 debug("Could not get attachment \"%s\": %s", cid, err.message);
             }
             if (attachment != null) {
-                this.save_attachment_to_file.begin(attachment);
+                this.save_attachment_to_file.begin(attachment, alt_text);
                 handled = true;
             }
         }
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index e495258..8615e03 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -252,7 +252,7 @@ public class ConversationMessage : Gtk.Grid {
     public signal void remember_remote_images();
 
     /** Fired when the user saves an inline displayed image. */
-    public signal void save_image(string? uri, Geary.Memory.Buffer buffer);
+    public signal void save_image(string? uri, string? alt_text, Geary.Memory.Buffer buffer);
 
     /** Fired when the user activates a specific search shortcut. */
     public signal void search_activated(string operator, string value);
@@ -293,7 +293,7 @@ public class ConversationMessage : Gtk.Grid {
             .activate.connect((param) => {
                 link_activated(param.get_string());
             });
-        add_action(ACTION_SAVE_IMAGE, true, VariantType.STRING)
+        add_action(ACTION_SAVE_IMAGE, true, new VariantType("(sms)"))
             .activate.connect(on_save_image);
         add_action(ACTION_SEARCH_FROM, true, VariantType.STRING)
             .activate.connect((param) => {
@@ -523,18 +523,14 @@ public class ConversationMessage : Gtk.Grid {
         }
     }
 
-    private Menu set_action_param_string(MenuModel existing, string value) {
+    private Menu set_action_param_value(MenuModel existing, Variant value) {
         Menu menu = new Menu();
         for (int i = 0; i < existing.get_n_items(); i++) {
             MenuItem item = new MenuItem.from_model(existing, i);
             Variant action = item.get_attribute_value(
                 Menu.ATTRIBUTE_ACTION, VariantType.STRING
             );
-            item.set_action_and_target(
-                action.get_string(),
-                VariantType.STRING.dup_string(),
-                value
-            );
+            item.set_action_and_target_value(action.get_string(), value);
             menu.append_item(item);
         }
         return menu;
@@ -820,7 +816,10 @@ public class ConversationMessage : Gtk.Grid {
                 ? context_menu_email
                 : context_menu_link;
             model.append_section(
-                null, set_action_param_string(link_menu, link_url)
+                null,
+                set_action_param_value(
+                    link_menu, new Variant.string(link_url)
+                )
             );
         }
 
@@ -828,7 +827,14 @@ public class ConversationMessage : Gtk.Grid {
             string uri = hit_test.get_image_uri();
             set_action_enabled(ACTION_SAVE_IMAGE, uri in this.resources);
             model.append_section(
-                null, set_action_param_string(context_menu_image, uri)
+                null,
+                set_action_param_value(
+                    context_menu_image,
+                    new Variant.tuple({
+                            new Variant.string(uri),
+                            new Variant("ms", hit_test.get_link_label()),
+                        })
+                )
             );
         }
 
@@ -932,11 +938,19 @@ public class ConversationMessage : Gtk.Grid {
     }
 
     private void on_save_image(Variant? param) {
-        WebKit.WebResource response = this.resources.get(param.get_string());
+        string cid_url = param.get_child_value(0).get_string();
+
+        string? alt_text = null;
+        Variant? alt_maybe = param.get_child_value(1).get_maybe();
+        if (alt_maybe != null) {
+            alt_text = alt_maybe.get_string();
+        }
+        WebKit.WebResource response = this.resources.get(cid_url);
         response.get_data.begin(null, (obj, res) => {
                 try {
                     uint8[] data = response.get_data.end(res);
                     save_image(response.get_uri(),
+                               alt_text,
                                new Geary.Memory.ByteBuffer(data, data.length));
                 } catch (Error err) {
                     debug(
diff --git a/src/engine/api/geary-attachment.vala b/src/engine/api/geary-attachment.vala
index 226bb1a..d46b66e 100644
--- a/src/engine/api/geary-attachment.vala
+++ b/src/engine/api/geary-attachment.vala
@@ -100,11 +100,16 @@ public abstract class Geary.Attachment : BaseObject {
      * construct one from the attachment's id and by guessing the file
      * name extension, and also guessing the MIME content type if
      * needed.
+     *
+     * If a file name is constructed and a non-empty value for the
+     * `alt_file_name` parameter is specified, then that will be used
+     * in preference over the content id and attachment id.
      */
-    public async string get_safe_file_name() {
+    public async string get_safe_file_name(string? alt_file_name = null) {
         string? file_name = this.content_filename;
         if (Geary.String.is_empty(file_name)) {
             string[] others = {
+                alt_file_name,
                 this.content_id,
                 this.id ?? "attachment",
             };
diff --git a/test/engine/api/geary-attachment-test.vala b/test/engine/api/geary-attachment-test.vala
index 0efad06..00f4d02 100644
--- a/test/engine/api/geary-attachment-test.vala
+++ b/test/engine/api/geary-attachment-test.vala
@@ -43,6 +43,8 @@ class Geary.AttachmentTest : Gee.TestCase {
                  get_safe_file_name_with_bad_content_name);
         add_test("get_safe_file_name_with_bad_file_name",
                  get_safe_file_name_with_bad_file_name);
+        add_test("get_safe_file_name_with_alt_file_name",
+                 get_safe_file_name_with_alt_file_name);
         add_test("get_safe_file_name_with_no_content_name",
                  get_safe_file_name_with_no_content_name);
         add_test("get_safe_file_name_with_no_content_name_or_id",
@@ -80,7 +82,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -101,7 +103,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -122,7 +124,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -142,7 +144,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -162,7 +164,28 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
+                async_complete(ret);
+            });
+
+        assert(test.get_safe_file_name.end(async_result()) == RESULT_FILENAME);
+    }
+
+    public void get_safe_file_name_with_alt_file_name() {
+        const string ALT_TEXT = "some text";
+        const string RESULT_FILENAME = "some text.png";
+        Attachment test = new TestAttachment(
+            ATTACHMENT_ID,
+            this.content_type,
+            null,
+            CONTENT_DESC,
+            content_disposition,
+            null,
+            this.file,
+            742
+        );
+
+        test.get_safe_file_name.begin(ALT_TEXT, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -182,7 +205,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });
 
@@ -205,7 +228,7 @@ class Geary.AttachmentTest : Gee.TestCase {
             742
         );
 
-        test.get_safe_file_name.begin((obj, ret) => {
+        test.get_safe_file_name.begin(null, (obj, ret) => {
                 async_complete(ret);
             });