Bug 736798 - Opening attached .eml's shouldn't list parent email's attachments too; r=bienvenu,a=Standard8
authorJim Porter <squibblyflabbetydoo@gmail.com>
Wed, 11 Apr 2012 23:03:42 -0500
changeset 10734 a4c78c9d4606c271ff4c7d013476183b615440c6
parent 10733 3936c046a8570c290c7310d36af2ec4bd4e1a978
child 10735 d464d18772e8a5d1995cb05f91d47719994b4d07
push idunknown
push userunknown
push dateunknown
reviewersbienvenu, Standard8
bugs736798
Bug 736798 - Opening attached .eml's shouldn't list parent email's attachments too; r=bienvenu,a=Standard8
mail/test/mozmill/attachment/test-attachment.js
mailnews/mime/src/mimeobj.cpp
mailnews/mime/src/mimeobj.h
--- a/mail/test/mozmill/attachment/test-attachment.js
+++ b/mail/test/mozmill/attachment/test-attachment.js
@@ -72,16 +72,23 @@ var setupModule = function (module) {
   wh.installInto(module);
   let composeHelper = collector.getModule('compose-helpers');
   composeHelper.installInto(module);
   let wh = collector.getModule('window-helpers');
   wh.installInto(module);
 
   folder = create_folder("AttachmentA");
 
+  var attachedMessage = msgGen.makeMessage({
+    body: { body: "I'm an attached email!" },
+    attachments: [{ body: textAttachment,
+                    filename: 'inner attachment.txt',
+                    format: '' }],
+  });
+
   // create some messages that have various types of attachments
   messages = [
     // no attachment
     {},
     // text attachment
     { attachments: [{ body: textAttachment,
                       filename: 'ubik.txt',
                       format: '' }],
@@ -106,16 +113,28 @@ var setupModule = function (module) {
     { attachments: [{ body: textAttachment,
                       filename: 'this-is-a-file-with-an-extremely-long-name-' +
                                 'that-seems-to-go-on-forever-seriously-you-' +
                                 'would-not-believe-how-long-this-name-is-it-' +
                                 'surely-exceeds-the-maximum-filename-length-' +
                                 'for-most-filesystems.txt',
                       format: '' }],
     },
+    // a message with a text attachment and an email attachment, which in turn
+    // has its own text attachment
+    {
+      bodyPart: new SyntheticPartMultiMixed([
+        new SyntheticPartLeaf("I'm a message!"),
+        new SyntheticPartLeaf(textAttachment,
+                              { filename: 'outer attachment.txt',
+                                contentType: 'text/plain',
+                                format: '' }),
+        attachedMessage,
+      ]),
+    },
     // evilly-named attachment; spaces should be collapsed and trimmed on the
     // ends
     { attachments: [{ body: textAttachment,
                       contentType: 'application/octet-stream',
                       filename: ' ubik  .txt                            .evil ',
                       sanitizedFilename: 'ubik .txt .evil',
                       format: '' }],
     },
@@ -199,16 +218,44 @@ function test_long_attachment_name() {
 
   let messagepaneBox = mc.e("messagepanebox");
   let attachmentBar = mc.e("attachmentBar");
 
   assert_true(messagepaneBox.boxObject.width >= attachmentBar.boxObject.width,
               "Attachment bar has expanded off the edge of the window!");
 }
 
+/**
+ * Make sure that, when opening attached messages, we only show the attachments
+ * "beneath" the attached message (as opposed to all attachments for the root
+ * message).
+ */
+function test_attached_message_attachments() {
+  be_in_folder(folder);
+
+  select_click_row(5);
+  assert_selected_and_displayed(5);
+
+  // Make sure we have the expected number of attachments in the root message:
+  // an outer text attachment, an attached email, and an inner text attachment.
+  assert_equals(mc.e("attachmentList").itemCount, 3);
+
+  // Open the attached email.
+  plan_for_new_window("mail:messageWindow");
+  mc.e("attachmentList").getItemAtIndex(1).attachment.open();
+  let msgc = wait_for_new_window("mail:messageWindow");
+  wait_for_message_display_completion(msgc, true);
+
+  // Make sure we have the expected number of attachments in the attached
+  // message: just an inner text attachment.
+  assert_equals(msgc.e("attachmentList").itemCount, 1);
+
+  close_window(msgc);
+}
+
 function test_attachment_name_click() {
   be_in_folder(folder);
 
   select_click_row(1);
   assert_selected_and_displayed(1);
 
   let attachmentList = mc.e("attachmentList");
 
--- a/mailnews/mime/src/mimeobj.cpp
+++ b/mailnews/mime/src/mimeobj.cpp
@@ -232,36 +232,39 @@ MimeObject_parse_begin (MimeObject *obj)
     obj->output_p = false;
   else if (!obj->options->part_to_load)
     obj->output_p = true;
   else
   {
     char *id = mime_part_address(obj);
     if (!id) return MIME_OUT_OF_MEMORY;
 
-      // We need to check if a part is the subpart of the part to load.
-      // If so and this is a raw or body display output operation, then
-      // we should mark the part for subsequent output.
-      //
+    // We need to check if a part is the subpart of the part to load.
+    // If so and this is a raw or body display output operation, then
+    // we should mark the part for subsequent output.
 
-      // First, check for an exact match
-      obj->output_p = !strcmp(id, obj->options->part_to_load);
-      if (!obj->output_p && (obj->options->format_out == nsMimeOutput::nsMimeMessageRaw ||
-             obj->options->format_out == nsMimeOutput::nsMimeMessageBodyDisplay
-             || obj->options->format_out == nsMimeOutput::nsMimeMessageAttach))
+    // First, check for an exact match
+    obj->output_p = !strcmp(id, obj->options->part_to_load);
+    if (!obj->output_p && (obj->options->format_out == nsMimeOutput::nsMimeMessageRaw ||
+                           obj->options->format_out == nsMimeOutput::nsMimeMessageBodyDisplay ||
+                           obj->options->format_out == nsMimeOutput::nsMimeMessageAttach))
     {
-        // Then, check for subpart
-        unsigned int partlen = strlen(obj->options->part_to_load);
-        obj->output_p = (strlen(id) >= partlen + 2) && (id[partlen] == '.') &&
-            !strncmp(id, obj->options->part_to_load, partlen);
-      }
+      // Then, check for subpart
+      unsigned int partlen = strlen(obj->options->part_to_load);
+      obj->output_p = (strlen(id) >= partlen + 2) && (id[partlen] == '.') &&
+        !strncmp(id, obj->options->part_to_load, partlen);
+    }
 
     PR_Free(id);
   }
 
+  // If we've decided not to output this part, we also shouldn't be showing it
+  // as an attachment.
+  obj->dontShowAsAttachment = !obj->output_p;
+
   return 0;
 }
 
 static int
 MimeObject_parse_buffer (const char *buffer, PRInt32 size, MimeObject *obj)
 {
   NS_ASSERTION(!obj->closed_p, "object shouldn't be closed");
   if (obj->closed_p) return -1;
--- a/mailnews/mime/src/mimeobj.h
+++ b/mailnews/mime/src/mimeobj.h
@@ -187,18 +187,19 @@ struct MimeObject {
   MimeObject *parent;    /* Backpointer to a MimeContainer object. */
 
   MimeDisplayOptions *options;  /* Display preferences set by caller. */
 
   bool closed_p;      /* Whether it's done being written to. */
   bool parsed_p;      /* Whether the parser has been shut down. */
   bool output_p;      /* Whether it should be written. */
   bool dontShowAsAttachment; /* Force an object to not be shown as attachment,
-                                  but when is false, it doesn't mean it will be
-                                  shown as attachment */
+                                but when is false, it doesn't mean it will be
+                                shown as attachment; specifically, body parts
+                                are never shown as attachments. */
 
   /* Read-buffer and write-buffer (on input, `parse_buffer' uses ibuffer to
    compose calls to `parse_line'; on output, `obuffer' is used in various
    ways by various routines.)  These buffers are created and grow as needed.
    `ibuffer' should be generally be considered hands-off, and `obuffer'
    should generally be considered fair game.
    */
   char *ibuffer, *obuffer;