Improvements to plaintext conversion

Dear both,

apologies that it took me a while to get back to you. Thanks so much to both of you for your time and effort.

My main points:

Regarding point 2: I agree that being consistent with the function used for HTML to plaintext conversion is a good thing. Perhaps it might be okay to break BC regarding the plain-text conversion instead of creating a lot of work and complexity that probably nobody is ever going to care about. As you said, I doubt that many people would use Swiftmailer to create plain-text messages anyway, and for those people who do the difference in output between the two functions probably doesn't matter that much. And if you're planning to start a new branch, then I agree it's the perfect timing to clear out deprecated code. If somebody objects, they can always write their own little patch to replace the Html2Text function with another function of their choosing.

Regarding your resolution of point 1: I'm not sure I agree. You're making the assumption that the swiftmailer.html.twig template file contains nothing that would be worth including when converting to plaintext. However, I think it's rather common to have something in there that would be worth keeping. Specifically, it would be great to be able to display any manually populated message parameters in this TWIG template, and ideally this should carry over to the auto-generated "alternative plain text". E.g. I have something like this in that file:

<p>Dear {{ message.params.recipient }},</p>
{{body}}
<p>Kind regards,<br/>{{ message.params.sender }}</p>

So this bit you want to keep in the html2text conversion, whereas you don't want to include things like logos, divs, boxes/frames around the text, preheaders (the message summary bit displayed in the inbox by some modern email apps), etc.

Having given it another thought (and I feel more confident about it now!), I believe the only clean way of handling this is to have two sets (or layers) of templates. One for the inner mail content (that processes the {{ body }} as well as other message params) plus an outer mail styling (that includes the <html> ... </html> tags and would add preheaders, frames, logos, etc.

I know you mentioned that this added layer of complexity is unnecessary because it can already be accomplished by site builders through the TWIG sub-template functionality (e.g. through the {% include ... %} statement). I would normally completely agree with you, but the only reason why this goes wrong has to do with the HTML2Plain conversion. If it weren't for that, I'd agree, but if you want to resolve this in a clean manner, you need another layer of templates.

I don't think adding these templates would necessarily have to break BC. You can just set the new templates to print back the output of the sub-template.

Let me give an example.

Say a module creates this content:

  <p>Your order #12345 has been processed, and we are now shipping your product. Your delivery will arrive on xxx.</p>

Then a sensible TWIG theming would look like this:

Example swiftmailer.twig.html contains:

<p>Dear {{ user_recipient.given_name }},</p>
{{ body }}
<p>If you have any questions, please contact <a href="mailto:{{ customer_service_email }}">customer service</a>.</p>
<p>You can view all details of your order online <a href="{{ link_to_order }}">here</a>.</p>
<p>Kind regards,<br/>Your friendly team at example.com.</p>

Example swiftmailer-plain.twig.html contains:

Dear {{ user_recipient.given_name }},
  
{{ plain }}
  
If you have any questions, please contact customer service here: {{ customer_service_email }}
You can view all details of your order online here [{{ link_to_order }}].

Kind regards,
Your friendly team at example.com.

The {{ plain }} variable is either the plain-text output of the respective module that created that email, or it's a HTML2Text version of {{ body }}. Now because most site builders are not going to care about writing their own swiftmailer-plain template file for this, they'll probably just want to use a default template like this:

Default swiftmailer-plain.twig.html contains:

{{ plaintext_from_template }}

Where {{ plaintext_from_template }} is now the plaintext conversion of the entire swiftmailer.twig.html template output, not just {{ body }}.

On top of that, you allow all the email styling stuff to go into two separate templates:

Example swiftmailer-footer.twig.html contains:

<html>
<!-- add your logo -->
<!-- add preheader -->
<!-- add loads of divs and tables etc to handle your styling -->
{{ body_content }}
<!-- add your HTML signature -->
<p>This email was created by your friendly site example.com. Please reply to this <a href="mailto:help@example.com">address</a>.</p>
</html>

Example swiftmailer-plain-footer.twig.html contains:

{{ plain_content }}
-- 
This email was created by your friendly site example.com
Please reply to this address: help@example.com

And of course these last two new templates should default to the bare minimum in order to ensure full BC.

I know this looks like an awful lot of changes and new templates, but IMHO this is the only way of dealing with this properly. It gives a lot of flexibility to people who want to do email styling correctly, yet those who just want to keep it simple will be able to stick with just using one template.

I know you could probably argue that all of this stuff is irrelevant and that all of the content creation should happen inside the module (perhaps through a separate template in there), and that all that the Swiftmailer module should ever care about is how to turn the body and plaintext of the message into an HTML or plaintext email, yet my personal experience is that it's a hell of a lot easier to add a few message params through a hook and then do all that in the swiftmailer.twig.html template than add another template myself and completely rewrite the content of an email produced by a module inside a hook.

I think email styling really matters to make D8 more competitive, and so I think finding ways to make the styling both flexible but also work out-of-the-box is important.

Let me know what you think! Also I fully understand if you prefer to move this discussion to another issue.

Wow, I'm impressed! Thanks for putting all your hard work into this. And thanks for reading and re-considering my answer so carefully. I also agree it's better to resolve all these things at once because they're intrinsically linked.

Here is my alternative idea. We can add a template variable is_plain which is a boolean. If the site wants to have parts of the template that are different between HTML and plain then they can simply use

{% if is_plain %}

What do you think?

I actually think this is a good idea! I was wondering, if it were possible to combine this with another approach:

Could we perhaps wrap everything that is supposed to be converted from HTML to plain in a <div> ... </div>? And then only feed the content of that div to the html2text function?

An example for a swiftmailer.html.twig file:

<html>
  <!-- add your logo -->
  <!-- add preheader -->
  <!-- add loads of divs and tables etc to handle your styling -->
  <div class="to-be-converted-to-plain">
    <p>Dear {{ user_recipient.given_name }},</p>
    {{ body }}
    {% if is_plain %}
    If you have any questions, please contact customer service: {{ customer_service_email }}
    {% else %}
    <p>If you have any questions, please contact <a href="mailto:{{ customer_service_email }}">customer service</a>.</p>
    {% endif %}
    <p>You can view all details of your order online <a href="{{ link_to_order }}">here</a>.</p>
    <p>Kind regards,<br/>Your friendly team at example.com.</p>
  </div>
  <!-- add your HTML signature -->
  <p>This email was created by your friendly site example.com. Please reply to this <a href="mailto:help@example.com">address</a>.</p>
</html>
{% if is_plain %}
  <div class="to-be-converted-to-plain">
    <!-- add your plaintext signature -->
  </div>
{% endif %}

So then you've got to make sure you pull out all content inside any <div class="to-be-converted-to-plain"> ... </div> environments. I suppose you could use the DOM extension? (Which I believe is a requirement of Drupal 8?). An example:

// create the DOMDocument object, and load HTML from a string
$dochtml = new DOMDocument();
$dochtml->loadHTML($swiftmailer_html_output);

// gets all <div> tags
$divs_all = $dochtml->getElementsByTagName('div');
$divs_process = array();

// traverse the object with all divs
foreach($divs_all as $div) {
  // if the current div has class="to-be-converted-to-plain", adds it in the $divs_process array
  if($div->getAttribute('class') == 'to-be-converted-to-plain') {
    $divs_process[] = $div->nodeValue;
  }
}

$plain = html2text(implode("\n", $divs_process));

Completely untested but just an idea.

What do you think about this?

Sorry, I haven't had a chance to look at your patches. Perhaps you want to have a think about my proposed change and perhaps try to fix that one fail that your last patch produced? I'll see if I can take a look at your patch and perhaps even get it tested in the meantime.

Concerning this:

In the plain text alternative URLs are converted to links then remain in the output twice, something like this

http://example.com [http://example.com]

To be very honest, I wouldn't worry about that too much. I think that's just something somebody would have to live with if they're relying on automated html to plaintext conversion of an email. The only way to stop this from happening is to stop Drupal from converting links of that type to ones enclosed by <a href="...">...</a> tags (which I'd definitely not recommend for the sake of usability for anyone writing HTML emails). An alternative way would be to check if the href bit inside the <a> tag matches the bit enclosed by that tag, and then remove those <a>...</a> tags. You could do that without too much trouble I presume...

Oops, that was an unintended change of the issue status.

Your solution makes complete sense, and will work much better than anything I had suggested!

I installed the patch, and it works like a charm! Amazing, thanks!!!

I'll set it to "Needs review", and if one more person could find the time to confirm this is working, then I guess we can consider this reviewed.

@geek-merlin: Would you mind giving it a quick test so we know it's working and we can get this committed?

Just install the patch and add some {% if is_html %} statements to your swiftmailer TWIG template. If it works for you as well, I think we can consider this done!

The template I'm using looks like this. Perhaps this is a good starting point?

{% if is_html %}
{% include '@my_theme/swiftmailer/swiftmailer-header.html.twig' %}
{% endif %}
{{ body }}
{% if is_html %}
{% include '@my_theme/swiftmailer/swiftmailer-footer.html.twig' %}
{% else %}
{% include '@my_theme/swiftmailer/swiftmailer-plain-footer.html.twig' %}
{% endif %}

Sure, sounds great! Amazing!