Posted by elonen on January 10, 2009 at 5:17pm
5 followers
| Project: | Signup |
| Version: | 6.x-1.0-rc3 |
| Component: | |
| Category: | bug report |
| Priority: | normal |
| Assigned: | dww |
| Status: | closed (fixed) |
Issue Summary
When sending a customized confirmation email, all text appears in one line in the resulting email, regardless of how many line feeds I put to the text area.
Comments
#1
#2
Ok, found the cause. The module calls html_to_text() on the mail templates, which has the surprising side effect of stripping away line feeds - see #298708: drupal_html_to_text() removes line endings.
Not sure if this should actually be considered a bug or not.. If not, how about adding a note to the help text stating that the mail template is supposed to be HTML and you have to put BR and P tags if you want line feeds?
#3
Thanks for the report. This is definitely a bug, and not just one about the help text. The underlying code isn't doing the right thing here at all. Reading those other threads, it seems like the correct solution given the current D6 core API is to remove our own call to drupal_html_to_text(). Please try the attached patch and let me know if it's working as you'd expect.
Thanks!
-Derek
#4
This patch fixed the issue with line endings. That said, if someone has used HTML in their messages, it will now presumably be left as-is, without being converted into formatted plain text. Not sure how big of an issue that is?
#5
No, this doesn't do it, either. It preserves the line endings, but then if you include HTML in your message, that's not converted to text as you'd expect for ASCII email. :(
For example, if the confirmation message for a node is configured like this:
Confirmation:Another line
And another
<ul>
<li>node_title: %node_title</li>
<li>node_start_time: %node_start_time</li>
<li>user_name: %user_name</li>
<li>user_mail: %user_mail</li>
<li>user_signup_info: %user_signup_info</li>
</ul>
<em>emphasized</em>
<strong>strong</strong>
<h2>Header</h2>
The email is supposed to look like this:
Confirmation:
Another line
And another
* node_title: datestamp_tz_site near
* node_start_time: 2009-01-14 14:40 America/New_York
* user_name: root
* user_mail: root@example.com
* user_signup_info: SIGNUP INFORMATION
Name: root
Phone: 123-456-7890
/emphasized/
*strong*
-------- HEADER
--------------------------------------------------------------
Without this patch, it looks like this (the originally reported bug):
Confirmation: Another line And another
* node_title: datestamp_tz_site near
* node_start_time: 2009-01-14 14:40 America/New_York
* user_name: root
* user_mail: root@example.com
* user_signup_info: SIGNUP INFORMATION Name: root Phone: 123-456-7890
/emphasized/ *strong*
-------- HEADER
--------------------------------------------------------------
With the patch, it looks like this:
Confirmation:Another line
And another
<ul>
<li>node_title: datestamp_tz_site near</li>
<li>node_start_time: 2009-01-14 14:40 America/New_York</li>
<li>user_name: root</li>
<li>user_mail: root@example.com</li>
<li>user_signup_info: SIGNUP INFORMATION
Name: root
Phone: 123-456-7890
</li>
</ul>
<em>emphasized</em>
<strong>strong</strong>
<h2>Header</h2>
Ugh. :( This requires more investigation. We probably need to fix the underlying problem in core, not here...
#6
Based on re-reading Heine's comment at #298708-12: drupal_html_to_text() removes line endings, the real question here is if the email configuration settings should be considered:
A) Plain text (sent as-is), in which case we need patch #3
or
B) Rich text (to be converted to HTML via check_markup(), and then back to ASCII via drupal_html_to_text()), which is what this patch does.
With the attached patch (and the previous configuration) the resulting email looks like this:
Confirmation:
Another line
And another
* node_title: datestamp_tz_site near
* node_start_time: 2009-01-14 14:40 America/New_York
* user_name: root
* user_mail: root@example.com [1]
* user_signup_info: SIGNUP INFORMATION
Name: root
Phone: 123-456-7890
/emphasized/
*strong*
-------- HEADER
--------------------------------------------------------------
[1] mailto:root@example.com
I'm honestly not sure if that's an improvement. ;) Probably, no matter how we have the code, some people will be nailed by this and get unexpected results. I'm loath to suggest this, but do we need Yet Another Setting(tm) to control signup's email sending behavior? Is there some other elegant way to hook into this mess so that sites which want HTML and drupal_html_to_text() can do so? Maybe that can all be done via hook_mail_alter()?
#7
Ok, upon further consideration, I think #3 is correct, my concerns in #5 are bogus, and #6A is the right answer. I tried this in D5 signup, and if you put HTML in your email templates, those show up in the resulting emails (just like the last example in #5). So, we weren't handling "smart" conversion from HTML to plain text in the past, and therefore, no one's going to have HTML in their email templates. So, I believe we should just treat all our email templates as plain text (with the token support, obviously), remove the call to drupal_html_to_text(), and if sites want something else, they can monkey around with hook_mail_alter() and/or use their own drupal_mail_wrapper(), etc.
#8
Discussed this with deviantintegral in IRC, and he agrees #7 is the right approach. So, I just committed patch #3 to HEAD and D6--1.
#9
Automatically closed -- issue fixed for 2 weeks with no activity.
#10
Thanks for this fix. I have updated the Signup module with the Dev version from April 21 and have verified that it works.