-
Notifications
You must be signed in to change notification settings - Fork 119
For HTML messages, add HTML tags to all URIs in message footer/header text #1916
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ssage footer/header text, so links are clickable when text is appended to HTML messages
| $finder->find(\$text, \&CGI::escapeHTML); | ||
|
|
||
| # and finally clean up the displayed link text on mailto links: | ||
| $text =~ s/>mailto:/>/g; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I personally think that this “mailto:” is included in the original text and may not be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only point I want to clarify: the mailto: is first added in an earlier line, and then removed in this line, so that the call to URI::Find::Schemeless will detect and wrap email addresses too. Otherwise, they remain un-clickable, just bare text with no link.
This is a bit of a rough technique, and maybe you don't agree it's worth doing, which is totally fine. But my patch both inserts and then removes the mailto: tag, so it does not need to be in the original text.
Should I resubmit my PR with the changes above but without this mailto: manipulation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I missed the first part. I understand your intention now.
However, I'm concerned that links might be created to things that aren't email addresses occasionally.
I recommend using Sympa::Regexps::email().
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, okay, thank you again.
I have now committed these last changes... but my commit history on this PR is a bit messy now, so please let me know if you'd like me to delete the whole thing and start fresh with a new one. I am still learning the procedures and protocols here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please feel free to submit a new PR until you're satisfied!
I’ll play with this PR this weekend.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @adam12b1,
I realized that a header message
mailto:[email protected]
gives
<a href="mailto:mailto:[email protected]">[email protected]</a>
|
@adam12b1 , are you willing to update your PR? |
|
Oh I'm very sorry @ikedas , I just noticed this one, I should have responded to you right away... I will take care of it on Monday. |
…e, email regex in uri_finder()
Wow, you did a lot of work... and I think it looks great, but I have not yet been able to test, and I have medical leave starting tomorrow (shoulder surgery) so it will be a couple of weeks before I'm really back online. But if you want to go ahead and close out my PR and use your new code in Sympa instead, please do! I appreciate your attention to this issue. |
Use URI::Find::Schemeless to add HTML tags around all URIs in message footer/header text, so links are clickable when text is appended to HTML messages. No effect on plaintext messages.