MailFormatHelper::htmlToText() does not handle the situation where there are newlines in the link text. For example, given the test sequence:
$html_text = "<a href=\"http://some.url\">Link \ntext</a>";
$plain_text = drupal_html_to_text($html_text);
We get:
Link text
when we should get:
Link text [1]
[1] http://some.url
Fix the regex to strip \n from anchor text.
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 309343-17.patch | 855 bytes | quietone |
| #6 | 309343_6.patch | 4.75 KB | mile23 |
| mail.inc_.fixnl_.patch | 606 bytes | Liberation |
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
compare
Comments
Comment #1
cspitzlayThis problem is still around in Drupal 6.15.
I encountered it trying to send multipart/alternative emails using the mimemail module
which uses the function drupal_html_to_text from core internally.
I feel that making that change in core is to be preferred to patching mimemail
which would mean mostly duplicating the functionality of drupal_html_to_text.
Comment #2
lyricnz commentedStill in D8.
Comment #6
mile23drupal_html_to_text() got moved: https://www.drupal.org/node/2309379
Here's a test-only patch which proves that the parser still misinterprets newlines.
The test is moved and renamed from the system module. It was a functional test, now it's a kernel test.
Comment #17
quietone commentedI 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.
Comment #21
pameeela commentedMade 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!
Comment #22
pameeela commentedAdded this as a new test rather than modifying the existing test, and reverted the initial fix. So this is currently a test-only change.
Comment #23
pameeela commentedComment #25
skipper-vp commentedEdited 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:
Comment #26
pameeela commentedComment #27
quietone commentedOnly took a brief look and think that some comments should be added.
Comment #28
pameeela commentedI 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?
Comment #29
pameeela commentedComment #30
pameeela commented@quietone confirmed to add comments to new tests.
Comment #31
smustgrave commentedStarted 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.
Comment #32
alexpottI left a couple of comments on the MR that need addressing.
Comment #34
mdranove commentedUpdated pattern, added test based on feedback. Seeing expected result when tags are placed inside of
<a>. Moving to NR.Comment #35
acbramley commentedChanges look good and comments have been addressed.
Comment #38
catchCommitted/pushed to 11.x, thanks!