[geary/wip/743676-cid] Further cleanup of the API and documentation



commit c76aeeca5006ce5e8b66d2164f3480f351e3a5d2
Author: Jim Nelson <jim yorba org>
Date:   Fri Feb 13 13:20:48 2015 -0800

    Further cleanup of the API and documentation

 src/client/composer/composer-widget.vala           |   10 +-
 .../conversation-viewer/conversation-viewer.vala   |   36 ++++--
 src/engine/rfc822/rfc822-message.vala              |  132 +++++++++++---------
 src/engine/rfc822/rfc822-utils.vala                |   37 ++++---
 src/engine/rfc822/rfc822.vala                      |    8 ++
 5 files changed, 137 insertions(+), 86 deletions(-)
---
diff --git a/src/client/composer/composer-widget.vala b/src/client/composer/composer-widget.vala
index 7850fda..4c4bff3 100644
--- a/src/client/composer/composer-widget.vala
+++ b/src/client/composer/composer-widget.vala
@@ -453,7 +453,7 @@ public class ComposerWidget : Gtk.EventBox {
                     if (referred.subject != null)
                         subject = referred.subject.value;
                     try {
-                        body_html = referred.get_message().get_body(true);
+                        body_html = referred.get_message().get_body(Geary.RFC822.TextFormat.HTML, null);
                     } catch (Error error) {
                         debug("Error getting message body: %s", error.message);
                     }
@@ -468,7 +468,8 @@ public class ComposerWidget : Gtk.EventBox {
                 case ComposeType.REPLY_ALL:
                     subject = reply_subject;
                     references = Geary.RFC822.Utils.reply_references(referred);
-                    body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_reply(referred, quote, true);
+                    body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_reply(referred, quote,
+                        Geary.RFC822.TextFormat.HTML);
                     pending_attachments = referred.attachments;
                     if (quote != null)
                         top_posting = false;
@@ -478,7 +479,8 @@ public class ComposerWidget : Gtk.EventBox {
                 
                 case ComposeType.FORWARD:
                     subject = forward_subject;
-                    body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_forward(referred, quote, true);
+                    body_html = "\n\n" + Geary.RFC822.Utils.quote_email_for_forward(referred, quote,
+                        Geary.RFC822.TextFormat.HTML);
                     add_attachments(referred.attachments);
                     pending_attachments = referred.attachments;
                 break;
@@ -898,7 +900,7 @@ public class ComposerWidget : Gtk.EventBox {
             WebKit.DOM.Document document = editor.get_dom_document();
             // Always use reply styling, since forward styling doesn't work for inline quotes
             document.exec_command("insertHTML", false,
-                Geary.RFC822.Utils.quote_email_for_reply(referred, quote, true));
+                Geary.RFC822.Utils.quote_email_for_reply(referred, quote, Geary.RFC822.TextFormat.HTML));
             
             if (!referred_ids.contains(referred.id)) {
                 add_recipients_and_ids(new_type, referred);
diff --git a/src/client/conversation-viewer/conversation-viewer.vala 
b/src/client/conversation-viewer/conversation-viewer.vala
index ddcf8bc..4206857 100644
--- a/src/client/conversation-viewer/conversation-viewer.vala
+++ b/src/client/conversation-viewer/conversation-viewer.vala
@@ -848,18 +848,31 @@ public class ConversationViewer : Gtk.Box {
         } catch (Error error) {
             debug("Failed to add preview text: %s", error.message);
         }
-
+        
+        //
+        // Build an HTML document from the email with two passes:
+        //
+        // * 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
+        //   converts them to an IMG tag with a data: URI if they are a supported image type.
+        //   Otherwise, the MIME part is dropped.
+        //
+        // * insert_html_markup() then strips everything outside the BODY, turning the BODY tag
+        //   itself into a DIV, and performs other massaging of the HTML.  It also looks for IMG
+        //   tags that refer to other MIME parts via their Content-ID, converts them to data: URIs,
+        //   and inserts them into the document.
+        //
+        // Attachments are generated and added in add_message(), which calls this method before
+        // building the HTML for them.  The above two steps take steps to avoid inlining images
+        // that are actually attachments (in particular, get_body() considers their
+        // Content-Disposition)
+        //
+        
         string body_text = "";
         remote_images = false;
         try {
-            // get_body() recursively walks the MIME structure and serializes all text portions
-            // into a single string, converting to HTML in the process.  The inline image replacer
-            // is called for MIME parts that should be converted to HTML (i.e. for images not listed
-            // as attachments or as referenced elsewhere in the document by a Content-ID)
-            body_text = message.get_body(true, inline_image_replacer) ?? "";
-            
-            // Next, convert the HTML into HTML suitable for our purposes, replacing references
-            // to images with data: URI's, noting when images refer to remote sources, and so forth
+            body_text = message.get_body(Geary.RFC822.TextFormat.HTML, 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);
@@ -922,6 +935,11 @@ public class ConversationViewer : Gtk.Box {
         return false;
     }
     
+    // 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.
+    // 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,
         Geary.Mime.ContentDisposition? disposition, string? content_id, Geary.Memory.Buffer buffer) {
         if (content_type == null) {
diff --git a/src/engine/rfc822/rfc822-message.vala b/src/engine/rfc822/rfc822-message.vala
index 87df7b9..454317f 100644
--- a/src/engine/rfc822/rfc822-message.vala
+++ b/src/engine/rfc822/rfc822-message.vala
@@ -310,7 +310,7 @@ public class Geary.RFC822.Message : BaseObject {
     public string get_preview() {
         string? preview = null;
         try {
-            preview = get_text_body(false, null);
+            preview = get_plain_body(false, null);
         } catch (Error e) {
             try {
                 preview = Geary.HTML.remove_html_tags(get_html_body(null));
@@ -446,33 +446,33 @@ public class Geary.RFC822.Message : BaseObject {
     }
     
     /**
-     * This method is the main utility method used by the other body constructors. It calls itself
-     * recursively via the last argument ("node").
-     * 
-     * The constructed body is stored in ref string? body. If the constructed body is null,
-     * ref string? body remains unmodified.
+     * This method is the main utility method used by the other body-generating constructors.
      *
-     * ref string? body is only modified if the constructed body is non-empty
-     * 
-     * Returns: a bool indicating whether a text part with the desired text_subtype was found
+     * 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.
+     *
+     * 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.).
+     *
+     * The final constructed body is stored in the body string.
+     *
+     * The initial call should pass the root of this message and UNSPECIFIED as its container
+     * subtype.
+     *
+     * @returns Whether a text part with the desired text_subtype was found
      */
-    private bool construct_body_from_mime_parts(ref string? body, InlinePartReplacer? replacer,
-        string text_subtype, bool to_html, GMime.Object? node, Mime.MultipartSubtype container_subtype)
-        throws RFC822Error {
-        if (node == null) {
-            node = message.get_mime_part();
-        }
-        
+    private bool construct_body_from_mime_parts(GMime.Object node, Mime.MultipartSubtype container_subtype,
+        string text_subtype, bool to_html, InlinePartReplacer? replacer, ref string? body) throws 
RFC822Error {
         Mime.ContentType? this_content_type = null;
         if (node.get_content_type() != null)
             this_content_type = new Mime.ContentType.from_gmime(node.get_content_type());
         
-        Mime.MultipartSubtype this_subtype = Mime.MultipartSubtype.from_content_type(this_content_type,
-            null);
-            
         // If this is a multipart, call ourselves recursively on the children
         GMime.Multipart? multipart = node as GMime.Multipart;
         if (multipart != null) {
+            Mime.MultipartSubtype this_subtype = Mime.MultipartSubtype.from_content_type(this_content_type,
+                null);
+            
             bool found_text_subtype = false;
             
             StringBuilder builder = new StringBuilder();
@@ -481,13 +481,13 @@ public class Geary.RFC822.Message : BaseObject {
                 GMime.Object child = multipart.get_part(i);
                 
                 string? child_body = null;
-                found_text_subtype |= construct_body_from_mime_parts(ref child_body, replacer,
-                    text_subtype, to_html, child, this_subtype);
+                found_text_subtype |= construct_body_from_mime_parts(child, this_subtype, text_subtype,
+                    to_html, replacer, ref child_body);
                 if (child_body != null)
                     builder.append(child_body);
             }
             
-            if (!Geary.String.is_empty_or_whitespace(builder.str))
+            if (!String.is_empty(builder.str))
                 body = builder.str;
             
             return found_text_subtype;
@@ -506,9 +506,7 @@ public class Geary.RFC822.Message : BaseObject {
         if (disposition != null && disposition.disposition_type == Mime.DispositionType.ATTACHMENT)
             return false;
         
-        /* Handle text parts that are not attachments
-         * They may have inline disposition, or they may have no disposition specified
-         */
+        // Assemble body from text parts that are not attachments
         if (this_content_type != null && this_content_type.has_media_type("text")) {
             if (this_content_type.has_media_subtype(text_subtype)) {
                 body = mime_part_to_memory_buffer(part, true, to_html).to_string();
@@ -532,28 +530,21 @@ public class Geary.RFC822.Message : BaseObject {
             return false;
         
         // Hand off to the replacer for processing
-        string? replaced_part = replacer(RFC822.Utils.get_clean_attachment_filename(part),
+        body = replacer(RFC822.Utils.get_clean_attachment_filename(part),
             this_content_type, disposition, part.get_content_id(), mime_part_to_memory_buffer(part));
-        if (replaced_part != null)
-            body = replaced_part;
         
-        return replaced_part != null;
+        return body != null;
     }
     
     /**
      * A front-end to construct_body_from_mime_parts() that converts its output parameters into
      * something that front-facing methods want to return.
-     *
-     * The allow_only_replaced flag indicates if it's allowable for the method to return only the
-     * InlinePartReplacer's returned text.  In other words, if only an inline MIME section is found
-     * but no portion of text_subtype, allow_only_replaced indicates if the InlinePartReplacer's
-     * returned text constitutes a "body".
      */
-    private string? internal_get_body(bool allow_only_replaced, string text_subtype, bool to_html,
-        InlinePartReplacer? replacer) throws RFC822Error {
+    private string? internal_get_body(string text_subtype, bool to_html, InlinePartReplacer? replacer)
+        throws RFC822Error {
         string? body = null;
-        if (!construct_body_from_mime_parts(ref body, replacer, text_subtype, to_html, null,
-            Mime.MultipartSubtype.UNSPECIFIED)) {
+        if (!construct_body_from_mime_parts(message.get_mime_part(), Mime.MultipartSubtype.UNSPECIFIED,
+            text_subtype, to_html, replacer, ref body)) {
             throw new RFC822Error.NOT_FOUND("Could not find any \"text/%s\" parts", text_subtype);
         }
         
@@ -563,48 +554,73 @@ public class Geary.RFC822.Message : BaseObject {
     /**
      * Returns the HTML portion of the message body, if present.
      *
-     * Throws { link RFC822Error.NOT_FOUND} if an HTML body is not 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}.
+     *
+     * @throws { link RFC822Error.NOT_FOUND} if an HTML body is not present.
      */
-    private string? get_html_body(InlinePartReplacer? replacer) throws RFC822Error {
-        return internal_get_body(true, "html", false, replacer);
+    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}.
+     *
      * 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 { link RFC822Error.NOT_FOUND} if a plaintext body is not present.
+     * @throws RFC822Error.NOT_FOUND if a plaintext body is not present.
      */
-    private string? get_text_body(bool convert_to_html, InlinePartReplacer? replacer) throws RFC822Error {
-        return internal_get_body(true, "plain", convert_to_html, replacer);
+    public string? get_plain_body(bool convert_to_html, InlinePartReplacer? replacer) throws RFC822Error {
+        return internal_get_body("plain", convert_to_html, replacer);
     }
     
     /**
-     * Returns a body of the email as HTML.
+     * Returns the complete email body as HTML.
      *
-     * The html_format flag indicates whether to use the HTML portion of the message body or to
+     * 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 html_format to be false and this method to return HTML (if plaintext
-     * is unavailable).  Consider using { link get_html_body} or { link get_text_body} if finer
-     * control is desired.
+     * 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 { link RFC822Error.NOT_FOUND if neither format is available.
+     * @throws RFC822Error.NOT_FOUND if neither format is available.
      */
-    public string? get_body(bool html_format, InlinePartReplacer? replacer = null) throws RFC822Error {
+    public string? get_body(TextFormat format, InlinePartReplacer? replacer) throws RFC822Error {
         try {
-            return html_format
-                ? internal_get_body(false, "html", false, replacer)
-                : internal_get_body(false, "plain", true, replacer);
+            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) {
-            return html_format
-                ? internal_get_body(true, "plain", true, replacer)
-                : internal_get_body(true, "html", false, replacer);
+            switch (format) {
+                case TextFormat.HTML:
+                    return get_plain_body(true, replacer);
+                
+                case TextFormat.PLAIN:
+                    return get_html_body(replacer);
+                
+                default:
+                    assert_not_reached();
+            }
         }
     }
     
@@ -623,7 +639,7 @@ public class Geary.RFC822.Message : BaseObject {
             html = true;
         } catch (Error e) {
             try {
-                body = get_text_body(false, null);
+                body = get_plain_body(false, null);
             } catch (Error e) {
                 // Ignore.
             }
diff --git a/src/engine/rfc822/rfc822-utils.vala b/src/engine/rfc822/rfc822-utils.vala
index 4f2da6b..abed06c 100644
--- a/src/engine/rfc822/rfc822-utils.vala
+++ b/src/engine/rfc822/rfc822-utils.vala
@@ -184,13 +184,20 @@ public string reply_references(Geary.Email source) {
     return (list.size > 0) ? string.joinv(" ", strings) : "";
 }
 
-public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses,
-    bool html_format) {
-    
+public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses, TextFormat format) {
     if (addresses == null)
         return "";
     
-    return html_format ? HTML.escape_markup(addresses.to_string()) : addresses.to_string();
+    switch (format) {
+        case TextFormat.HTML:
+            return HTML.escape_markup(addresses.to_string());
+        
+        case TextFormat.PLAIN:
+            return addresses.to_string();
+        
+        default:
+            assert_not_reached();
+    }
 }
 
 
@@ -203,7 +210,7 @@ public string email_addresses_for_reply(Geary.RFC822.MailboxAddresses? addresses
  * If html_format is true, the message will be quoted in HTML format.
  * Otherwise it will be in plain text.
  */
-public string quote_email_for_reply(Geary.Email email, string? quote, bool html_format) {
+public string quote_email_for_reply(Geary.Email email, string? quote, TextFormat format) {
     if (email.body == null && quote == null)
         return "";
     
@@ -219,13 +226,13 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
         /// the original sender.
         string QUOTED_LABEL = _("On %1$s, %2$s wrote:");
         quoted += QUOTED_LABEL.printf(email.date.value.format(DATE_FORMAT),
-                                      email_addresses_for_reply(email.from, html_format));
+                                      email_addresses_for_reply(email.from, format));
 
     } else if (email.from != null) {
         /// The quoted header for a message being replied to (in case the date is not known).
         /// %s will be replaced by the original sender.
         string QUOTED_LABEL = _("%s wrote:");
-        quoted += QUOTED_LABEL.printf(email_addresses_for_reply(email.from, html_format));
+        quoted += QUOTED_LABEL.printf(email_addresses_for_reply(email.from, format));
 
     } else if (email.date != null) {
         /// The quoted header for a message being replied to (in case the sender is not known).
@@ -236,7 +243,7 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
     
     quoted += "<br />";
     
-    quoted += "\n" + quote_body(email, quote, true, html_format);
+    quoted += "\n" + quote_body(email, quote, true, format);
     
     if (quote != null)
         quoted += "<br /><br />\n";
@@ -253,7 +260,7 @@ public string quote_email_for_reply(Geary.Email email, string? quote, bool html_
  * If html_format is true, the message will be quoted in HTML format.
  * Otherwise it will be in plain text.
  */
-public string quote_email_for_forward(Geary.Email email, string? quote, bool html_format) {
+public string quote_email_for_forward(Geary.Email email, string? quote, TextFormat format) {
     if (email.body == null && quote == null)
         return "";
     
@@ -261,31 +268,31 @@ public string quote_email_for_forward(Geary.Email email, string? quote, bool htm
     
     quoted += _("---------- Forwarded message ----------");
     quoted += "\n\n";
-    string from_line = email_addresses_for_reply(email.from, html_format);
+    string from_line = email_addresses_for_reply(email.from, format);
     if (!String.is_empty_or_whitespace(from_line))
         quoted += _("From: %s\n").printf(from_line);
     quoted += _("Subject: %s\n").printf(email.subject != null ? email.subject.to_string() : "");
     quoted += _("Date: %s\n").printf(email.date != null ? email.date.to_string() : "");
-    string to_line = email_addresses_for_reply(email.to, html_format);
+    string to_line = email_addresses_for_reply(email.to, format);
     if (!String.is_empty_or_whitespace(to_line))
         quoted += _("To: %s\n").printf(to_line);
-    string cc_line = email_addresses_for_reply(email.cc, html_format);
+    string cc_line = email_addresses_for_reply(email.cc, format);
     if (!String.is_empty_or_whitespace(cc_line))
         quoted += _("Cc: %s\n").printf(cc_line);
     quoted += "\n";  // A blank line between headers and body
     
     quoted = quoted.replace("\n", "<br />");
     
-    quoted += quote_body(email, quote, false, html_format);
+    quoted += quote_body(email, quote, false, format);
     
     return quoted;
 }
 
-private string quote_body(Geary.Email email, string? quote, bool use_quotes, bool html_format) {
+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(html_format);
+        body_text = quote ?? email.get_message().get_body(format, null);
     } catch (Error error) {
         debug("Could not get message text. %s", error.message);
     }
diff --git a/src/engine/rfc822/rfc822.vala b/src/engine/rfc822/rfc822.vala
index 170d5c5..9f2db8d 100644
--- a/src/engine/rfc822/rfc822.vala
+++ b/src/engine/rfc822/rfc822.vala
@@ -6,6 +6,14 @@
 
 namespace Geary.RFC822 {
 
+/**
+ * Common text formats supported by { link Geary.RFC822}.
+ */
+public enum TextFormat {
+    PLAIN,
+    HTML
+}
+
 private int init_count = 0;
 
 internal Regex? invalid_filename_character_re = null;


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