Commons Email
  1. Commons Email
  2. EMAIL-23

[email] [patch] HTML email with plain text alternative and attachments

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: Nightly Builds
    • Fix Version/s: None
    • Labels:
      None
    • Environment:

      Operating System: other
      Platform: Other

      Description

      With the current MultiPartEmail class, you currently cannot properly create a
      message that contains an html part, a text alternative, and attachments. The
      current HtmlEmail will not properly create such a message either. The attached
      patch to MultiPartEmail allows you to do this, e.g.:
      MultiPartEmail email = new MultiPartEmail();
      email.setFrom("me@mydomain.com");
      email.addTo("you@yourdomain.com");
      email.setSubject("message subject");
      email.setSubType("mixed");

      MimeMultipart multiPart = new MimeMultipart("alternative");

      // text alternative
      MimeBodyPart bodyPart1 = new MimeBodyPart();
      bodyPart1.setContent("Text message", "text/plain");
      multiPart.addBodyPart(bodyPart1);

      // html alternative
      MimeBodyPart bodyPart2 = new MimeBodyPart();
      bodyPart2.setContent("<html><body><p>html message</p></body></html>", "text/html");
      multiPart.addBodyPart(bodyPart2);

      email.addPart(multiPart);
      // add attachments
      email.send()

        Activity

        Hide
        Corey Scott added a comment -

        All sounds great to me Eric, sorry about the name of the test case, I just
        feel like being creative at the time

        -Corey

        Show
        Corey Scott added a comment - All sounds great to me Eric, sorry about the name of the test case, I just feel like being creative at the time -Corey
        Hide
        David Eric Pugh added a comment -

        Okay.. I think we can close this now! I applied it, but changed the name of
        the new testcase, I don't like test cases named after bugs, as then you need to
        go look at the bug to see what the testcase is about. I did add the bug id in
        the description though..

        Show
        David Eric Pugh added a comment - Okay.. I think we can close this now! I applied it, but changed the name of the new testcase, I don't like test cases named after bugs, as then you need to go look at the bug to see what the testcase is about. I did add the bug id in the description though..
        Hide
        David Eric Pugh added a comment -

        Is this then the one I should apply? It has the sum total of previous changes,
        correct?

        Show
        David Eric Pugh added a comment - Is this then the one I should apply? It has the sum total of previous changes, correct?
        Hide
        Corey Scott added a comment -

        Created an attachment (id=13379)
        resubmission of previous patches with minor changes (thanks to directions from scott)

        Show
        Corey Scott added a comment - Created an attachment (id=13379) resubmission of previous patches with minor changes (thanks to directions from scott)
        Hide
        Corey Scott added a comment -

        Scott (all),

        Thanks for you help and patience with this matter.
        I am happy to say that I believe that I have finally got it licked once and
        for all.

        Please let me know how is goes for you.
        -Corey

        Show
        Corey Scott added a comment - Scott (all), Thanks for you help and patience with this matter. I am happy to say that I believe that I have finally got it licked once and for all. Please let me know how is goes for you. -Corey
        Hide
        Scott added a comment -

        Hi,

        This is a step in the right direction...a message with html and plain text will
        display properly. However I noticed two issues after applying the patch:

        1. Embedded images. It appears to construct a message like (assuming 1 embedded
        jpeg):
        multipart/alternative
        text/plain
        text/html
        image/jpeg

        Since they all belong to the alternative part, only the last part (the image) is
        displayed by the email client, and the rest is ignored. The html and embedded
        image content need to be in a nested mulipart/related.

        2. Attachments. Attachments are added to the email by MultipartEmail.attach()
        before the rest of the message is constructed during HtmlEmail.send(), creating
        (assuming 1 jpeg attachment):
        multipart/mixed
        image/jpeg
        multipart/alternative
        text/plain
        text/html

        This isn't necessarily incorrect, but would probably be unexpected behavior for
        users because many clients (I'm testing in thunderbird, evolution, and apple
        mail) will display the attachment (or an icon representing the attachment)
        inline at the top of the email.

        Scott

        Show
        Scott added a comment - Hi, This is a step in the right direction...a message with html and plain text will display properly. However I noticed two issues after applying the patch: 1. Embedded images. It appears to construct a message like (assuming 1 embedded jpeg): multipart/alternative text/plain text/html image/jpeg Since they all belong to the alternative part, only the last part (the image) is displayed by the email client, and the rest is ignored. The html and embedded image content need to be in a nested mulipart/related. 2. Attachments. Attachments are added to the email by MultipartEmail.attach() before the rest of the message is constructed during HtmlEmail.send(), creating (assuming 1 jpeg attachment): multipart/mixed image/jpeg multipart/alternative text/plain text/html This isn't necessarily incorrect, but would probably be unexpected behavior for users because many clients (I'm testing in thunderbird, evolution, and apple mail) will display the attachment (or an icon representing the attachment) inline at the top of the email. Scott
        Hide
        Corey Scott added a comment -

        Scott,

        Does this resolve the problem for you?

        -Corey

        Show
        Corey Scott added a comment - Scott, Does this resolve the problem for you? -Corey
        Hide
        Corey Scott added a comment -

        Created an attachment (id=13348)
        proposed changes to fix this bug. Note: test does not validate output (yet), output can be validated manually

        Show
        Corey Scott added a comment - Created an attachment (id=13348) proposed changes to fix this bug. Note: test does not validate output (yet), output can be validated manually
        Hide
        Corey Scott added a comment -

        Yes, still open.

        Here is an except from the commons-dec mailing list (thx scott).

        I am going to try to create a unit test for this and then see when we can take
        it.

        [Except]
        HtmlEmail - still broken. if, for instance, you create an html email
        with 2 embedded jpeg files, you end up with this structure:

        multipart/related
        text/html
        image/jpeg
        image/jpeg
        text/plain

        which will not display properly with many email clients. we really want:

        multipart/alternative
        text/plain
        multipart/related
        text/html
        image/jpeg
        image/jpeg

        or, if there are email attachments, something like:

        multipart/mixed
        multipart/alternative
        text/plain
        multipart/related
        text/html
        image/jpeg
        image/jpeg
        attachment1-content-type
        attachment2-content-type

        [end except]

        Show
        Corey Scott added a comment - Yes, still open. Here is an except from the commons-dec mailing list (thx scott). I am going to try to create a unit test for this and then see when we can take it. [Except] HtmlEmail - still broken. if, for instance, you create an html email with 2 embedded jpeg files, you end up with this structure: multipart/related text/html image/jpeg image/jpeg text/plain which will not display properly with many email clients. we really want: multipart/alternative text/plain multipart/related text/html image/jpeg image/jpeg or, if there are email attachments, something like: multipart/mixed multipart/alternative text/plain multipart/related text/html image/jpeg image/jpeg attachment1-content-type attachment2-content-type [end except]
        Hide
        David Eric Pugh added a comment -

        Hey, is this still open then?

        Show
        David Eric Pugh added a comment - Hey, is this still open then?
        Hide
        Scott added a comment -

        This is likely because the patch on this issue moves the initialization of
        primaryBodyPart from init() to getPrimaryBodyPart(). Because the patch wasn't
        entirely applied (code not removed from init()), this code was duplicated in two
        places.

        Then, over at COM-1681, Corey noticed the duplicate code and removed it from
        getPrimaryBodyPart(), now you are removing it again and primaryBodyPart isn't
        initialized anywhere.

        I'm all of this is clear as mud , but I'd like to keep the initialization of
        primaryBodyPart in getPrimaryBodyPart() so that it only happens if
        primaryBodyPart is actually used. If its helpful, I can wait until all of the
        patches are in and submit a new one for MultipartEmail.

        Show
        Scott added a comment - This is likely because the patch on this issue moves the initialization of primaryBodyPart from init() to getPrimaryBodyPart(). Because the patch wasn't entirely applied (code not removed from init()), this code was duplicated in two places. Then, over at COM-1681 , Corey noticed the duplicate code and removed it from getPrimaryBodyPart(), now you are removing it again and primaryBodyPart isn't initialized anywhere. I'm all of this is clear as mud , but I'd like to keep the initialization of primaryBodyPart in getPrimaryBodyPart() so that it only happens if primaryBodyPart is actually used. If its helpful, I can wait until all of the patches are in and submit a new one for MultipartEmail.
        Hide
        David Eric Pugh added a comment -

        Hi.. I tried removing the lines like you suggested, and that made
        MultipartEmailTest fail!

        I think what happened is that the order of patches got messed up, which is why
        some of them apply and some didn't apply cleanly.

        I am going to try and get the other patches in, and then see about removin these
        lines...

        Show
        David Eric Pugh added a comment - Hi.. I tried removing the lines like you suggested, and that made MultipartEmailTest fail! I think what happened is that the order of patches got messed up, which is why some of them apply and some didn't apply cleanly. I am going to try and get the other patches in, and then see about removin these lines...
        Hide
        Scott added a comment -

        The patch was not completely applied, below is the missing part. I'm also unable
        to get the test cases working, but after removing the four lines below, things
        appear to work fine. Thanks.

        Index: MultiPartEmail.java
        ===================================================================
        RCS file:
        /home/cvspublic/jakarta-commons-sandbox/email/src/java/org/apache/commons/mail/MultiPartEmail.java,v
        retrieving revision 1.7
        diff -u -r1.7 MultiPartEmail.java
        — MultiPartEmail.java 24 Oct 2004 17:27:21 -0000 1.7
        +++ MultiPartEmail.java 24 Oct 2004 22:00:58 -0000
        @@ -122,10 +122,6 @@
        container = new MimeMultipart();
        super.setContent(container);

        • // Add the first body part to the message. The fist body part must be
        • primaryBodyPart = new MimeBodyPart();
        • container.addBodyPart(primaryBodyPart);
          -
          initialized = true;
          }
        Show
        Scott added a comment - The patch was not completely applied, below is the missing part. I'm also unable to get the test cases working, but after removing the four lines below, things appear to work fine. Thanks. Index: MultiPartEmail.java =================================================================== RCS file: /home/cvspublic/jakarta-commons-sandbox/email/src/java/org/apache/commons/mail/MultiPartEmail.java,v retrieving revision 1.7 diff -u -r1.7 MultiPartEmail.java — MultiPartEmail.java 24 Oct 2004 17:27:21 -0000 1.7 +++ MultiPartEmail.java 24 Oct 2004 22:00:58 -0000 @@ -122,10 +122,6 @@ container = new MimeMultipart(); super.setContent(container); // Add the first body part to the message. The fist body part must be primaryBodyPart = new MimeBodyPart(); container.addBodyPart(primaryBodyPart); - initialized = true; }
        Hide
        David Eric Pugh added a comment -

        I have applied the patch by hand, please double check it. Unforutnantly, I
        can't get the unit tests to run.. I'm getting relay errors.. Not sure, but
        dumbster doesn't seem tob e the SMTP server, I think I may have one running
        locally or something...

        Show
        David Eric Pugh added a comment - I have applied the patch by hand, please double check it. Unforutnantly, I can't get the unit tests to run.. I'm getting relay errors.. Not sure, but dumbster doesn't seem tob e the SMTP server, I think I may have one running locally or something...
        Hide
        Corey Scott added a comment -

        please refer to patch submitted for bug: 18971.

        This bug and 18971 are actually caused by the same problem.

        Show
        Corey Scott added a comment - please refer to patch submitted for bug: 18971. This bug and 18971 are actually caused by the same problem.
        Hide
        Scott added a comment -

        Created an attachment (id=12585)
        MultiPartEmail patch

        Show
        Scott added a comment - Created an attachment (id=12585) MultiPartEmail patch

          People

          • Assignee:
            Unassigned
            Reporter:
            Scott
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development