Fix the format=flowed; delsp=yes encoding of email messages

Created on 31 May 2024, 30 days ago
Updated 28 June 2024, 2 days ago

Problem/Motivation

Drupal documents that email messages are formatted using format=flowed; delsp=yes encoding, and specifies this encoding in the Content-Type header for email messages. However, #2078917: E-mails contain double spaces in soft-wrapped sentences β†’ broke the format=flowed; delsp=yes encoding (I guess because folks didn't test on a client that interprets this encoding, and didn't fully understand how the encoding works).

In addition, the PhpMail class re-wraps email messages by calling MailFormatHelper::htmlToText() and then MailFormatHelper::wrapMail(). First of all, MailFormatHelper::htmlToText() calls MailFormatHelper::wrapMail() on each chunk internally, so this is not necessary.

More importantly, when called on an already-formatted body, MailFormatHelper::wrapMail() breaks the format=flowed; delsp=yes encoding because it converts soft breaks to hard breaks. So, this unnecessary call to MailFormatHelper::wrapMail() needs to be removed.

Steps to reproduce

To reproduce this issue, you will need to read email messages sent by Drupal using any email client that can properly interpret format=flowed; delsp=yes encoding, such as Mutt, Thunderbird, etc.

In these email readers, you will see that soft-wrapped lines have been converted to hard-wrap: Lines that were supposed to be soft-wrapped purely for purposes of sending short lines on the wire, and unwrapped/re-flowed by the email reader, will still be hard-wrapped in the displayed email message. We want these lines to appear "flowed," and only soft-wrapped by the email reader where needed to fit in the user's viewport. See the attached screenshots for what email messages look like in an email reader (e.g. Thunderbird) with and without the patch.

In addition, in any cases where MailFormatHelper::wrapMail() is called only once on the email body (e.g. the SymfonyMailer plugin), email readers supporting format=flowed; delsp=yes will be missing a space when they re-flow wrapped lines, which is also incorrect. A space is deleted when the encoding is interpreted, but no extra space was added during encoding, so you end up with words stuck together.

Proposed resolution

Undo the change from #2078917: E-mails contain double spaces in soft-wrapped sentences β†’ and port πŸ› DefaultMailSystem implements MailSystemInterface::format() incorrectly RTBC to Drupal 11.

Remaining tasks

User interface changes

Vastly improved display of email messages in email clients that interpret format=flowed; delsp=yes encoding. An extra space will be present in soft-wrapped lines if the email reader does not properly interpret this encoding.

Before

After

API changes

Email messages are already specified in the email headers and documented in the code as being format=flowed; delsp=yes so there is no API change here.

Data model changes

None

Release notes snippet

πŸ› Bug report
Status

RTBC

Version

11.0 πŸ”₯

Component
MailΒ  β†’

Last updated 2 days ago

No maintainer
Created by

πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @mfb
  • Pipeline finished with Failed
    30 days ago
    Total: 540s
    #187354
  • Pipeline finished with Success
    30 days ago
    Total: 570s
    #187375
  • Status changed to Needs review 30 days ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco
  • Status changed to Needs work 27 days ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Can we provide examples in the summary for how this is better?

    Thanks

  • Status changed to Needs review 27 days ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    @smustgrave I fleshed out part of the steps to reproduce a bit. If we need more, maybe you can clarify what you would like an example of.

  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Vastly improved display of email messages in email clients

    can we get screenshots of the improvements

  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    Ok I added some screenshots for the default out-of-the-box mailer, with and without the patch applied. (The other bug fixed by this patch is that the body is soft-wrapped but words are stuck together due to missing space; I didn't yet dig into how to reproduce it, but I suspect it could happen with a non-default mailer setup.)

  • Status changed to RTBC 26 days ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Thanks, examples seem good improvements.

  • πŸ‡³πŸ‡ΏNew Zealand quietone New Zealand

    I read the issue summary, comments and the MR. Just about everything here is in order. It would be better if the screenshots were available from the issue summary but there are only 8 comments so far, so they are not hard to find. There are no unanswered questions. The comments in the MR are clear and easy to understand.

    Leaving at RTBC

  • πŸ‡ΊπŸ‡ΈUnited States xjm

    Embedding the screenshots.

  • Status changed to Needs work 13 days ago
  • πŸ‡ΊπŸ‡ΈUnited States xjm

    Made a change to the issue summary... originally it sounded like this needed to be postponed on reverting another issue, but the issue was committed in 2013, so what is actually proposed here is to undo the change from that issue and implement a different fix instead. The situation was further confused by the fact that a better fix was proposed for Drupal 7 but not for Drupal 8, violating the backport policy. :)

  • πŸ‡ΊπŸ‡ΈUnited States xjm

    An important thing to understand for this issue (which I had to re-read the summary to get) is that MailFormatHelper::htmlToText() already calls MailFormatHelper::wrapMail(), so the second call in the PHP mailer was redundant.

  • πŸ‡ΊπŸ‡ΈUnited States xjm

    NW for the above comments/suggestions I posted on the MR, which should get reviewed by @mfb or another previous issue contributor (i.e., credit will not be granted to someone doing a drive-by application of my suggestions). Thanks!

  • πŸ‡ΊπŸ‡ΈUnited States xjm
  • Pipeline finished with Failed
    2 days ago
    Total: 626s
    #210297
  • Status changed to Needs review 2 days ago
  • πŸ‡ΊπŸ‡ΈUnited States mfb San Francisco

    Thanks @xjm, I further clarified the comments.

  • Pipeline finished with Success
    2 days ago
    Total: 501s
    #210307
  • Status changed to RTBC 2 days ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Feedback appears to be reviewed by @mfb mvoing back to RTBC.

Production build 0.69.0 2024