Merge lp:~purejava/pantheon-mail/improve-mime-handling into lp:~elementary-apps/pantheon-mail/trunk

[Ralph Plawetzki]

Improve mimetype handling in the conversation viewer

Patch to address LP#1545605 and LP#1543629

Mail processes all nested content types of an email recursively.
First, Mail assumes Geary.RFC822.TextFormat.HTML and walks through the email. If there is a least one content type of html or an inlined image, these parts are used.
If none are found, Mail assumes Geary.RFC822.TextFormat.PLAIN and walks through the email a second time. Now, if there is at least one content type of plain text or an inlined image, these parts are used.

LP#1543629
Mail already has a functionality for saving inline images, which has been modified with commit 89ca016 [1] to only allow this for Mime.MultipartSubtype.MIXED.
This patch reverts this restriction. As images that are allowed to be saved have a tag "replaced-id", this tag gets added in ConversationViewer#insert_html_markup(..) for images that have a content-id, but no data prefix yet.

[1] https://github.com/GNOME/geary/commit/89ca0167830dfbc63e4b90e6085764e690fc4fde

LP#1545605
The problem here is, that assuming Geary.RFC822.TextFormat.HTML during the first walk through ignores the plain text parts but finds the inlined images which are returned.
The reason for this is line 539 in rfc822-message.vala. The variable body has a valid content (with an replaced image), so the return statement returns true for the inlined image.

Setting this to always false means that inlined images are never treated as valid content here, but line 487 (found_text_subtype |= construct_body_from_mime_parts...) takes care of this. As, depending on the Geary.RFC822.TextFormat, other valid parts have been found and the body gets contructed correctly.

As line 539 is always false now, it does not reflect returns of null in inline_image_replacer(..) any more which indicates errors, but this is ok (see above).

=== modified file 'src/client/conversation-viewer/conversation-viewer.vala'
--- src/client/conversation-viewer/conversation-viewer.vala	2016-01-18 10:11:06 +0000
+++ src/client/conversation-viewer/conversation-viewer.vala	2016-02-16 16:41:42 +0000
@@ -182,6 +182,7 @@
     private int next_replaced_buffer_number = 0;
     private Gee.HashMap<string, ReplacedImage> replaced_images = new Gee.HashMap<string, ReplacedImage>();
     private Gee.HashSet<string> replaced_content_ids = new Gee.HashSet<string>();
+    private Gee.HashMap<string, string> replaced_images_index = new Gee.HashMap<string, string>();
     private Gee.HashSet<string> blacklist_ids = new Gee.HashSet<string>();
     
     public ConversationViewer() {
@@ -859,7 +860,7 @@
         //
         // * Geary.RFC822.Message.get_body() recursively walks the message's MIME structure looking
         //   for text MIME parts and assembles them sequentially.  If non-text MIME parts are
-        //   discovered within a multipart/mixed container, it calls inline_image_replacer(), which
+        //   discovered, it calls inline_image_replacer(), which
         //   converts them to an IMG tag with a data: URI if they are a supported image type.
         //   Otherwise, the MIME part is dropped.
         //
@@ -941,7 +942,7 @@
     }
     
     // This delegate is called from within Geary.RFC822.Message.get_body while assembling the plain
-    // or HTML document when a non-text MIME part is encountered within a multipart/mixed container.
+    // or HTML document when a non-text MIME part is encountered.
     // 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
@@ -1012,6 +1013,10 @@
             buffer);
         replaced_images.set(replaced_image.id, replaced_image);
         
+        if (!Geary.String.is_empty(content_id)) {
+            replaced_images_index.set(content_id, replaced_image.id);
+        }
+        
         return "<img alt=\"%s\" class=\"%s %s\" src=\"%s\" replaced-id=\"%s\" %s />".printf(
             Geary.HTML.escape_markup(filename),
             DATA_IMAGE_CLASS, REPLACED_IMAGE_CLASS,
@@ -2005,7 +2010,8 @@
                 
                 // if image has a Content-ID and it's already been replaced by the image replacer,
                 // drop this tag, otherwise fix up this one with the Base-64 data URI of the image
-                if (!replaced_content_ids.contains(content_id)) {
+                // and the replaced id
+                if (!src.has_prefix("data:")) {
                     string? filename = message.get_content_filename_by_mime_id(content_id);
                     Geary.Memory.Buffer image_content = message.get_content_by_mime_id(content_id);
                     Geary.Memory.UnownedBytesBuffer? unowned_buffer =
@@ -2021,11 +2027,12 @@
                     string mimetype = ContentType.get_mime_type(guess);
                     
                     // Replace the SRC to a data URI, the class to a known label for the popup menu,
-                    // and the ALT to its filename, if supplied
+                    // the ALT to its filename, if supplied and add the replaced-id
                     img.set_attribute("src", assemble_data_uri(mimetype, image_content));
                     img.set_attribute("class", DATA_IMAGE_CLASS);
                     if (!Geary.String.is_empty(filename))
                         img.set_attribute("alt", filename);
+                    img.set_attribute("replaced-id", replaced_images_index.get(content_id));
                     
                     // stash here so inlined image isn't listed as attachment (esp. if it has no
                     // Content-Disposition)

=== modified file 'src/engine/rfc822/rfc822-message.vala'
--- src/engine/rfc822/rfc822-message.vala	2015-03-12 01:11:48 +0000
+++ src/engine/rfc822/rfc822-message.vala	2016-02-16 16:41:42 +0000
@@ -451,8 +451,8 @@
     /**
      * This method is the main utility method used by the other body-generating constructors.
      *
-     * Only text/* MIME parts of the specified subtype are added to body.  If a non-text part is
-     * within a multipart/mixed container, the {@link InlinePartReplacer} is invoked.
+     * Only text/* MIME parts of the specified subtype are added to body.  For non-text parts
+     * the {@link InlinePartReplacer} is invoked.
      *
      * If to_html is true, the text is run through a filter to HTML-ize it.  (Obviously, this
      * should be false if text/html is being searched for.).
@@ -525,18 +525,14 @@
         if (disposition == null || disposition.disposition_type == Mime.DispositionType.UNSPECIFIED)
             return false;
         
-        // Use inline part replacer *only* if in a mixed multipart where each element is to be
-        // presented to the user as structure dictates; for alternative and related, the inline
-        // part is referred to elsewhere in the document and it's the callers responsibility to
-        // locate them
-        if (replacer == null || container_subtype != Mime.MultipartSubtype.MIXED)
+        if (replacer == null)
             return false;
         
         // Hand off to the replacer for processing
         body = replacer(RFC822.Utils.get_clean_attachment_filename(part),
             this_content_type, disposition, part.get_content_id(), mime_part_to_memory_buffer(part));
         
-        return body != null;
+        return false;
     }
     
     /**
@@ -586,8 +582,8 @@
      * Returns the complete email body as HTML.
      *
      * get_body() recursively walks the MIME structure (depth-first) serializing all text MIME
-     * parts of the specified type into a single UTF-8 string.  Non-text MIME parts inside of
-     * multipart/mixed containers are offered to the {@link InlinePartReplacer}, which can either
+     * parts of the specified type into a single UTF-8 string.  Non-text MIME parts
+     * are offered to the {@link InlinePartReplacer}, which can either
      * return null or return a string that is inserted in lieu of the MIME part into the final
      * document.  All other MIME parts are ignored.
      *