DefaultMailSystem implements MailSystemInterface::format() incorrectly

Here's an updated patch for this issue, as well as a later-introduced bug which broke the format=flowed delsp email formatting.

Would love if someone wants to review this - could be part of a future bugfix release for Drupal 7? :)

-    // Convert any HTML to plain-text.
+    // Convert any HTML to plain-text and wrap the mail body for sending.
     $message['body'] = drupal_html_to_text($message['body']);
-    // Wrap the mail body for sending.
-    $message['body'] = drupal_wrap_mail($message['body']);
     return $message;

Since the call to drupal_wrap_mail() is removed, the comment cannot says and wrap the mail body for sending, as that is done by drupal_wrap_mail().

-  $line = wordwrap($line, 77 - $values['length'], $values['soft'] ? " \n" : "\n");
+  $line = wordwrap($line, 77 - $values['length'], $values['soft'] ? "  \n" : "\n");
   // Break really long words at the maximum width allowed.
   $line = wordwrap($line, 996 - $values['length'], $values['soft'] ? " \n" : "\n", TRUE);

The changed line uses " \n" (two spaces and a new line) instead of " \n" (a space and a new line), but the other call to wordwrap() uses " \n" (a space and a new line). I am not sure this is a typo or a inconsistent change.

Since the call to drupal_wrap_mail() is removed, the comment cannot says and wrap the mail body for sending, as that is done by drupal_wrap_mail().

drupal_html_to_text() wraps the mail for sending. As the function documentation states: "The output will be suitable for use as 'format=flowed; delsp=yes' text (RFC 3676) and can be passed directly to drupal_mail() for sending." Within drupal_html_to_text(), drupal_wrap_mail() will be called on each chunk.

The changed line uses " \n" (two spaces and a new line) instead of " \n" (a space and a new line), but the other call to wordwrap() uses " \n" (a space and a new line). I am not sure this is a typo or a inconsistent change.

The trailing space will be deleted when format=flowed "delsp" email is formatted for display. When wrapping at a space, you want to preserve the space, so you end up with two spaces at the end of the line. For extremely long words, there should be no space when the word is reassembled for display, so you have just one space at the end of the line.

With this change, I am simply reverting 273c78ded2005cac80e2fa965f9131a85be16f0b, so this code goes back to it's original logic in #154218: Add HTML to text convertor for email (and wrap mails properly) (mostly, there was apparently an additional bug fix in there meanwhile)

If this seems too unclear, I could add additional verbiage to the code comments?

I apologize: That comment is correct because drupal_html_to_text() calls drupal_wrap_mail(). This issue should be sufficient to document that calling drupal_wrap_mail() after drupal_html_to_text() is not necessary.

With what explained in comment #14, I think this issue is R&TBC.

Thanks for working on this. I reviewed the patch and compared it with the D10. I am curious, if there is an issue for this for D10? Because it seems like the code in D10 is the same as the D7 code:

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/lib/Drupal/Co...

Do we need to change this in D10 first?

Thanks!

This proposed change was commited in D10 here: #1328696: Problem with _drupal_wrap_mail_line and attachment files and then "reverted" here: #2078917: E-mails contain double spaces in soft-wrapped sentences

Filed #3451611: Fix the format=flowed; delsp=yes encoding of email messages to fix this on Drupal 11

I created a merge request from the last patch, since patches are no longer tested.

This was now fixed in drupal 11 and 10, and merge request is the same as my patch at #11, so setting back to RTBC

Thanks! I compared the D10/11 committed code and this MR and the changes looks the same, see: https://git.drupalcode.org/project/drupal/-/commit/c29768f4068611be2c81a... (from #3451611: Fix the format=flowed; delsp=yes encoding of email messages). So I think this looks good.

The tests are green: https://git.drupalcode.org/issue/drupal-1447236/-/pipelines/228167

Tagging for a final review.

Thanks!

🥳