- 🇮🇹Italy apaderno Brescia, 🇮🇹
- // 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 bydrupal_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 line uses" \n"
(a space and a new line). I am not sure this is a typo or a inconsistent change. - Status changed to Needs review
9 months ago 11:49am 23 April 2024 - 🇺🇸United States mfb San Francisco
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?
- Status changed to RTBC
9 months ago 6:00pm 23 April 2024 - 🇮🇹Italy apaderno Brescia, 🇮🇹
I apologize: That comment is correct because
drupal_html_to_text()
callsdrupal_wrap_mail()
. This issue should be sufficient to document that callingdrupal_wrap_mail()
afterdrupal_html_to_text()
is not necessary.With what explained in comment #14, I think this issue is R&TBC.
- Status changed to Needs review
8 months ago 10:50am 31 May 2024 - 🇸🇰Slovakia poker10
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!
- 🇸🇰Slovakia poker10
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 →
- 🇺🇸United States mfb San Francisco
Filed 🐛 Fix the format=flowed; delsp=yes encoding of email messages Active to fix this on Drupal 11
- Merge request !8827Issue #1447236: DefaultMailSystem implements MailSystemInterface::format() incorrectly → (Open) created by apaderno
- 🇮🇹Italy apaderno Brescia, 🇮🇹
I created a merge request from the last patch, since patches are no longer tested.
- Status changed to RTBC
6 months ago 2:22pm 22 July 2024 - 🇺🇸United States mfb San Francisco
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
- 🇸🇰Slovakia poker10
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 🐛 Fix the format=flowed; delsp=yes encoding of email messages Active ). So I think this looks good.
The tests are green: https://git.drupalcode.org/issue/drupal-1447236/-/pipelines/228167
Tagging for a final review.
Automatically closed - issue fixed for 2 weeks with no activity.