Wrong implementation of hook_mail_alter()
mindgame - April 29, 2009 - 21:42
| Project: | Mime Mail |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | needs review |
Description
In mimemail.module, function mimemail(), the hook_mail_alter() is still implemented as
hook_mail_alter(&$mailkey, &$to, &$subject, &$body, &$from, &$headers)However, parameters changed for D6:
hook_mail_alter(&$message)
#1
I agree... I discovered that this is blocker when using Mime Mail with Subscriptions Mail module... You will get a "white screen of death"...
I'm not very familiar with Drupal mail system... Maybe swapping module alteration with mail preparation can solve the issue...
Now...
foreach (module_implements('mail_alter') as $module) {
$function = $module .'_mail_alter';
$function($mailkey, $recipient, $subject, $body, $sender, $headers);
}
$engine_prepare = variable_get('mimemail_engine', 'mimemail') .'_prepare';
if (function_exists($engine_prepare)) {
$message = $engine_prepare($sender, $recipient, $subject, $body, $plaintext, $headers, $text, $attachments, $mailkey);
}
else {
$message = mimemail_prepare($sender, $recipient, $subject, $body, $plaintext, $headers, $text, $attachments, $mailkey);
}
and (maybe) we can use it like...
$engine_prepare = variable_get('mimemail_engine', 'mimemail') .'_prepare';
if (function_exists($engine_prepare)) {
$message = $engine_prepare($sender, $recipient, $subject, $body, $plaintext, $headers, $text, $attachments, $mailkey);
}
else {
$message = mimemail_prepare($sender, $recipient, $subject, $body, $plaintext, $headers, $text, $attachments, $mailkey);
}
foreach (module_implements('mail_alter') as $module) {
$function = $module .'_mail_alter';
$function($message);
}
#2
Here's an attempted fix which I've not yet fully tested. But without it, mimemail horribly broke my site :)
#3
Here is an interesting comment about which hooks mimemail should implement and which ones it should not.
I dont understand all of it, but it looks like the suggestions made would make mimemail work together with other mailing modules much better.
http://drupal.org/node/132903#comment-1610070
#4
It does sound like there are some architectural issues to work on. In the meantime hopefully we could at least fix this really critical bug (for those of us who need to alter our mail :)
#5
Workaround is to test the number of args passed in to your module's mail alter function.
/*** Implementation of hook_mail_alter()
*/
function my_module_mail_alter(&$message, &$to = '', &$subject = '', &$body = '', &$from = '', &$headers = array()) {
if(count(func_get_args()) == 1) {
$message['to'] = 'to@example.com';
$message['from'] = 'from@example.com';
}
else {
$to = 'to@example.com';
$from = 'from@example.com';
}
}
#6
See also #443964: media=<print|screen|all> selector stripped from mime_mail embedded css
#7
I got to this issue by searching the queue. Not really because of the same problem explained in this issue but it's definitely related. I agree with some of the comments on the node posted by batje in #3. Basically I have followed this issue historically. That code, the mail_alter hook was added into the module in #263142: support hook_mail_alter for better integration with other modules to overcome missuse of mimemail() function in favor drupal_mail() directly. Basically, what was happening is that simplenews, for instance, was doing something like:
<?phpif (module_exists(mimemail)) {
mimemail(...);
} else {
drupal_mail(...);
}
?>
Simplenews, at least, has improved the way it sends mail, and now, although calling mimemail directly, it also calls drupal_mail() so that function can render the mail. drupal_mail() has an argument that tells not to actually send the mail, but just process it (including mail_alter). So basically, what you are getting with this code (fixed or not) is that mail_alter is mainly executed twice (once from drupal_mail() and once from mimemail(), which I think is bad behavior). I am working with a site that uses a combination of simplenews, simplenews_template and mimemail to send fancy html newsletter and I just found the calling of mail_alter twice. It's not really something that for now will break my site, but I just think it's completely unnecessary to call mail_alter twice.
Attached is a patch that strips the whole code alltogether, besides, it makes mimemail at least one tiny step to be easier to maintain.
Hopefully, if you agree, you could try out the patch in your configuration and see if that actually works, and if not, why and what modules is actually not working, but hook_mail_alter() should be run by drupal_mail() only, and not by any other module, unless it works completely by itself, which is not currently the case of mimemail (as mimemail actually plugis into drupal_mail()).