fix handling of inline text attachments when display attachments inline is set to false, r=squib, bug 583419
authorDavid Bienvenu <bienvenu@nventure.com>
Fri, 06 May 2011 08:19:23 -0700
changeset 7714 d538b947880cd3cfb8b51fb6c36aade2c8053b0b
parent 7713 165a011f1c2acc8152862a7b7cd13fde6429b768
child 7715 87dfb7e869387c759bd074c6a35823baea1299ad
push id5920
push userbienvenu@nventure.com
push dateFri, 06 May 2011 15:18:52 +0000
treeherdercomm-central@d538b947880c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssquib, bug
bugs583419
fix handling of inline text attachments when display attachments inline is set to false, r=squib, bug 583419
mailnews/mime/src/mimei.cpp
mailnews/mime/src/mimei.h
mailnews/mime/src/mimemalt.cpp
mailnews/mime/test/unit/test_text_attachment.js
--- a/mailnews/mime/src/mimei.cpp
+++ b/mailnews/mime/src/mimei.cpp
@@ -821,17 +821,17 @@ mime_find_class (const char *content_typ
   }
 
   return clazz;
 }
 
 
 MimeObject *
 mime_create (const char *content_type, MimeHeaders *hdrs,
-       MimeDisplayOptions *opts)
+       MimeDisplayOptions *opts, PRBool forceInline /* = PR_FALSE */)
 {
   /* If there is no Content-Disposition header, or if the Content-Disposition
    is ``inline'', then we display the part inline (and let mime_find_class()
    decide how.)
 
    If there is any other Content-Disposition (either ``attachment'' or some
    disposition that we don't recognise) then we always display the part as
    an external link, by using MimeExternalObject to display it.
@@ -933,27 +933,40 @@ mime_create (const char *content_type, M
           (clazz != (MimeObjectClass *)&mimeInlineTextEnrichedClass) &&
           (clazz != (MimeObjectClass *)&mimeMessageClass) &&
           (clazz != (MimeObjectClass *)&mimeInlineImageClass) )
       clazz = (MimeObjectClass *)&mimeExternalObjectClass;
   }
 
   /* If the option `Show Attachments Inline' is off, now would be the time to change our mind... */
   /* Also, if we're doing a reply (i.e. quoting the body), then treat that according to preference. */
-  if (opts && (!opts->show_attachment_inline_p ||
+  if (opts && ((!opts->show_attachment_inline_p && !forceInline) ||
                (!opts->quote_attachment_inline_p &&
                 (opts->format_out == nsMimeOutput::nsMimeMessageQuoting ||
                  opts->format_out == nsMimeOutput::nsMimeMessageBodyQuoting))))
   {
     if (mime_subclass_p(clazz, (MimeObjectClass *)&mimeInlineTextClass))
     {
       /* It's a text type.  Write it only if it's the *first* part
-         that we're writing*/
+         that we're writing, and then only if it has no "filename"
+         specified (the assumption here being, if it has a filename,
+         it wasn't simply typed into the text field -- it was actually
+         an attached document.) */
       if (opts->state && opts->state->first_part_written_p)
         clazz = (MimeObjectClass *)&mimeExternalObjectClass;
+      else
+      {
+        /* If there's a name, then write this as an attachment. */
+        char *name = (hdrs ? MimeHeaders_get_name(hdrs, opts) : nsnull);
+        if (name)
+        {
+          clazz = (MimeObjectClass *)&mimeExternalObjectClass;
+          PR_Free(name);
+        }
+      }
     }
     else
       if (mime_subclass_p(clazz,(MimeObjectClass *)&mimeContainerClass) &&
            !mime_subclass_p(clazz,(MimeObjectClass *)&mimeMessageClass))
         /* Multipart subtypes are ok, except for messages; descend into
            multiparts, and defer judgement.
 
            Encrypted blobs are just like other containers (make the crypto
--- a/mailnews/mime/src/mimei.h
+++ b/mailnews/mime/src/mimei.h
@@ -269,22 +269,24 @@ extern "C" void mime_free (MimeObject *o
    then "text/x-unknown" will return MimeInlineTextPlainType, but if it is
    false, it will return NULL.
  */
 extern MimeObjectClass *mime_find_class (const char *content_type,
                      MimeHeaders *hdrs,
                      MimeDisplayOptions *opts,
                      PRBool exact_match_p);
 
-/* Given a content-type string, creates and returns an appropriate subclass
-   of MimeObject.  The headers (from which the content-type was presumably
-   extracted) are copied.
+/** Given a content-type string, creates and returns an appropriate subclass
+ * of MimeObject.  The headers (from which the content-type was presumably
+ * extracted) are copied. forceInline is set to true when the caller wants
+ * the function to ignore opts->show_attachment_inline_p and force inline
+ * display, e.g., mimemalt wants the body part to be shown inline.
  */
 extern MimeObject *mime_create (const char *content_type, MimeHeaders *hdrs,
-                MimeDisplayOptions *opts);
+                MimeDisplayOptions *opts, PRBool forceInline = PR_FALSE);
 
 
 /* Querying the type hierarchy */
 extern PRBool mime_subclass_p(MimeObjectClass *child,
                  MimeObjectClass *parent);
 extern PRBool mime_typep(MimeObject *obj, MimeObjectClass *clazz);
 
 /* Returns a string describing the location of the part (like "2.5.3").
--- a/mailnews/mime/src/mimemalt.cpp
+++ b/mailnews/mime/src/mimemalt.cpp
@@ -464,23 +464,24 @@ MimeMultipartAlternative_display_cached_
 {
   int status;
 
   char *ct = (hdrs
         ? MimeHeaders_get (hdrs, HEADER_CONTENT_TYPE, PR_TRUE, PR_FALSE)
         : 0);
   const char *dct = (((MimeMultipartClass *) obj->clazz)->default_part_type);
   MimeObject *body;
+  /** Don't pass in NULL as the content-type (this means that the
+   * auto-uudecode-hack won't ever be done for subparts of a
+   * multipart, but only for untyped children of message/rfc822.
+   */
   const char *uct = (ct && *ct) ? ct : (dct ? dct: TEXT_PLAIN);
-  
-  /* Don't pass in NULL as the content-type (this means that the
-   auto-uudecode-hack won't ever be done for subparts of a
-   multipart, but only for untyped children of message/rfc822.
-   */
-  body = mime_create(uct, hdrs, obj->options);
+
+  // We always want to display the cached part inline.
+  body = mime_create(uct, hdrs, obj->options, PR_TRUE);
   PR_FREEIF(ct);
   if (!body) return MIME_OUT_OF_MEMORY;
   body->output_p = do_display;
 
   status = ((MimeContainerClass *) obj->clazz)->add_child(obj, body);
   if (status < 0)
   {
     mime_free(body);
new file mode 100644
--- /dev/null
+++ b/mailnews/mime/test/unit/test_text_attachment.js
@@ -0,0 +1,131 @@
+/* ***** BEGIN LICENSE BLOCK *****
+ * Version: MPL 1.1/GPL 2.0/LGPL 2.1
+ *
+ * The contents of this file are subject to the Mozilla Public License Version
+ * 1.1 (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ * http://www.mozilla.org/MPL/
+ *
+ * Software distributed under the License is distributed on an "AS IS" basis,
+ * WITHOUT WARRANTY OF ANY KIND, either express or implied. See the License
+ * for the specific language governing rights and limitations under the
+ * License.
+ *
+ * The Original Code is mozilla.org code.
+ *
+ * The Initial Developer of the Original Code is
+ * David Bienvenu <bienvenu@mozillamessaging.com>.
+ * Portions created by the Initial Developer are Copyright (C) 2011
+ * the Initial Developer. All Rights Reserved.
+ *
+ * Alternatively, the contents of this file may be used under the terms of
+ * either the GNU General Public License Version 2 or later (the "GPL"), or
+ * the GNU Lesser General Public License Version 2.1 or later (the "LGPL"),
+ * in which case the provisions of the GPL or the LGPL are applicable instead
+ * of those above. If you wish to allow use of your version of this file only
+ * under the terms of either the GPL or the LGPL, and not to allow others to
+ * use your version of this file under the terms of the MPL, indicate your
+ * decision by deleting the provisions above and replace them with the notice
+ * and other provisions required by the GPL or the LGPL. If you do not delete
+ * the provisions above, a recipient may use your version of this file under
+ * the terms of any one of the MPL, the GPL or the LGPL.
+ *
+ * ***** END LICENSE BLOCK ***** */
+
+/**
+ * This test verifies that we don't display text attachments inline
+ * when mail.inline_attachments is false.
+ */
+load("../../../resources/mailDirService.js");
+load("../../../resources/mailTestUtils.js");
+load("../../../resources/logHelper.js");
+load("../../../resources/asyncTestUtils.js");
+
+load("../../../resources/messageGenerator.js");
+load("../../../resources/messageModifier.js");
+load("../../../resources/messageInjection.js");
+
+let gMessenger = Cc["@mozilla.org/messenger;1"]
+                   .createInstance(Ci.nsIMessenger);
+
+// Create a message generator
+const msgGen = gMessageGenerator = new MessageGenerator();
+
+const textAttachment =
+  "inline text attachment";
+
+// create a message with a text attachment
+let messages = [
+  // text attachment
+  { attachments: [{ body: textAttachment,
+                    filename: 'test.txt',
+                    format: '' }]},
+  ];
+
+
+let gStreamListener = {
+  QueryInterface: XPCOMUtils.generateQI([Ci.nsIStreamListener]),
+
+  _str:"",
+  // nsIRequestObserver part
+  onStartRequest: function (aRequest, aContext) {
+  },
+  onStopRequest: function (aRequest, aContext, aStatusCode) {
+    // check that text attachment contents didn't end up inline.
+    do_check_eq(this._str.indexOf(textAttachment), -1);
+    async_driver();
+  },
+
+  /* okay, our onDataAvailable should actually never be called.  the stream
+     converter is actually eating everything except the start and stop
+     notification. */
+  // nsIStreamListener part
+  _stream : null,
+
+  onDataAvailable: function (aRequest,aContext,aInputStream,aOffset,aCount) {
+    if (this._stream === null) {
+      this._stream = Cc["@mozilla.org/scriptableinputstream;1"].
+                    createInstance(Ci.nsIScriptableInputStream);
+      this._stream.init(aInputStream);
+    }
+    this._str += this._stream.read(aCount);
+  },
+};
+
+let msgWindow = Cc["@mozilla.org/messenger/msgwindow;1"]
+                  .createInstance(Ci.nsIMsgWindow);
+
+function test_message_attachments(info) {
+  let synMsg = gMessageGenerator.makeMessage(info);
+  let synSet = new SyntheticMessageSet([synMsg]);
+  yield add_sets_to_folder(gInbox, [synSet]);
+
+  let msgURI = synSet.getMsgURI(0);
+  let msgService = gMessenger.messageServiceFromURI(msgURI);
+
+  let streamURI = msgService.streamMessage(
+    msgURI,
+    gStreamListener,
+    msgWindow,
+    null,
+    true, // have them create the converter
+    // additional uri payload, note that "header=" is prepended automatically
+    "filter",
+    false);
+
+  yield false;
+}
+
+/* ===== Driver ===== */
+
+let tests = [
+  parameterizeTest(test_message_attachments, messages),
+];
+
+let gInbox;
+
+function run_test() {
+  gInbox = configure_message_injection({mode: "local"});
+  Services.prefs.setBoolPref("mail.inline_attachments", false);
+  async_run_tests(tests);
+}