- 🇳🇿New Zealand quietone
I tested this on Drupal 10.1.x. I added the one new test string to \Drupal\Tests\system\Functional\Mail\HtmlToTextTest::testTags and ran the test. The test did fail. So, base on that this is still valid. I also tried the proposed fix in the original patch and the test still failed.
- First commit to issue fork.
- Merge request !7878Fix for #309343 trim newlines from anchor text in MailFormatHelper::htmlToText() → (Closed) created by pameeela
- 🇦🇺Australia pameeela
Made a MR with the updated test and initial fix just to confirm what @quietone said. So this still needs work, but it feels fixable by someone better at regex than me!
- 🇦🇺Australia pameeela
Added this as a new test rather than modifying the existing test, and reverted the initial fix. So this is currently a test-only change.
- First commit to issue fork.
- 🇦🇺Australia skipper-vp
Edited pattern and added replacing of the new lines.
\Drupal\Tests\system\Functional\Mail\HtmlToTextTest passes for me locally.
But on the build this one fails:1) Drupal\Tests\comment\Functional\CommentPreviewTest::testCommentPreview Failed asserting that '3' is an instance of interface "Drupal\Component\Render\MarkupInterface". - Status changed to Needs review
12 months ago 1:54am 3 May 2024 - 🇳🇿New Zealand quietone
Only took a brief look and think that some comments should be added.
- 🇦🇺Australia pameeela
I have added a comment explaining the regex. Uncertain about adding a comment for the test as it's one among many and none of the others have comments (other than @todos that are.... a bit confusing anyway). Should we be adding a comment to new tests as a rule?
- Status changed to RTBC
11 months ago 11:20pm 6 May 2024 - 🇺🇸United States smustgrave
Started taking a look at this several hours ago and got distracted
But issue summary appears fine to me so removing that tag
Ran the test-only feature https://git.drupalcode.org/issue/drupal-309343/-/jobs/1501515 to show coverage
Appears feedback has been addressed and changes look fine to me.
- Status changed to Needs work
11 months ago 8:38am 7 May 2024 - 🇬🇧United Kingdom alexpott 🇪🇺🌍
I left a couple of comments on the MR that need addressing.
- First commit to issue fork.
Updated pattern, added test based on feedback. Seeing expected result when tags are placed inside of . Moving to NR.
- 🇦🇺Australia acbramley
Changes look good and comments have been addressed.
Automatically closed - issue fixed for 2 weeks with no activity.
