The standard e-mail signature separator is this:
--
(hyphen, hyphen, space, newline)

I'd like to include this in my e-mail body, but the drupal_mail() function calls drupal_wrap_mail() and this trims trailing spaces from all lines. This breaks the sig separator, as it ends up being hypen-hyphen-newline.

Should be fix this up as a special case, after all lines have been wrapped, or deal with it differently in the first place?

Is there a better way to append a signature to an e-mail with the standard sig separator line?

CommentFileSizeAuthor
#25 mail-preserve-signature-310315-25.patch1.35 KBsuperspring
#22 mail-preserve-signature-marker-310315-19.patch2.83 KBnaxoc
#19 mail-preserve-signature-marker-310315-19-do-not-test.patch2.83 KBsuperspring
#17 mail-preserve-signature-marker-310315-17.patch3.12 KBsuperspring
#13 mail-preserve-signature-marker-310315-13.patch2.54 KBjustafish
#11 mail-preserve-signature-marker-310315-11.patch2.3 KBjustafish
#10 mail-preserve-signature-marker-310315-10-test-only.patch1.02 KBnaxoc
#10 mail-preserve-signature-marker-310315-10.patch1.85 KBnaxoc
#3 mail-preserve-signature-marker-310315-3.patch870 bytesWaltemath
#4 mail-preserve-signature-marker-310315-3.patch870 bytesWaltemath

Comments

fonant’s picture

fonant commented

I've done a quick workaround using a fix-up at the end of drupal_wrap_mail():

  // Standard e-mail signature separator line.
  $text = preg_replace('/^--\n/m', "-- \n", $text);

but this doesn't feel "correct", as any line that has just two hyphens will be turned into signature separators, even if that wasn't the intention when composing the body text.

It strikes me that it would be nicer if the $message array used by drupal_mail() had a ['signature'] in it, which could then be appended to the end of the message, using the standard separator, within drupal_mail_send(). This would also keep the signature separate from the body of the message within the data array, which may have its uses.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Component: base system » mail system
Waltemath’s picture

Waltemath commented
Version: 7.x-dev » 6.4
StatusFileSize
new mail-preserve-signature-marker-310315-3.patch870 bytes

The attached patch should solve this problem (applies probably to all versions of drupal).

It just preserves the trailing space in case the signature marker is found (i.e., trailing space is only removed, if it is not preceded by "^--"; otherwise there must be at least two spaces).

See http://tools.ietf.org/html/rfc3676#section-4.3

Waltemath’s picture

Waltemath commented
Version: 6.4 » 7.x-dev
Status: Active » Needs review
StatusFileSize
new mail-preserve-signature-marker-310315-4.patch870 bytes

Resubmitted patch.

tyr’s picture

tyr commented
Version: 6.4 » 7.x-dev
Status: Needs review » Reviewed & tested by the community

Patch (#4) works, thanks Waltemath!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

Need tests for this.

Tor Arne Thune’s picture

Tor Arne Thune commented
Version: 7.x-dev » 8.x-dev
Issue tags: +Needs backport to D7

Still a valid issue in Drupal 7.0.

wojtha’s picture

wojtha commented

Similar (but not duplicate) of #1328696: Problem with _drupal_wrap_mail_line and attachment files

naxoc’s picture

naxoc commented
Status: Needs work » Needs review
StatusFileSize
new mail-preserve-signature-marker-310315-10.patch1.85 KB
new mail-preserve-signature-marker-310315-10-test-only.patch1.02 KB

Here are two patches: One with test only and one with fix and test.

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
StatusFileSize
new mail-preserve-signature-marker-310315-11.patch2.3 KB

The tests in #10 do not run for me locally through the UI as DrupalUnitTestCase isn't available (I think)

I've added a test for drupal_wrap_mail removing a whitespace character before a newline and one for an incorrect signature.

Tor Arne Thune’s picture

Tor Arne Thune commented

@justafish: #1563620: All unit tests blow up with a fatal error.
That's why you can't run unit tests locally without failures at the moment.

justafish’s picture

justafish
she/her
Primary language English
Location London, UK
commented
StatusFileSize
new mail-preserve-signature-marker-310315-13.patch2.54 KB

Rerolled against latest HEAD.

blakehall’s picture

blakehall commented
Status: Needs review » Reviewed & tested by the community

Tests passing, and the functionality looks good to me. Marking RTBC

catch’s picture

catch
he/him
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Patch and tests both look good. Committed/pushed to 8.x, moving to 7.x for backport.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Version: 7.x-dev » 8.x-dev
Status: Patch (to be ported) » Needs work

Whoopsie... MailTest was not exactly the right test class ;)

There's a DrupalHtmlToTextTest class somewhere else, and we need to move the new test methods over there :) The MailTest class' main purpose is to serve as an actual Mail system implementation for tests.

superspring’s picture

superspring commented
Status: Needs work » Needs review
StatusFileSize
new mail-preserve-signature-marker-310315-17.patch3.12 KB

Moving tests to DrupalHtmlToTextTest class.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community

Excellent, thanks! :)

superspring’s picture

superspring commented
StatusFileSize
new mail-preserve-signature-marker-310315-19-do-not-test.patch2.83 KB

Same patch as above but for D7.

Marked as do not test until the D8 patch has been commited, then the D7 patch can be considered.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

FWIW, #17 came back green.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Needs review

Committed and pushed #17 to 8.x. Thanks!

Marking down to 7.x for #19.

naxoc’s picture

naxoc commented
StatusFileSize
new mail-preserve-signature-marker-310315-19.patch2.83 KB

I downloaded the patch from #19, removed the "do-not-test", and uploaded it again for the bot.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: -Needs tests +Needs backport to D6
David_Rothstein’s picture

David_Rothstein commented
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/5808159

Moving to Drupal 6 for backport.

superspring’s picture

superspring commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new mail-preserve-signature-310315-25.patch1.35 KB

Attached is a D6 version of the patch.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.