[geary] Clean up DB handling of attachments without specified file names.

[Michael Gratton <mjog src gnome org>]

commit 992528862e441ab2c66fb872490222b131e47d9c
Author: Michael James Gratton <mike vee net>
Date:   Sat Feb 11 10:58:44 2017 +1100

    Clean up DB handling of attachments without specified file names.
    
    This could probably use a DB migration to set all attachments will "none"
    as their filename to NULL, but that's a lot of trouble for little gain.
    
    * src/client/conversation-viewer/conversation-email.vala (if): Remove
      workaround for "none" attachment file names.
    
    * src/client/conversation-viewer/conversation-message.vala
      (ConversationMessage::inline_image_replacer): Handle translation for
      null attachment file names here, rather than in the engine.
    
    * src/engine/api/geary-email.vala
      (Email::get_searchable_attachment_list): Don't bother appending file
      names for attachments when they are null.
    
    * src/engine/imap-db/imap-db-attachment.vala (Attachment): Use a const
      for null file name replacement string so it can be used elsewhere.
    
    * src/engine/imap-db/imap-db-folder.vala (Folder::do_list_attachments):
      Clean up old "none" file names when loading from DB.
      (Folder::do_save_attachments_db): Allow for file name being null.
    
    * src/engine/rfc822/rfc822-message.vala (Message::InlinePartReplacer):
      Allow file name to be null, update call sites.
    
    * src/engine/rfc822/rfc822-utils.vala (get_clean_attachment_filename):
      Don't attempt to translate null filenames, just return it null. Update
      call sites.

 .../conversation-viewer/conversation-email.vala    |   14 ++------------
 .../conversation-viewer/conversation-message.vala  |   12 ++++++++++--
 src/engine/api/geary-email.vala                    |    6 ++++--
 src/engine/imap-db/imap-db-attachment.vala         |   11 +++++------
 src/engine/imap-db/imap-db-folder.vala             |   14 +++++++++++---
 src/engine/rfc822/rfc822-message.vala              |    2 +-
 src/engine/rfc822/rfc822-utils.vala                |   20 +++++++++-----------
 7 files changed, 42 insertions(+), 37 deletions(-)
---
diff --git a/src/client/conversation-viewer/conversation-email.vala 
b/src/client/conversation-viewer/conversation-email.vala
index bd70da1..725c182 100644
--- a/src/client/conversation-viewer/conversation-email.vala
+++ b/src/client/conversation-viewer/conversation-email.vala
@@ -113,10 +113,7 @@ public class ConversationEmail : Gtk.Box {
                 mime_content_type
             );
 
-            string file_name = null;
-            if (attachment.has_content_filename) {
-                file_name = attachment.file.get_basename();
-            }
+            string? file_name = attachment.content_filename;
             string file_desc = ContentType.get_description(gio_content_type);
             if (ContentType.is_unknown(gio_content_type)) {
                 // Translators: This is the file type displayed for
@@ -125,14 +122,7 @@ public class ConversationEmail : Gtk.Box {
             }
             string file_size = Files.get_filesize_as_string(attachment.filesize);
 
-            // XXX Geary.ImapDb.Attachment will use "none" when
-            // saving attachments with no filename to disk, this
-            // seems to be getting saved to be the filename and
-            // passed back, breaking the has_supplied_filename
-            // test - so check for it here.
-            if (file_name == null ||
-                file_name == "" ||
-                file_name == "none") {
+            if (Geary.String.is_empty(file_name)) {
                 // XXX Check for unknown types here and try to guess
                 // using attachment data.
                 file_name = file_desc;
diff --git a/src/client/conversation-viewer/conversation-message.vala 
b/src/client/conversation-viewer/conversation-message.vala
index f6bb7cd..2b062a0 100644
--- a/src/client/conversation-viewer/conversation-message.vala
+++ b/src/client/conversation-viewer/conversation-message.vala
@@ -668,7 +668,7 @@ public class ConversationMessage : Gtk.Grid {
     // If this returns null, the MIME part is dropped from the final returned document; otherwise,
     // this returns HTML that is placed into the document in the position where the MIME part was
     // found
-    private string? inline_image_replacer(string filename, Geary.Mime.ContentType? content_type,
+    private string? inline_image_replacer(string? filename, Geary.Mime.ContentType? content_type,
         Geary.Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer) {
         if (content_type == null) {
             debug("Not displaying inline: no Content-Type");
@@ -688,8 +688,16 @@ public class ConversationMessage : Gtk.Grid {
 
         this.web_view.add_internal_resource(id, buffer);
 
+        // Translators: This string is used as the HTML IMG ALT
+        // attribute value when displaying an inline image in an email
+        // that did not specify a file name. E.g. <IMG ALT="Image" ...
+        string UNKNOWN_FILENAME_ALT_TEXT = _("Image");
+        string clean_filename = Geary.HTML.escape_markup(
+            filename ?? UNKNOWN_FILENAME_ALT_TEXT
+        );
+
         return "<img alt=\"%s\" class=\"%s\" src=\"%s%s\" />".printf(
-            Geary.HTML.escape_markup(filename),
+            clean_filename,
             REPLACED_IMAGE_CLASS,
             ClientWebView.CID_URL_PREFIX,
             Geary.HTML.escape_markup(id)
diff --git a/src/engine/api/geary-email.vala b/src/engine/api/geary-email.vala
index 16d01da..6209333 100644
--- a/src/engine/api/geary-email.vala
+++ b/src/engine/api/geary-email.vala
@@ -272,8 +272,10 @@ public class Geary.Email : BaseObject {
     public string get_searchable_attachment_list() {
         StringBuilder search = new StringBuilder();
         foreach (Geary.Attachment attachment in attachments) {
-            search.append(attachment.file.get_basename());
-            search.append("\n");
+            if (attachment.has_content_filename) {
+                search.append(attachment.content_filename);
+                search.append("\n");
+            }
         }
         return search.str;
     }
diff --git a/src/engine/imap-db/imap-db-attachment.vala b/src/engine/imap-db/imap-db-attachment.vala
index 7ed57f3..74fde3f 100644
--- a/src/engine/imap-db/imap-db-attachment.vala
+++ b/src/engine/imap-db/imap-db-attachment.vala
@@ -6,7 +6,8 @@
 
 private class Geary.ImapDB.Attachment : Geary.Attachment {
     public const Email.Field REQUIRED_FIELDS = Email.REQUIRED_FOR_MESSAGE;
-    
+
+    internal const string NULL_FILE_NAME = "none";
     private const string ATTACHMENTS_DIR = "attachments";
 
     public Attachment(int64 message_id,
@@ -31,17 +32,15 @@ private class Geary.ImapDB.Attachment : Geary.Attachment {
     private static string generate_id(int64 attachment_id) {
         return "imap-db:%s".printf(attachment_id.to_string());
     }
-    
+
     public static File generate_file(File data_dir, int64 message_id, int64 attachment_id,
         string? filename) {
-        // "none" should not be translated, or the user will be unable to retrieve their
-        // attachments with no filenames after changing their language.
         return get_attachments_dir(data_dir)
             .get_child(message_id.to_string())
             .get_child(attachment_id.to_string())
-            .get_child(filename ?? "none");
+            .get_child(filename ?? NULL_FILE_NAME);
     }
-    
+
     public static File get_attachments_dir(File data_dir) {
         return data_dir.get_child(ATTACHMENTS_DIR);
     }
diff --git a/src/engine/imap-db/imap-db-folder.vala b/src/engine/imap-db/imap-db-folder.vala
index f7d0d7a..af31564 100644
--- a/src/engine/imap-db/imap-db-folder.vala
+++ b/src/engine/imap-db/imap-db-folder.vala
@@ -2009,6 +2009,14 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
 
         Gee.List<Geary.Attachment> list = new Gee.ArrayList<Geary.Attachment>();
         do {
+            string? content_filename = results.string_at(1);
+            if (content_filename == ImapDB.Attachment.NULL_FILE_NAME) {
+                // Prior to 0.12, Geary would store the untranslated
+                // string "none" as the filename when none was
+                // specified by the MIME content disposition. Check
+                // for that and clean it up.
+                content_filename = null;
+            }
             Mime.ContentDisposition disposition = new Mime.ContentDisposition.simple(
                 Mime.DispositionType.from_int(results.int_at(4)));
             list.add(
@@ -2019,7 +2027,7 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
                     results.string_at(5),
                     results.string_at(6),
                     disposition,
-                    results.string_at(1),
+                    content_filename,
                     cx.database.db_file.get_parent(),
                     results.int64_at(3)
                 )
@@ -2048,8 +2056,8 @@ private class Geary.ImapDB.Folder : BaseObject, Geary.ReferenceSemantics {
             string? disposition = attachment.get_disposition();
             string? content_id = attachment.get_content_id();
             string? description = attachment.get_content_description();
-            string filename = RFC822.Utils.get_clean_attachment_filename(attachment);
-            
+            string? filename = RFC822.Utils.get_clean_attachment_filename(attachment);
+
             // Convert the attachment content into a usable ByteArray.
             GMime.DataWrapper? attachment_data = attachment.get_content_object();
             ByteArray byte_array = new ByteArray();
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index 584b199..9dd2565 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -13,7 +13,7 @@ public class Geary.RFC822.Message : BaseObject {
      * referred to by rich text in alternative or related documents must be located by the caller
      * and appropriately presented.
      */
-    public delegate string? InlinePartReplacer(string filename, Mime.ContentType? content_type,
+    public delegate string? InlinePartReplacer(string? filename, Mime.ContentType? content_type,
         Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer);
 
     private const string HEADER_SENDER = "Sender";
diff --git a/src/engine/rfc822/rfc822-utils.vala b/src/engine/rfc822/rfc822-utils.vala
index 8a50b28..49b3261 100644
--- a/src/engine/rfc822/rfc822-utils.vala
+++ b/src/engine/rfc822/rfc822-utils.vala
@@ -425,19 +425,17 @@ public GMime.ContentEncoding get_best_encoding(GMime.Stream in_stream) {
     return filter.encoding(GMime.EncodingConstraint.7BIT);
 }
 
-public string get_clean_attachment_filename(GMime.Part part) {
+public string? get_clean_attachment_filename(GMime.Part part) {
     string? filename = part.get_filename();
-    if (String.is_empty(filename)) {
-        /// Placeholder filename for attachments with no filename.
-        filename = _("none");
-    }
-    
-    try {
-        filename = invalid_filename_character_re.replace_literal(filename, filename.length, 0, "_");
-    } catch (RegexError e) {
-        debug("Error sanitizing attachment filename: %s", e.message);
+    if (filename != null) {
+        try {
+            filename = invalid_filename_character_re.replace_literal(
+                filename, filename.length, 0, "_"
+            );
+        } catch (RegexError e) {
+            debug("Error sanitizing attachment filename: %s", e.message);
+        }
     }
-    
     return filename;
 }