I need to be able to force the module to parse HTML emails, if I send them as they are currently I just see the actual HTML code.

CommentFileSizeAuthor
#4 signup.patch3.99 KByoav_y
#6 signup.patch3.67 KByoav_y
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dww’s picture

Priority: Critical » Normal
Status: Active » Postponed (maintainer needs more info)

this module doesn't parse emails at all, so i have no idea what you're asking for. please provide more info about what you're talking about, or no one is ever going to be able to help make it happen (if it's appropriate for this module at all).

MikeyGYSE’s picture

Status: Postponed (maintainer needs more info) » Active

This module sends emails out, in the form of signup confirmations and reminders, I want those emails to be sent in HTML format - so that they can include a header image and a formatted footer containing our legal information. Is it possible to do that?

dww’s picture

thanks for clarifying what you're asking for. not currently possible, no. however, i have no use for this feature (and generally detest HTML email, anyway) so you'll either have to provide a patch yourself, or hire someone to write it. ;)

good luck,
-derek

yoav_y’s picture

Status: Needs work » Active
FileSize
3.99 KB

The attached patch for 6.x-1.x-RC3 will allow you to use HTML and CSS within emails, but ONLY if you also have the mimemail (http://drupal.org/project/mimemail) module installed. Otherwise it will behave exactly as before.

Mimemail allows users to specify if they prefer plaintext or html e-mails, and this will behave as expected.

Hopefully this resolves your issue...

edit : ignore this patch and use the one further down please

dww’s picture

Title: HTML Emails » Mime / HTML Emails
Version: 5.x-2.x-dev » 6.x-1.x-dev
Status: Active » Needs work

@yoav_y: Thanks for the patch. When you post something that you want the maintainer (me) to review, please set the status to "needs review"...

However, this "needs work" for a few reasons on a quick skim of the patch:

A) Please use a "unified diff" (e.g. "cvs diff -up") when creating a patch, which makes it easier to read and review. See http://drupal.org/patch/create for more info.

B) function mail_token_fixup() needs to start with "signup" in the name to avoid potential namespace conflicts. In other words, it should at least be named function signup_mail_token_fixup(). I haven't looked closely or thought about it at all, so maybe it needs a better name than that, but that's a start in the right direction. ;)

If/when you post a new patch to correct these things, set the status back to "needs review". If it's good and people test it and it works, I'll commit it, otherwise, I'll set it back to "needs work" and explain why.

Cheers,
-Derek

yoav_y’s picture

FileSize
3.67 KB

Ok fixed as per-requested.

the mail_token_fixup was just a convenience function that was initially part of the mail hook but that needed to be called for the mimemail functions too, so I called it that. I renamed it to signup_mail_token_fixup -- although I suppose any of a million other names work too.. I'll be happy to rename more if you like.

unified diff posted (sorry, context diff is the one I'm used to).

switching to needs review

yoav_y’s picture

Status: Active » Needs review

odd it didn't take. Trying needs review again.

Summit’s picture

subscribing, greetings, Martijn

dww’s picture

Status: Needs review » Postponed (maintainer needs more info)

Actually, looking more closely at mimemail.module, I don't see why this patch is needed. Mimemail provides a setting to hijack all email sent by your site and pipe them through mimemail instead of the standard drupal mail delivery function. Why can't you just install, enable, and configure mimemail to handle email for your site and be done with it? Why should signup conditionally test for the existence of mimemail and call mimemail() directly?

Also, note that

} else {

is a code-style bug. It should be:

  }
  else {
yoav_y’s picture

If you try enabling mime mail and tell it to simply trap all system mails and use mimemail for it, you will see that the signup emails get royally borked (the subject line will get split up incorrectly, and several tokens will be misparsed). That's what prompted me to write the little patch....

I apologize for the coding style issues...

Anyways, I'm not really sure what you're asking. If you want me to clean up the code style and resubmit as a patch I'm happy to. I'm fairly sure that just installing mimemail and not including this patch does not work (someone else may want to chime in, but it certainly isnt working for me). If you'd rather not support mime mails (at least via the mimemail module) then there's not much I can offer here. I believe the patch is about as non-invasive as can be (certainly if you don't have mimemail installed nothing changes).

I see that in future versions you're planning on splitting off the email functionality anyways, so perhaps that is the more appropriate place for worrying about mime support (and leaving it broken in the current release). Was only trying to help when I noticed others had also asked about mime support, but it's your module and your call...

dww’s picture

Status: Postponed (maintainer needs more info) » Needs review

I was just trying to understand why this was needed, that's all. Yes, I'd like to overhaul email handling completely, but this is a small change, and if it makes mimemail work, I'd be in favor of committing it before the next release. I'd still like to understand why things don't just work naturally if you enable mimemail. Meanwhile, if anyone else wants to test this, I'd appreciate it. Thanks!

yoav_y’s picture

Well, the biggest issue with the code as written is the line:

$message['body'][] = drupal_html_to_text($body);

Which strips out a bunch of the tags that you would want to keep for mime mail. by calling mime_mail instead of the regular mail_hook we don't strip it (and instead let the mime mail module decide what is safe and what is not safe to pass).

dww’s picture

Status: Needs review » Postponed (maintainer needs more info)

Have you updated your signup code in the last few months? ;) See #356968: Line feeds stripped from confirmation email

Can you try this again with RC4? and/or the latest code in the DRUPAL-6--1 branch?

yoav_y’s picture

trying RC-4 without patch right now..

yoav_y’s picture

Hey cool. Ok with the override turned on, it looks like mime emails are going through correctly (apart from an extra carriage return at the end of the subect line).
So patch no longer required. yay...

dww’s picture

Status: Postponed (maintainer needs more info) » Closed (works as designed)

Excellent. Glad to hear it.

johnnydarkko’s picture

Glad to hear that rc4 works right but it seems like we're having the same behavior with rc6 with emails displaying HTML code. Does 6.x-1.0 include the fix in rc4?

johnnydarkko’s picture

eh... its still showing HTML code when I updated from rc6 to 6.x-1.0. I ended up just turning fckeditor off.