Account created on 17 May 2011, over 13 years ago
#

Recent comments

🇳🇱Netherlands P44T

Patches have been updated to address coding standard issue.

🇳🇱Netherlands P44T

@catch, thank your for clarifying. I've added new patch files:

  • 3270647-59-d10.patch: based against 10.0.x, including the block for Drupal 9.5.
  • 3270647-59-d10-only.patch: minimal patch based against 10.0.x, without backward compatiblity for Drupal 9.5.
    • 3270647-59-d9.patch: based against 9.5.x.

      Please let me know if there's anything else I can do!

🇳🇱Netherlands P44T

> I don't see how this is only needed for 9.5.x:

It's not. The fix in the patch is also very relevant for Drupal 10+. The backwards compatibility for PHP < 8 is only relevant for Drupal 9.5 (which is still supported, I think), though. Since Drupal 10 doesnt support PHP 7. That concerns these lines:

    if (version_compare(PHP_VERSION, '8.0.0') < 0) {
      // For headers, PHP's API suggests that we use CRLF normally,
      // but some MTAs incorrectly replace LF with CRLF. See #234403.
      // PHP 8+ requires headers to be separated by CRLF,
      // so we'll replace CRLF by LF only when using PHP < 8. See:
      // - https://bugs.php.net/bug.php?id=81158
      // - https://github.com/php/php-src/commit/6983ae751cd301886c966b84367fc7aaa1273b2d#diff-c6922cd89f6f75912eb377833ca1eddb7dd41de088be821024b8a0e340fed3df
      $mail_headers = str_replace("\r\n", "\n", $mail_headers);
      $mail_subject = str_replace("\r\n", "\n", $mail_subject);
    }

In Drupal 10 and onwards, this particular block can be removed, but the rest of the patch is still needed.

🇳🇱Netherlands P44T

Here's another patch, which addresses the concern of breaking the current behaviour for PHP < 8 raised by @alexpott in #34. Since Drupal 10 doesn't support PHP versions lower than 8, the check can be removed again in the 10.x branch.

I've tested the patch on a Drupal 9.5.8 site with PHP 8.1 on Debian and it works: the webform module is able to send HTML mails again.

It was also brought up that this needs a unit test, but I could use some help here, as I'm not really a PHP developer anymore these days. When I look at \Drupal\Tests\Core\Mail\Plugin\Mail\PhpMailTest.php, I see how the doMail() method was mocked and I could probably verify the arguments passed to the mock somehow, but since the code now checks the constant PHP_VERSION to determine how to behave, this makes unit testing hard or impossible. Is there some way of injecting the PHP version as a dependency of the PhpMail class? It would seem weird to me write a unit test that makes different assertions based on which PHP version the test runs on, right? If anyone could give me some pointers here, I'd be happy to give it another try. (For Drupal 10, a test would be more straighforward.)

Production build 0.71.5 2024