[geary/geary-0.11] Fix missing text displaying an image with text parts and inline images.



commit 822bdb72990a9cff389f3ca5d1954b486531b8ec
Author: Michael James Gratton <mike vee net>
Date:   Fri Jun 17 20:30:29 2016 -0400

    Fix missing text displaying an image with text parts and inline images.
    
    When Geary.RFC822.Message::construct_body_from_mime_parts is looking to
    build a message body to display, during the first pass that looks for
    HTML content the plain text part is ignored but the inline image gets
    converted to a HTML IMG element and returned as the body. This makes it
    look like an HTML part was found, and hence the second pass looking for
    plain text never takes place.
    
    Fix this by not taking a wild stab at the body type - check to see if it
    has HTML parts and if so just het them, if not then get plain text parts.
    
    Bug 767438.
    
    * src/engine/rfc822/rfc822-message.vala (Message): Add ::has_html_body
      and ::has_plain_body to allow callers to query available body content
      types before attempting to get them. Remove ::get_body since it's
      making a policy decision that callers should be making instead, fix
      call sites.

 src/client/composer/composer-widget.vala           |    7 +-
 .../conversation-viewer/conversation-viewer.vala   |    6 +-
 src/engine/rfc822/rfc822-message.vala              |  137 ++++++++++++--------
 src/engine/rfc822/rfc822-utils.vala                |   30 +++-
 4 files changed, 114 insertions(+), 66 deletions(-)
---
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index db571c1..312c31a 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -480,7 +480,12 @@ public class ComposerWidget : Gtk.EventBox {
                     if (referred.subject != null)
                         subject = referred.subject.value;
                     try {
-                        body_html = referred.get_message().get_body(Geary.RFC822.TextFormat.HTML, null);
+                        Geary.RFC822.Message message = referred.get_message();
+                        if (message.has_html_body()) {
+                            body_html = message.get_html_body(null);
+                        } else {
+                            body_html = message.get_plain_body(true, null);
+                        }
                     } catch (Error error) {
                         debug("Error getting message body: %s", error.message);
                     }
diff --git a/src/client/conversation-viewer/conversation-viewer.vala 
b/src/client/conversation-viewer/conversation-viewer.vala
index 3c1d9f0..bf1dea1 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -872,7 +872,11 @@ public class ConversationViewer : Gtk.Box {
         string body_text = "";
         remote_images = false;
         try {
-            body_text = message.get_body(Geary.RFC822.TextFormat.HTML, inline_image_replacer) ?? "";
+            if (message.has_html_body()) {
+                body_text = message.get_html_body(inline_image_replacer);
+            } else {
+                body_text = message.get_plain_body(true, inline_image_replacer);
+            }
             body_text = insert_html_markup(body_text, message, out remote_images);
         } catch (Error err) {
             debug("Could not get message text. %s", err.message);
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index 58491c0..90f21c3 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -447,7 +447,66 @@ public class Geary.RFC822.Message : BaseObject {
     public Memory.Buffer get_network_buffer(bool dotstuffed) throws RFC822Error {
         return message_to_memory_buffer(true, dotstuffed);
     }
-    
+
+    /**
+     * Determines if the message has one or display HTML parts.
+     */
+    public bool has_html_body() {
+        return has_body_parts(message.get_mime_part(), "html");
+    }
+
+    /**
+     * Determines if the message has one or plain text display parts.
+     */
+    public bool has_plain_body() {
+        return has_body_parts(message.get_mime_part(), "text");
+    }
+
+    /**
+     * Determines if the message has any body text/subtype MIME parts.
+     *
+     * A body part is one that would be displayed to the user,
+     * i.e. parts returned by { link get_html_body} or { link
+     * get_plain_body}.
+     *
+     * The logic for selecting text nodes here must match that in
+     * construct_body_from_mime_parts.
+     */
+    private bool has_body_parts(GMime.Object node, string text_subtype) {
+        bool has_part = false;
+        Mime.ContentType? this_content_type = null;
+        if (node.get_content_type() != null)
+            this_content_type =
+                new Mime.ContentType.from_gmime(node.get_content_type());
+
+        GMime.Multipart? multipart = node as GMime.Multipart;
+        if (multipart != null) {
+            int count = multipart.get_count();
+            for (int i = 0; i < count && !has_part; ++i) {
+                has_part = has_body_parts(multipart.get_part(i), text_subtype);
+            }
+        } else {
+            GMime.Part? part = node as GMime.Part;
+            if (part != null) {
+                Mime.ContentDisposition? disposition = null;
+                if (part.get_content_disposition() != null)
+                    disposition = new Mime.ContentDisposition.from_gmime(
+                        part.get_content_disposition()
+                    );
+
+                if (disposition == null ||
+                    disposition.disposition_type != Mime.DispositionType.ATTACHMENT) {
+                    if (this_content_type != null &&
+                        this_content_type.has_media_type("text") &&
+                        this_content_type.has_media_subtype(text_subtype)) {
+                        has_part = true;
+                    }
+                }
+            }
+        }
+        return has_part;
+    }
+
     /**
      * This method is the main utility method used by the other body-generating constructors.
      *
@@ -553,80 +612,46 @@ public class Geary.RFC822.Message : BaseObject {
         
         return body;
     }
-    
+
     /**
      * Returns the HTML portion of the message body, if present.
      *
-     * See { link get_body} for more details on how the body is assembled from the message's MIME
-     * structure and the role of the { link InlinePartReplacer}.
+     * Recursively walks the MIME structure (depth-first) serializing
+     * all text/html 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 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.
      *
      * @throws RFC822Error.NOT_FOUND if an HTML body is not present.
      */
     public string? get_html_body(InlinePartReplacer? replacer) throws RFC822Error {
         return internal_get_body("html", false, replacer);
     }
-    
+
     /**
      * Returns the plaintext portion of the message body, if present.
      *
-     * See { link get_body} for more details on how the body is assembled from the message's MIME
-     * structure and the role of the { link InlinePartReplacer}.
+     * Recursively walks the MIME structure (depth-first) serializing
+     * all text/plain 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 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.
      *
-     * The convert_to_html flag indicates if the plaintext body should be converted into HTML.
-     * Note that the InlinePartReplacer's output is not converted; it's up to the caller to know
-     * what format to return when invoked.
+     * The convert_to_html flag indicates if the plaintext body should
+     * be converted into HTML.  Note that the InlinePartReplacer's
+     * output is not converted; it's up to the caller to know what
+     * format to return when invoked.
      *
      * @throws RFC822Error.NOT_FOUND if a plaintext body is not present.
      */
     public string? get_plain_body(bool convert_to_html, InlinePartReplacer? replacer) throws RFC822Error {
         return internal_get_body("plain", convert_to_html, replacer);
     }
-    
-    /**
-     * 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
-     * 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.
-     *
-     * The format flag indicates whether to assemble the HTML portion of the message body or to
-     * convert the plaintext portion into HTML.  If the requested portion is not present, the
-     * method will fallback and attempt to return the other (converted to HTML, if necessary).
-     * It is possible for format to be PLAIN and this method to return an HTML portion of the
-     * message (if plaintext is unavailable).
-     *
-     * Note that the InlinePartReplacer's output is never converted and should return HTML.
-     *
-     * @throws RFC822Error.NOT_FOUND if neither format is available.
-     */
-    public string? get_body(TextFormat format, InlinePartReplacer? replacer) throws RFC822Error {
-        try {
-            switch (format) {
-                case TextFormat.HTML:
-                    return get_html_body(replacer);
-                
-                case TextFormat.PLAIN:
-                    return get_plain_body(true, replacer);
-                
-                default:
-                    return null;
-            }
-        } catch (Error err) {
-            switch (format) {
-                case TextFormat.HTML:
-                    return get_plain_body(true, replacer);
-                
-                case TextFormat.PLAIN:
-                    return get_html_body(replacer);
-                
-                default:
-                    assert_not_reached();
-            }
-        }
-    }
-    
+
     /**
      * Return the body as a searchable string.  The body in this case should
      * include everything visible in the message's body in the client, which
diff --git a/src/engine/rfc822/rfc822-utils.vala b/src/engine/rfc822/rfc822-utils.vala
index 0b3347a..a71caaf 100644
--- a/src/engine/rfc822/rfc822-utils.vala
+++ b/src/engine/rfc822/rfc822-utils.vala
@@ -283,18 +283,32 @@ public string quote_email_for_forward(Geary.Email email, string? quote, TextForm
 }
 
 private string quote_body(Geary.Email email, string? quote, bool use_quotes, TextFormat format) {
-    string? body_text = "";
-    
-    try {
-        body_text = quote ?? email.get_message().get_body(format, null);
-    } catch (Error error) {
-        debug("Could not get message text. %s", error.message);
+    string? body_text = quote ?? "";
+    if (quote == null) {
+        try {
+            Message message = email.get_message();
+            switch (format) {
+            case TextFormat.HTML:
+                body_text = message.has_html_body()
+                    ? message.get_html_body(null)
+                    : message.get_plain_body(true, null);
+                    break;
+
+            case TextFormat.PLAIN:
+                body_text = message.has_plain_body()
+                    ? message.get_plain_body(true, null)
+                    : message.get_html_body(null);
+                    break;
+            }
+        } catch (Error error) {
+            debug("Could not get message text for quoting: %s", error.message);
+        }
     }
-    
+
     // Wrap the whole thing in a blockquote.
     if (use_quotes && !String.is_empty(body_text))
         body_text = "<blockquote type=\"cite\">%s</blockquote>".printf(body_text);
-    
+
     return body_text;
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]