Skip to content
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

Support HTML Email for things like sending Articles via email - Add a Set of Standard Filters to use For Filtering Email Messages #1003

Closed
eSilverStrike opened this issue Nov 29, 2019 · 23 comments
Assignees
Labels
Feature Issues that describe new features.
Milestone

Comments

@eSilverStrike
Copy link
Member

A lot of our articles now use HTML (including advance editors). When emails that Geeklog sends includes the article, comment, etc... content it gets converted to plain text and a lot of times it doesn't look very nice.

(Content Submission notifications, Emailing an article to someone, etc...)

Maybe we should include an option to support HTML emails?

If not, we should at least show a message on these email forms indicating that the formatting of the article content (or whatever content) may not look like the article since the HTML is being removed.

@eSilverStrike eSilverStrike added the Feature Issues that describe new features. label Nov 29, 2019
@eSilverStrike eSilverStrike added this to the 2.2.2 milestone Nov 29, 2019
@eSilverStrike
Copy link
Member Author

eSilverStrike commented Nov 30, 2019

Another problem we are having is that the $message sent to com_mail should already be completely filtered as depending on what is being sent (ie straight notification text emails with no user generated content or emails with the actual comment or article text in it) depends on how it is filtered. See #965
For example a comment about actual HTML entities could have html entities codes in the example surrounded by the code html tag. With our current filtering this all gets stripped away. Things will never look perfectly in these emails if the html tags use css classes but we can defiantly make things better than they are.

All emails sent by Geeklog should use a standard filter (either a new class or added to the GL Text class?) with a few different settings depending on what types of emails are being filtered (plain text, html, user content, etc…). Right now for example the comment submission notification and article submission notification have their own code to deal with striping away html tags and prepping the article or comment text for sending via email. This way if the text is being converted we can do things in a standardized way and if the text is being converted from html to plain text we can even do a few more things to make it look better than just stripping all html tags (like adding \n for <br> and </p> tags, etc…)

We can also do this so com_mail is backwards compatible and can filter content still if for example a flag is not set when the function is called then we know that whatever called com_mail didn’t most likely filter the message properly and it can do the filtering we already have in place.

@eSilverStrike eSilverStrike changed the title Support HTML Mail for things like sending Articles via email Support HTML Email for things like sending Articles via email - Add a Set of Standard Filters to use For Filtering Email Messages Nov 30, 2019
@eSilverStrike
Copy link
Member Author

Also wonder if we should setup a standard email template for all Geeklog notifications so all emails are formatted the same way with for example the same "End of Message" message, etc... Plugins should also be able to use these email templates if they want.

@mystralkk
Copy link
Member

According to https://swiftmailer.symfony.com/docs/messages.html , the SwiftMailer Geeklog uses can create a multipart message. How about creating such a message in using COM_mail() function?

@mystralkk
Copy link
Member

After looking at COM_mail() function, I realized that when you send an HTML mail through COM_mail(), it automatically adds a plain-text part internally. That means all we have to do is create an HTML message and send it through COM_mail. Am I missing anything?

@eSilverStrike
Copy link
Member Author

eSilverStrike commented May 4, 2020

From your link it sates:

Setting the Body Content¶
The body of the message – seen when the user opens the message – is specified by calling the setBody() method. If an alternative body is to be included, addPart() can be used.

The body of a message is the main part that is read by the user. Often people want to send a message in HTML format (text/html), other times people want to send in plain text (text/plain), or sometimes people want to send both versions and allow the recipient to choose how they view the message.

As a rule of thumb, if you’re going to send a HTML email, always include a plain-text equivalent of the same content so that users who prefer to read plain text can do so.

If the recipient’s mail client offers preferences for displaying text vs. HTML then the mail client will present that part to the user where available. In other cases the mail client will display the “best” part it can - usually HTML if you’ve included HTML:

So it looks to be either HTML or Text will be displayed, You can send both but only one will be displayed depending on the settings of the email client.

Maybe COM_Mail as you suggested needs to send 2 versions automatically, text and html.
COM_Mail can then take the unfiltered email passed to it and then filter it for text mode, and converting the best it can all the <br>, <P> and any other tags we can to line feeds and then strip the rest of the html. Like wise in html mode it can convert convert linefeeds to <br> and if any HTML is present, filter/clean it up as well so the HTML hopefully displays well in an email. (for example someone emailing an article in Geeklog, that article could contain a lot of css classes that would not be accessible by the email client viewing the email)

If I remember correctly emails created for example by comments has a standard text template for the regular part of the email and then uses some filtering code on the comment (which could be an HTML comment) it self which it then inserts into the email and sends it to COM_Mail. I see this changing where lib-comment would tell COM_Mail to use standard email template A (just an example that maybe all Geeklog Notifications use). Lib-comment would then build part of the body of the email as it needed and pass it to COM_Mail which would then filter the email for text mode and HTML mode. Since Com_mail takes care of all the filtering it would have to handle cases where it could be a mix of text with line feeds and HTML and then convert all of it as needed.

Does this make sense?

@eSilverStrike
Copy link
Member Author

Hmm.. thinking about this again I am thinking that for example lib-comment may have to filter it's own content (using a common class that all of Geeklog can use) before passing it to COM_Mail since for example if it uses a regular text template for the email and then adds in for example a comment that contains HTML, that HTML content will have to be filtered separately first. It can't all be filtered after it has been combined with the text email since I am sure that the html content will have linebreaks in it which we will want to remove first etc...

@mystralkk mystralkk added Feature Issues that describe new features. and removed Feature Issues that describe new features. labels Mar 17, 2022
@mystralkk
Copy link
Member

How about adding a new function like COM_mailHTML($to, $subject, $message, $altMessage = '', $priority = 0, $optional = null, array $attachments = []), since most of the current notification messages seem to be just plain text that don't need API changes?

@eSilverStrike
Copy link
Member Author

eSilverStrike commented Mar 17, 2022

Since we are using Swiftmailer now things have changed slightly since this issue was created.

I've read through the issue several times and will try to summarize how I see things. Hopefully it is correct and how I remember things work :-)

I am not sure what a COM_mailHTML would accomplish that COM_Mail could not since it has an $html flag already and we could pass $message in as an array (unless I am missing something).

Here are the features I see that this issue has been talking about:

Point 1

Add message to email at bottom (could be in every email like how the other bottom text is done in COM_Mail) and "Mail Article to a Friend" page (and any other similar pages if any) Something like:

Note: Content displayed in emails from the website may have formatting changes and therefore look different

It looks like we should add an HTML version for these messages as well or better yet use a template for both plain text and HTML so we are not hard coding it in code. This could be appended to the bottom of the passed email like it is already.

Point 2

Allow Emails that contain website content to be sent in HTML so the email formatting can match the website better. This includes:

  • Mail Article to a Friend
  • Comment Notifications for users that contain comment content
  • anything else???

For this the plain text version would also be included by whatever is creating the email for those people who have their emails to display only plain text. As a backup it could also just be created by the mail class like it is currently

Point 3

Add Filter Class to GLText that COM_Mail can use to filter an HTML Email and convert to HTML if needed. This would include converting linebreaks from plain text to <br> and stripping/cleaning out thml that will cause issues with HTML in emails (like classes, etc). This would only be performed in COM_Mail if a new flag passed to COM_Mail is not set (for example by plugins that have not been updated for this) telling the function that no filtering has been performed. Else if flag is set then function can assume only HTML is being passed and proper filtering has already been done on the HTML.

Point 4

When something like "Mail Article to a Friend" sends an HTML email it should prepare the email content using HTML. Right now it uses language strings and line breaks set in code. We should actually have email templates. One for HTML and one for plain text.

We could then send both to COM_Mail when an HTML email is sent maybe by passing $message as an array in this case?

Final Thoughts

That is sort of how I see it working while still keeping backwards compatibility.

Of course things could change a bit once coding starts.

I could work on this if you do not want to.

Do you see any issues I missed?

@mystralkk
Copy link
Member

mystralkk commented Mar 19, 2022

I am not sure what a COM_mailHTML would accomplish that COM_Mail could not since it has an $html flag already and we could pass $message in as an array (unless I am missing something).

$message is a string, not an array. That's why I suggested introducing COM_mailHTML, which accepts $message and $altMessage respectively. That said, it makes a lot of sense to make templates for plaintext and HTML emails.

@eSilverStrike
Copy link
Member Author

So I started to create email templates for the "Mail an article to a friend" feature. This has a template for html and one for plain text. I will convert Geeklog core emails to use templates as well but most of the rest will just be plain text.

For COM_Mail I am thinking of doing it this way:

  • $message can be passed as array but $html has to be set to true
  • If $message is array then first element is considered the html message, and the second plain text
  • If $message is not an array then it is considered whatever the $html flag is set as. A plain text version of the message will then be created automatically

This way we can use the same function as before and keep it backwards compatible.

HTML to Text converting will be just basic stuff as any class I found requires at least PHP 7.

Ideally the emails should be converted already to HTML and/or plain text already (hence the templates but obviously plugins will not be using them until they get their own email templates added).

I will have to change the mail class as well so it can except both an HTML and Plain text versions of the same email.

@eSilverStrike eSilverStrike self-assigned this Mar 22, 2022
eSilverStrike added a commit that referenced this issue Mar 22, 2022
…r emails

For feature #1003

Added email templates for plain text and HTML for
- Mail Article to a Friend
- Article Submissions
- Link Submissions
@eSilverStrike
Copy link
Member Author

eSilverStrike commented Mar 22, 2022

Updated com_mail and mail class to accept 2 versions of a mail message (plain text and HTML).

Updated email footer to include extra messages about noreply and website content in email along with IP that initiated the sending of the email. These messages are only included when it makes sense (like IP is only included when emails are notifications and to site mail address).

Added functions html2Text and htmlFixURLs to GLText class. Not sure if this is the best spot for these as the GLText class was mainly for dealing with articles but a number of functions in it are used elsewhere in Geeklog. @mystralkk Should we leave them here or should I just add them as regular COM_ functions in lib-common?

Added email templates for plain text and HTML for

  • Mail Article to a Friend
  • Article Submissions
  • Event Submission (calendar)
  • Link Submissions
  • Report Link in Links Plugin
  • COM_emailUserTopics (Daily Digest) - Fixed an improved to use article class and only for active users
  • Email User Form (signature now uses postmode)
  • Comment Notifications (new comment, comment reply, comment abuse report) - Fix for comment reply notifications only sent to active users
  • User Settings page (notifyAdminOfUserUpdate)
  • User emails for New (USER_sendNotification), User Info (USER_createAndSendPassword), Password Request (USER_requestPassword), Invalid Login Notification, and User Email Change
  • User Reminders (batchreminders) and Send User Password
  • Forum Plugin Email Notifications

Still Need to do:

  • None

Not Needed

Other Plugins to do in order of importance

  • Forum (Done!)
  • Downloads
  • Messenger
  • Media Gallery
  • ?

@mystralkk
Copy link
Member

Should we leave them here or should I just add them as regular COM_ functions in lib-common?

I would leave them in GLText to prevent "lib-common.php" from getting larger.

@eSilverStrike
Copy link
Member Author

@mystralkk taking me a little longer to do as I have been finding other bugs that needed fixing.

I also now see that the Description field for Links in the links plugin and Events for the calendar are a little messed up especially if they are submitted by anonymous user as plaintext but then edited by an Admin as it's like plaintext and html gets mixed as linebreaks get converted to breaks while the html still gets used if added by admin. I will have to fix this as well.

eSilverStrike added a commit that referenced this issue Mar 26, 2022
eSilverStrike added a commit that referenced this issue Mar 28, 2022
…ng on page and in emails

For issue #1003

Added remove4byteUtf8Chars to link submission form.

Since no postmode but admin can add HTML emails now try to detect if HTML or plaintext description. Uses same logic here as comments.

Added same logic for when link is displayed by plugin in directory.
eSilverStrike added a commit that referenced this issue Mar 30, 2022
For issue #1003

- Daily Digest now uses article class to retrieve article info to allow images etc to be included (from image tags)
- Only Active users now can get Daily Digest
- Added unsubscribe instructions to top of email
eSilverStrike added a commit that referenced this issue Mar 31, 2022
…HTML Emails

For feature #1003

Updated signature to respect postmode for issue #1119

Added signature divider language variables for both plaintext and HTML.
eSilverStrike added a commit that referenced this issue Apr 5, 2022
…d is also sent as HTML

For feature #1003

Removed unused USER_sendActivationEmail function
eSilverStrike added a commit that referenced this issue Apr 6, 2022
eSilverStrike added a commit that referenced this issue Apr 6, 2022
…ent as HTML

For feature #1003

Also fixed Send User Password logic and User Admin form now requires strong passwords like user settings
@eSilverStrike
Copy link
Member Author

eSilverStrike commented Apr 8, 2022

@mystralkk

I should have figured out this issue before with the plaintext email templates before finishing with Geeklog Core but I didn't so will have to take the time to fix things now.

With the plain text email template I notice I had to double line space everything like

{lang_user_info_msg}


{lang_username}: {username}

{lang_password}: {password}


{lang_password_msg}


{site_name}{!if site_slogan} - {site_slogan}{!endif}

{site_url}

This would return something like:

Admin at Geeklog Site changed the password of your account as follows.  Please save this mail for further reference.

Username: Test
New Password: password1

Since this password was sent by email, it is recommended that you change it immediately. To change your password, log in and then click My Account from the User Functions menu.

Geeklog Site - Another Nifty Geeklog Site
http://www.geeklog.com/

Looking into it now I figured out that any line that ends with a template variable {foo}. A line break gets removed for some reason (hence we adding an extra ones in the template files). If there is a space or text after the template variable or if there is just text on a line then the extra line break is not needed.

So it can get a little confusing for someone making these templates.

Technically it comes down to how the variables are processed:

'<?php echo $this->val_echo(\'' . $val . '\'); ?>

Template variables are swapped in like above and if no text is after it we loose a line break in the text file.

It would be the same if I added straight PHP:

<?php echo "Hello"; ?>

to the template file.

I hope I am explaining this in a way that makes sense...

One work around I found for plaintext emails I could strip out any actually line breaks in the template file right at the beginning of it being processed and instead just rely on any template variables that the template file has. For example {LB} would insert a line break.

So the new email plaintext templates would look like this:

{lang_user_info_msg}{LB}
{LB}
{lang_username}: {username}{LB}
{lang_password}: {password}{LB}
{LB}
{lang_password_msg}
{LB}
{site_name}{!if site_slogan} - {site_slogan}{!endif}{LB}
{site_url}{LB}

Does it make sense to do it this way? This way the user always knows a new line requires the template variable {LB} whether or not spaces or text appear after a template variable.

@eSilverStrike
Copy link
Member Author

@mystralkk

Another issue is that some emails use language variables that have hard coded line breaks like "\n" or includes a URL that is clickable in plaintext but not in an HTML email.

I've been redoing these language variables and breaking them where the linebreaks occur (each variable is a paragraph) and pulling out the URLs (so they can be formatted in the template file).

I think this is the best way to do it even when you consider the other languages.

The only other options I see is I either scan these language variables and convert them on the fly for plaintext or HTML (which can be a headache) or we have 2 versions of the language variable 1 for plaintext and one for html (even if neither has linebreaks or html tags in them).

Thoughts?

@mystralkk
Copy link
Member

@mystralkk

I should have figured out this issue before with the plaintext email templates before finishing with Geeklog Core but I didn't so will have to take the time to fix things now.

With the plain text email template I notice I had to double line space everything like

{lang_user_info_msg}


{lang_username}: {username}

{lang_password}: {password}


{lang_password_msg}


{site_name}{!if site_slogan} - {site_slogan}{!endif}

{site_url}

This would return something like:

Admin at Geeklog Site changed the password of your account as follows.  Please save this mail for further reference.

Username: Test
New Password: password1

Since this password was sent by email, it is recommended that you change it immediately. To change your password, log in and then click My Account from the User Functions menu.

Geeklog Site - Another Nifty Geeklog Site
http://www.geeklog.com/

Looking into it now I figured out that any line that ends with a template variable {foo}. A line break gets removed for some reason (hence we adding an extra ones in the template files). If there is a space or text after the template variable or if there is just text on a line then the extra line break is not needed.

So it can get a little confusing for someone making these templates.

Technically it comes down to how the variables are processed:

'<?php echo $this->val_echo(\'' . $val . '\'); ?>

Template variables are swapped in like above and if no text is after it we loose a line break in the text file.

It would be the same if I added straight PHP:

<?php echo "Hello"; ?>

to the template file.

I hope I am explaining this in a way that makes sense...

One work around I found for plaintext emails I could strip out any actually line breaks in the template file right at the beginning of it being processed and instead just rely on any template variables that the template file has. For example {LB} would insert a line break.

So the new email plaintext templates would look like this:

{lang_user_info_msg}{LB}
{LB}
{lang_username}: {username}{LB}
{lang_password}: {password}{LB}
{LB}
{lang_password_msg}
{LB}
{site_name}{!if site_slogan} - {site_slogan}{!endif}{LB}
{site_url}{LB}

Does it make sense to do it this way? This way the user always knows a new line requires the template variable {LB} whether or not spaces or text appear after a template variable.

I am not sure if I understand all, but all the line ends in template files are kept as they are, aren't they?

@mystralkk
Copy link
Member

@mystralkk

Another issue is that some emails use language variables that have hard coded line breaks like "\n" or includes a URL that is clickable in plaintext but not in an HTML email.

I've been redoing these language variables and breaking them where the linebreaks occur (each variable is a paragraph) and pulling out the URLs (so they can be formatted in the template file).

I think this is the best way to do it even when you consider the other languages.

The only other options I see is I either scan these language variables and convert them on the fly for plaintext or HTML (which can be a headache) or we have 2 versions of the language variable 1 for plaintext and one for html (even if neither has linebreaks or html tags in them).

Thoughts?

I would put placeholders like {url} in language file items and set the placeholders beforehand when I use them in template files. For example:

$someItem = 'My email address is {email}.';
if ($isHtml) {
  $someItem = strtr($someItem, ['{url}' => '<a href="mailto@' . $email . '">' . $email . '</a>']);
} else {
  $someItem = strtr($someItem, ['{url}' => $email]);
}

$template->set_var('someItem', $someItem);

@eSilverStrike
Copy link
Member Author

I am not sure if I understand all, but all the line ends in template files are kept as they are, aren't they?

Yeah it was kind of hard to explain. The answer is No. Any line ends right after a template variable gets removed. If there is a space or character after the template variable then it does not get removed.

I wish the answer was yes but I cannot figure out why the template class does this. The class runs compile_template which then runs functions like replace_vars, replace_lang, etc... which uses preg_replace. It must be preg_replace that not only finds the template variable but for some reason also replaces the line feed if it comes right after the template variable.

Maybe you can look at it and see why this is happening? I can implement my workaround/hack solution with {LB} but I rather it work the right way.

@eSilverStrike
Copy link
Member Author

eSilverStrike commented Apr 9, 2022

I would put placeholders like {url} in language file items and set the placeholders beforehand when I use them in template files.

Yeah that is one of my optional ways to do it. I could sort of like using COM_nl2br or GLText::html2Text for text content depending on post mode.

It would mean that we assume all language variables for emails are in plaintext and doesn't in some cases allow for much in terms of improving the look of HTML emails. I guess Admin can always use the Language Override Manager to change things up a bit.

Hmm another though should I then always strip HTML tags from language variables used by plaintext emails?

I am overthinking this and am making this way more complicated than it should be...


While writing this message I thought things over a bit more and am now circling back to my original plan and the assumption I made initially. Language variables used by emails are assumed to have no formatting what so ever (no linebreaks, no html). This way I can insert the same language variable without any preprocessing in both html and plaintext email templates. This will make it easier moving forward and less rules to remember when creating templates for emails. (else I will have to start adding to versions of language variables one for plaintext and one for HTML to the templates (right now I only have one)

The only downside with this approach I guess is that it means:

  • Current language variables with formatting will have to be broken up
  • Language variables which have emails or links will not be clickable in HTML (unless they get broken up which can then make translating into other language a little more difficult I guess)

Does this sound good?

@mystralkk
Copy link
Member

mystralkk commented Apr 9, 2022

As for line ends getting removed by the Template class, regular expressions with s modifier match line ends.

https://www.php.net/manual/en/reference.pcre.pattern.modifiers.php

Lines 1764, 1945 and 2008 in "template.class.php" use s modifier.

Does this sound good?

It sounds good to me.

eSilverStrike added a commit that referenced this issue Apr 11, 2022
For feature #1003

Need to add space after PHP code if PHP code right at the end of line (with no characters after). If not then a line feed gets removed when template file is evaluate. It is needed for plain text email templates
@eSilverStrike
Copy link
Member Author

@mystralkk Looked into the s modifier and that is only used for finding code blocks or comments so it wasn't the issue.

I did end up finding it though after a lot more testing.

Basically right before writing the cache file or evaluating the template code string

I scan the template code and add a space after the PHP code embedded in it if PHP code right at the end of line (with no characters after).

$templateCode = str_replace(array("?>" . PHP_EOL), '?> ' . PHP_EOL, $templateCode);

If not then a line feed gets removed when template file/code is evaluate.

eSilverStrike added a commit that referenced this issue Apr 11, 2022
For feature #1003

Template class preprocess_fn used to remove any linefeeds from initial template file.
@eSilverStrike
Copy link
Member Author

@mystralkk Ignore the previous comment. After updating all the plaintext templates I found it doesn't work when you have logic processing in the template (like if statements, blocks, etc..) as those line feeds get added to the content as well.

Therefore the less ideal approach of removing all linefeeds first using the template class preprocess function

	// Remove line feeds from plain text templates since required to use {LB} template variable
	$t->preprocess_fn = "CTL_removeLineFeeds"; // Set preprocess_fn before the template file you want to use it on

And then requiring any lines feeds you want in a plaintext email to be set using {LB} template variable will be used.

@eSilverStrike
Copy link
Member Author

Closing this feature as added the feature request to the specific plugin github repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Issues that describe new features.
Projects
None yet
Development

No branches or pull requests

2 participants