D7 Backport: Links with "@" are converted into email addresses even if there is no domain suffix present

There is a D7 patch, so moving to Needs Review to see if tests are green.

Thanks for working on this!

I have reviewed this patch and it seems like a straight backport of the D9 issue (https://git.drupalcode.org/project/drupal/-/commit/f7ccf32f21b76e4b8c9c8...). The code is the same and the changes in tests are present as well. Moving to RTBC and adding a tag for final review/commit.

Adding credit from the parent issue.

Thanks everybody!

After applying the patch, the correctness of CJK characters was affected. For example,

"中文abc@example.com中文"

both drupal 7.99 and 10.1.3 was converted to an entire link.

Old content with email addresses like above may be entire paragraph converted into strange characters, for example:

"以電子郵件寄è‡ï¼Œä¿¡ä»¶æ¨™é¡Œã€Œæ‡‰å¾µå…¨è·äººå“¡ï¼ˆæ‚¨çš„姓名）」〠..." this only happens at drupal 7.99.

After applying the patch, the correctness of CJK characters was affected.

I'm having the same problem and need help.

Thanks for reporting this. Can you please provide more information and steps to reproduce?

I launched Drupal 7.98 on simpletest.me and with filtered HTML format tried to save "中文abc@example.com中文". Only the "abc@example.com" part was converted to email address. Then I tried the same on Drupal 7.99 and the result was the same. Text format configuration is default (clean installation).

It would be great if you can provide information which text format are you using, what is the configuration of that text format and what is the real vs expected output on each Drupal 7.98 and 7.99. Thanks!

//Edit: You can also see that drupal.org converted the mail the same way as I have experienced and I think that d.o. is not yet on 7.99.

Hi poker10,

Thanks for your testing and information. I would do more testing to find out the cause (configuration or environment).

Hi poker10,

It turned out that errors occurred with certain Chinese characters: "來信至abc@example.com"
Test with https://simplytest.me/ (Drupal 7.99 + PHP 7.4.28):

As shown below(left), it is completely blank and cannot be displayed.

If you add a half-shaped space, the content can be displayed correctly.

Thanks for providing additional information. I debugged this based on your example and it seems like the problem is that D7 does not have "u" modifier on the pattern.

There is an issue which added this to D8+, but this issue was not backported to D7 yet, see: #1657886: Filter "Convert URLs into links" doesn't support multilingual web addresses

So currently the D10 has this (see the second row):

  $url_pattern = "[\p{L}\p{M}\p{N}._+-]{1,254}@(?:$email_domain)";
  $pattern = "`($url_pattern)`u";

And D7 this:

  $url_pattern = "[\p{L}\p{M}\p{N}._+-]{1,254}@(?:$email_domain)";
  $pattern = "`($url_pattern)`";

When I added this "u" modifier manually to test it, the email address started to display correctly again.

The question is, whether we can finish and backport the #1657886: Filter "Convert URLs into links" doesn't support multilingual web addresses into D7 (so we would have a full fix available), or if it would be easier to just add these modifiers without any side effects, if the rest of the fix from the mentioned issue will be not backported. Currently I do not have a full knowledge what impact could have adding that modifier, so need to discuss this further.

Depending on how we proceed, we might want a new followup issue.

Therefore I've not created an MR, but here's a quick patch with one suggestion for how we could handle this issue.

Rather than making a(nother) potentially impactful change within D7's core filtering, we could add a drupal_alter() which would allow contrib/custom modules to make changes required for e.g. better unicode support.

This patch includes a test based on the examples provided in the previous few comments.

Oops - there's a bit of copypasta and other mess left over in the patch; I'll tidy it up if we decide to proceed in this direction.

Test only patch, and a little cleanup.

Thanks for the idea @mcdruid! I like this idea to add a new hook, instead of trying to push all complex changes from #1657886: Filter "Convert URLs into links" doesn't support multilingual web addresses without proper review / testing (in this D7 phase).

Should we create follow-up issue for this hook implementation?

@charlesc If you would have a minute, can you please confirm, if such an approach will work for you? Thanks!

Yeah a follow-up would be a good idea if we're going to add the hook.