Closed (fixed)
Project:
Signup
Version:
6.x-1.0-rc3
Component:
Email
Priority:
Normal
Category:
Bug report
Assigned:
Reporter:
Created:
10 Jan 2009 at 17:17 UTC
Updated:
6 May 2009 at 12:53 UTC
Jump to comment: Most recent file
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.
| Comment | File | Size | Author |
|---|---|---|---|
| #6 | 356968_signup_email_linefeeds.6.patch | 661 bytes | dww |
| #3 | 356968_signup_email_linefeeds.3.patch | 603 bytes | dww |
Comments
Comment #1
elonen commentedComment #2
elonen commentedOk, 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?
Comment #3
dwwThanks 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
Comment #4
geodaniel commentedThis 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?
Comment #5
dwwNo, 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:
The email is supposed to look like this:
Without this patch, it looks like this (the originally reported bug):
With the patch, it looks like this:
Ugh. :( This requires more investigation. We probably need to fix the underlying problem in core, not here...
Comment #6
dwwBased 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:
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()?
Comment #7
dwwOk, 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.
Comment #8
dwwDiscussed this with deviantintegral in IRC, and he agrees #7 is the right approach. So, I just committed patch #3 to HEAD and D6--1.
Comment #10
capellicThanks for this fix. I have updated the Signup module with the Dev version from April 21 and have verified that it works.