Problem

  • _drupal_wrap_mail_line() adds two spaces at the end of soft-wrapped lines, causing double spaces to appear at random positions in sentences in some e-mail clients; i.e.:
    $ php -r 'include "mail.inc"; $i = drupal_wrap_mail("_drupal_wrap_mail_line() adds two spaces at the end of soft-wrapped lines, causing double spaces to appear at random positions in sentences in some e-mail clients"); $o = preg_replace("/( +)\\n/", "$1", $i); var_dump($i, $o);'
    string(166) "_drupal_wrap_mail_line() adds two spaces at the end of soft-wrapped lines,
    causing double spaces to appear at random positions in sentences in some
    e-mail clients"
    string(164) "_drupal_wrap_mail_line() adds two spaces at the end of soft-wrapped lines,  causing double spaces to appear at random positions in sentences in some  e-mail clients"

Details

  • RFC 2646 defines the generation of format=flowed as follows:

    A soft line break is a SP CRLF sequence

    If the line ends in one or more spaces, the line is flowed. Otherwise it is fixed. Trailing spaces are part of the line's content, but the CRLF of a soft line break is not.

    If the line ends with one or more space characters, it is flowed.

Proposed solution

  1. Remove the second space. One space + CRLF is sufficient for flowed lines.
Files: 
CommentFileSizeAuthor
#9 drupal.mail-wrap-line.9.patch656 bytessun
PASSED: [[SimpleTest]]: [MySQL] 40,413 pass(es).
[ View ]
#2 drupal8.mail-wrap-line.2.patch1.38 KBsun
PASSED: [[SimpleTest]]: [MySQL] 58,471 pass(es).
[ View ]
drupal8.mail-wrap-line.0.patch653 bytessun
FAILED: [[SimpleTest]]: [MySQL] 58,021 pass(es), 1 fail(s), and 0 exception(s).
[ View ]

Comments

Status:Needs review» Needs work

The last submitted patch, drupal8.mail-wrap-line.0.patch, failed testing.

Assigned:Unassigned» sun
Status:Needs work» Needs review
StatusFileSize
new1.38 KB
PASSED: [[SimpleTest]]: [MySQL] 58,471 pass(es).
[ View ]

Adjusted the unit test accordingly.

Simple enough. This change is complete and ready to be committed.

Will backport to D7 and D6 afterwards.

Still looking good.

Status:Needs review» Reviewed & tested by the community

In line with RFC 2646 so there is no reason that I can think of that this could cause any havoc.

#2: drupal8.mail-wrap-line.2.patch queued for re-testing.

Title:_drupal_wrap_mail_line() causes double spaces in soft-wrapped format=flowed sentencesE-mails contain double spaces in soft-wrapped sentences

People may be scared by the issue title. Let's see whether a simplification helps.


Apparently not. Looks like the RTBC queue is very long.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed 4be4a98 and pushed to 8.x. Thanks!

This was introduced in #154218: Add HTML to text convertor for email (and wrap mails properly) what is interesting is that the second call to wordwrap uses the correct sot-wrapped line break.

  if (!$line_is_mime_header) {
    // Use soft-breaks only for purely quoted or unindented text.
    $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");

There is also the suggestion of an optimisation here as if $line_is_mime_header is FALSE we will be calling wordwrap twice which seems unnecessary.

Status:Patch (to be ported)» Needs review
StatusFileSize
new656 bytes
PASSED: [[SimpleTest]]: [MySQL] 40,413 pass(es).
[ View ]

Backported to D7.

The unit test in D8 does not appear to exist in D7.

Status:Needs review» Reviewed & tested by the community

Identical patch, so I do not realistically see what would be wrong with moving it to RTBC.

RTBC++

The green patch verifies that this doesn't break existing tests, but I also checked that there are no tests that could be amended easily to add test coverage for this.

Issue summary:View changes

Updated issue summary.

Is anything blocking this?

Version:7.x-dev» 6.x-dev
Issue summary:View changes
Status:Reviewed & tested by the community» Patch (to be ported)

Committed to 7.x - thanks! http://drupalcode.org/project/drupal.git/commit/273c78d

Moving to Drupal 6 for possible backport.

BTW, is it proper to use wordwrap(), strlen() and other byte level function for UTF-8 texts?

Version:6.x-dev» 7.x-dev
Status:Patch (to be ported)» Fixed

Thanks! I'm not going to work on a backport to D6 anymore, since my original trigger and use-case has vanished. Thus closing as fixed.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.