Problem/Motivation

Currently LegacyEmailBuilder only supports limited headers - see code in LegacyMailerHelper.

Proposed resolution

We'd like to support all headers (except for a few that the library adds, and shouldn't be messed with like 'Message-ID'). We can code this very simply in a loop calling Headers::addHeader(), without needing to understand the type of each header.

Also support the plain-text version from $message['plain'].

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#12 symfony_mailer.legacy-headers.3334230-12.patch5.16 KBadamps
#10 symfony_mailer.legacy-headers.3334230-10.patch4.49 KBadamps
#4 symfony_mailer.legacy-headers.3334230-interdiff-3-4.txt1.25 KBadamps
#4 symfony_mailer.legacy-headers.3334230-4.patch2.07 KBadamps
#3 symfony_mailer.legacy-headers.3334230-interdiff-2-3.txt1.26 KBadamps
#3 symfony_mailer.legacy-headers.3334230-3.patch3.06 KBadamps
#2 symfony_mailer.legacy-headers.3334230-2.patch1.94 KBadamps

Comments

AdamPS created an issue. See original summary.

adamps’s picture

adamps commented
Status: Active » Needs review
StatusFileSize
new symfony_mailer.legacy-headers.3334230-2.patch1.94 KB
adamps’s picture

adamps commented
StatusFileSize
new symfony_mailer.legacy-headers.3334230-3.patch3.06 KB
new symfony_mailer.legacy-headers.3334230-interdiff-2-3.txt1.26 KB

Fix coding standards

adamps’s picture

adamps commented
Issue summary: View changes
StatusFileSize
new symfony_mailer.legacy-headers.3334230-4.patch2.07 KB
new symfony_mailer.legacy-headers.3334230-interdiff-3-4.txt1.25 KB

Add support for $message['plain']

  • AdamPS committed d9dfaf57 on 1.x 
    Issue #3334230 by AdamPS: Support all headers in LegacyEmailBuilder
adamps’s picture

adamps commented
Status: Needs review » Fixed
jungle’s picture

jungle
He/Him
Location Chongqing, China
commented
Status: Fixed » Needs review

TypeError: Symfony\Component\Mime\Header\Headers::addMailboxListHeader(): Argument #2 ($addresses) must be of type array, string given, called in /var/www/html/vendor/symfony/mime/Header/Headers.php on line 154 in Symfony\Component\Mime\Header\Headers->addMailboxListHeader() (line 82 of /var/www/html/vendor/symfony/mime/Header/Headers.php)
#0 /var/www/html/vendor/symfony/mime/Header/Headers.php(154): Symfony\Component\Mime\Header\Headers->addMailboxListHeader('Reply-to', 'Example #1 /var/www/html/web/modules/contrib/symfony_mailer/src/LegacyMailerHelper.php(126): Symfony\Component\Mime\Header\Headers->addHeader('Reply-to', 'Example

Got an error with the change here.

The root cause is probably that In the webform module, it's Reply-to, here it's Reply-To. See https://git.drupalcode.org/project/webform/-/blob/6.1.x/webform.module#L462

Not sure if this should be reported to webform.

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Found one occurrence of Reply-to in https://www.rfc-editor.org/rfc/rfc1036#section-3.6

adamps’s picture

adamps commented
Status: Needs review » Needs work

Many thanks for spotting that.

The Symfony Mailer library accepts 'Reply-to', and expects an array (because the header allows multiple values).

Drupal Symfony Mailer looks for 'Reply-To', it doesn't match so puts a single element.

This question/answer indicates that headers are not case-sensitive.

I'll fix it.....

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new symfony_mailer.legacy-headers.3334230-10.patch4.49 KB

When I look closely, Drupal does complex and strange things. The header is called 'Reply-to' in core. The 'To' field is taken from $message['to'] rather than from $message['headers']['to']. The 'From' and 'Reply-To' are stored in 2 places. Latest patch tries to copy what Core does (only applies to LegacyEmailBuilder).

Status: Needs review » Needs work

The last submitted patch, 10: symfony_mailer.legacy-headers.3334230-10.patch, failed testing. View results

adamps’s picture

adamps commented
Status: Needs work » Needs review
StatusFileSize
new symfony_mailer.legacy-headers.3334230-12.patch5.16 KB
adamps’s picture

adamps commented

  • AdamPS committed 60fdaa71 on 1.x 
    Issue #3334230 by AdamPS, jungle: Support all headers in...
adamps’s picture

adamps commented
Status: Needs review » Fixed

Hi @jungle it should be fixed now - please can you try again?

jungle’s picture

jungle
He/Him
Location Chongqing, China
commented

Tested manually with 1.x-dev which includes this commit. It works as expected.

@AdamPS Many thanks!



+++ b/src/LegacyMailerHelper.php
@@ -83,15 +86,22 @@ class LegacyMailerHelper implements LegacyMailerHelperInterface {
+    // Drupal doesn't store the 'To' header in $message['headers'].

I think it's better to open a follow-up to write a test. What if, in the future, Drupal stores 'To' in $message['headers']? What's your opinion?

Status: Fixed » Closed (fixed)

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