HTML in email body is not rendered => one time login link gets messed up

Will need a test case showing the issue.

Also can the issue summary be converted to use the standard template please.

I am getting a similar problem when using symfony_mailer and sending a password reset mail containing the [site:name] token.
The LegacyEmailBuilder from symfony_mailer seems to assume that all core email are plain text.
But user_mail() adds an HTML body to the email (because it uses Token::replace() and not Token::replacePlain().

When using core to send emails you don't notice it because PhpMail::format() then removes the HTML.
But when using symfony_mailer, this HTML is double encoded.

LegacyMailerHelper::formatBody() from symfony_mailer converts the message body to a #processed_text element without specifying a format, so the default format is used.
On most websites, the default form will be plain_text with the FilterHtmlEscape filter enabled.
This means if the website name contains a ', user_mail() will convert the token to a string containing ' and FilterHtmlEscape will double escape it as '.

Is symfony_mailer correct to assume all core emails are plain text? If so, user_mail() should add a plain text body to the message.

@ot1201 instead of using PlainTextOutput::renderFromHtml($token_service->replace()), it is probably cleaner to use $token->replacePlain().

Started work on test coverage. Pushed to the MR.

Test coverage is in place, removing tag.

The test-only test is passing. This suggests that there are no HTML entities contained in the reset password email. The issue description states,

Before the update to Drupal 10..

That implies that the issue is fixed in Drupal 11.x..

It seems there is no problem with the password reset email text in current Drupal with default email setup. The problem is when symfony_mailer module (as mentioned in the issue summary) is installed and configured for email. This needs to be made clear in the 'steps to reproduce'.

@prudloff Can you please specify which version of Drupal and Symfony mailer you were using in #5. If possible can you please provide screenshots to compare with my own.

Setting issue status to 'Needs review'. Unless the test is deemed useful as a guard against future problems with email as described in the issue summary I think the reviewer may consider postponing this issue for further information or closing it since it appears to be already fixed in core.

I am using Drupal 11.0.9 with symfony_mailer 1.5.0.
I don't have the problem with the reset link reported by @ot1201. But I have a double encoding problem when the site name contains a ' character.

I am attaching screenshots of the problem.

@prudloff Ah thanks! In that case I think we can broaden the scope of this issue a bit. The existing issue summary does provide before and after code which shows that a test for HTML entities in the body of the reset email would cover the issue. But the 'trigger' for the issue seems to have changed. It is not triggered by the link in the email body any more.

Now it is more of an edge case. As you mention, it is triggered by the single quotation character. So I think the way ahead is to change the test so it generates a test email with a body containing a quotation mark. Would be worth including a few other similar characters like double quotation marks, things that the exiting fix in the MR will convert from HTML entities back to the plain text versions.

Changing status to 'Needs work'.

Manual testing in D11 without the fix: screenshot of html entities in email →

Change to 'needs review', hope to also get advice on test coverage.

Improved the issue summary, removing the tag. Pipeline is running all green.

Set to needs review. Reviewer may be able to suggest how to make the test-only test fail without the Symfony mailer module installed.

prudloff → changed the visibility of the branch 11.x to hidden.

@oily thanks for the tests!

I don't think it is useful to duplicate the whole test scenario in the FunctionalJavascript test. This one should only test AJAX-specific behavior.
IMHO the current Functional test is enough.

But the test will always pass until tested with the Symfony mailer contributed module installed and configured. It is necessary to find a way of applying a test-only test that fails.

I added a lower level test that can fail even without symfony_mailer.

Seems close! Left some comments on the MR

If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!

I refactored the tests to avoid avoid having methods that are only called once.

Renamed to better correspond to the current issue summary

Feedback appears to be addressed.

A patch working for D10.4 containing only the one line fix. Could be of use to someone.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I think the bot tested the patch instead of the MR.
The MR does not have conflicts.

Committed/pushed to 11.x, thanks!

This doesn't apply to 10.5.x, so moving to 'to be ported'.

catch → closed merge request !10611

10.5 window still open

Clean backport.