I misunderstand this task #212224: Mime Mail module: HTML emails and Plain text emails
I think it started to move in the different direction, that's why I decide to create new issue.

Mail editor should provide two fieldsets. In the first one we would create and edit the HTML message. In the second part, we could edit the plaintext version.
The result should be a multipart mail message that would be handled automatically by the mail client.

CommentFileSizeAuthor
#9 interdiff-2076827-4-9-do-not-test.diff1.31 KBdas-peter
#9 mail_edit-add-plaintext-support-2076827-9.patch3.28 KBdas-peter
#4 mail_edit-2076827-4.patch3.53 KBmrded
#4 interdiff.txt449 bytesmrded
#2 mail_edit-2076827-2.patch3.53 KBmrded
#1 mail_edit-2076827-1.patch2.81 KBmrded

Comments

mrded’s picture

mrded commented
Status: Active » Needs review
StatusFileSize
new mail_edit-2076827-1.patch2.81 KB

Take a look my patch.

To send plaintext with HTML message together you should enable mimemail module.

mrded’s picture

mrded commented
StatusFileSize
new mail_edit-2076827-2.patch3.53 KB

Sorry, I completely forgot about tokens. This patch is with tokens replacements.

salvis’s picture

salvis
he/his
commented

I agree that we should aim to provide a text/plain alternative. However, this is a breaking change for Subscriptions and possibly other client modules that extend Mail Editor, which probably means we have to go to 7.x-2.x and double our maintenance effort.

Is there existing support for sending multipart/alternative emails? Have you actually tried this successfully? In what set-up?

Some comments on the patch:

  1. +++ b/mail_edit.admin.inc
@@ -459,6 +459,22 @@ function mail_edit_template_subform($template) {
+        'visible' => array(':input[name="rewrite_plaintext"]' => array("checked" => TRUE)),

    What is "':input"? I've never seen that before. 

  2. +++ b/mail_edit.install
@@ -146,3 +155,11 @@ function mail_edit_update_7001() {
+function mail_edit_update_7002() {
+  db_add_field('mail_edit', 'rewrite_plaintext', array('type' => 'int', 'not null' => TRUE, 'default' => 0));

    This would be 7101, or even 7201, if we go to 7.x-2.x.

mrded’s picture

mrded commented
StatusFileSize
new interdiff.txt449 bytes
new mail_edit-2076827-4.patch3.53 KB

Is there existing support for sending multipart/alternative emails? Have you actually tried this successfully? In what set-up?

We already use this in both of our projects, it works fine.

What is "':input"? I've never seen that before.

It because it's new setting for plaintext.

salvis’s picture

salvis
he/his
commented

It because it's new setting for plaintext.

Yeah, but where is the piece of code that interprets that?

mrded’s picture

mrded commented

Yeah, but where is the piece of code that interprets that?

+++ b/mail_edit.admin.inc
@@ -459,6 +459,22 @@ function mail_edit_template_subform($template) {
+    $form['rewrite_plaintext'] = array(
+      '#type' =>'checkbox',
+      '#title' => t('Rewrite plaintext'),
+      '#default_value' => isset($template['rewrite_plaintext']) ? $template['rewrite_plaintext'] : '',
+    );
salvis’s picture

salvis
he/his
commented

I'm still not seeing the light...

        'visible' => array(':input[name="rewrite_plaintext"]' => array("checked" => TRUE)),

This creates an array with one element, whose index is ':input[name="rewrite_plaintext"]' and whose value is another array.

That's a very odd index value. Index value are typically either numbers or strings of letters. They don't usually contain colons, brackets, and double-quotes. I have to assume that this index value is somehow evaluated somewhere.

Please explain where, how, and what the result is.

mrded’s picture

mrded commented 
$form['rewrite_plaintext'] = array(
  '#type' =>'checkbox',
  '#title' => t('Rewrite plaintext'),
  '#default_value' => isset($template['rewrite_plaintext']) ? $template['rewrite_plaintext'] : '',
);

Will render as html bellow:

<div>
 <input type="checkbox" name="rewrite_plaintext" value="1" checked="checked">
 <label for="edit-rewrite-plaintext">Rewrite plaintext</label>
</div>

So for plaintext field, I basically check input tag with name="rewrite_plaintext" to the state of being checked.

$form['plaintext'] = array(
  //..
  '#states' => array(
    'visible' => array(':input[name="rewrite_plaintext"]' => array("checked" => TRUE)),
  ),
);

You can read more about Form API #states here.

mrded’s picture

mrded commented
Issue summary: View changes

Update

das-peter’s picture

das-peter commented
Issue summary: View changes
Status: Needs review » Reviewed & tested by the community
Related issues: +#212224: Mime Mail module: HTML emails and Plain text emails, +#2033637: Add Mail Editor support
StatusFileSize
new mail_edit-add-plaintext-support-2076827-9.patch3.28 KB
new interdiff-2076827-4-9-do-not-test.diff1.31 KB

I just gave this a shot and it works just fine (Also tested with the patches for rules and mimemail rules integration).
The review of the patch also looks good, I've just some more or less cosmetic changes:

  • Missing space in mail_edit_template_subform()
  • Enumeration of the update hook is somehow strange why 7101 instead 7002?
  • Changed update hook to reuse the defined schema and only run if the fields are missing.

So I'm bold enough to set this to RTBC since the only concerns that savlis had were related to a states handling of fapi which shouldn't be a problem at all. Details about drupals states feature can be found here too: https://api.drupal.org/api/drupal/includes!common.inc/function/drupal_pr...

mrded’s picture

mrded commented
Status: Reviewed & tested by the community » Needs work

Patch is not ready yet, because it doesn't provide plaintext field for comments.

Steps to Reproduce:

  • Install Subscriptions module;
  • Install forum module;
  • Go to '/admin/config/system/mail-edit/subscriptions_mail_node-type-forum/en' page;

You should see plaintext for body and comments. Currently it's just for body.

salvis’s picture

salvis
he/his
commented

So I'm bold enough to set this to RTBC since the only concerns that savlis had were related to a states handling of fapi which shouldn't be a problem at all.

That's where I got stuck. It didn't mean that I was through. :)
[Thank you for the explanation in #8, mrded! I hadn't encountered #states before.]

Yes, for Subscriptions we need both fields for both body and comments. The challenge here is that the comment field is supplied by Subscriptions, which means Subscriptions needs to add and manage the plain-text comment field, too. I've created #2147671: Add a plain-text field into Mail Editor for this.

So, here goes the review:

  1. 70xx update numbers are reserved for updating from D6 to D7. Updates within 7.x-7.1 should use 71xx — 7101 is correct.
  2. Is $message['params']['plaintext'] Mime Mail-specific, or will it work with HTML Mail / Mail MIME as well? I don't want to leave the latter behind.
  3. What if the site sends plain text only? The 'Rewrite plaintext' checkbox is superfluous and confusing in that case. We probably cannot tell in a generic way, so I guess we need an option on the admin/config/system/mail-edit page. If this is on, we'll take the opportunity to tell the user:

    Note: Sending HTML mail requires installation of a third-party HTML mail system, such as HTML Mail or Mail Mime.

  4. "Rewrite plaintext" and "Plaintext" are not sufficient. We need something like "Use a separate plain-text template when sending HTML mail" and "Plain-text body".
  5. I'd prefer 'body_plaintext' and 'use_plaintext' as column names.
  6. If an HTML mail system is installed, is this an all or nothing choice, i.e. will all outgoing mail be in HTML? Mime Mail says "Allows you to theme messages with a specific mailkey" — does that mean we can choose to send some as plain-text-only? How can Mail Editor know? How can Subscriptions know? Or can Mail Editor control this?

This will need testing with HTML Mail / Mail MIME as well before it can be RTBC.