Introducing hook_mail_alter() [#71194]

I have looked through the user_mail() function and the code calling it. Found some patterns, so integrated them into user_mail():

X-Mailer: Drupal was in the extra headers of every call, now in user_mail()
The setting of from, reply-to, return-path and errors-to to the same email address was so common, that now it is in user_mail() as a new optional feature

Apart from these consistency fixes, the goal of this patch is to introduce the hook_mail_alter() hook on the heels of hook_form_alter(). Possiblities with this hook include:

HTML-ize your mails, adding company visuals, keeping an alternate text version supposedly
Add additional headers. I need this to help hungarian free hosting users, since some of them need a special X header with an email account key to be able to send emails. With this hook in place, it is easy to provide a simple module to add this key.
Apply preg magic to text mails. Like I would like to modify the contact mails sent by the system, since I found it disturbing that the disclaimer stuff is on top. But adding a common footer to all outgoing mails and such is also a nice option.

For this to make work, I needed to make the headers an associative array first, so that is it not that hard to mangle with them. Then I decided to turn the mail parameters into a form array like structure for consistency with other parts of Drupal. This last step is not neccesery, but we need some structure to pass on to the _mail_alter() hooks, and it was simply logical to borrow a methodology already used elsewhere in Drupal. Especially if we call the hook _mail_alter(), similarity with _form_alter() is helpful.

BTW I also added mail ids to the mails, so it is easy to pick what to alter.

Feedback welcome.

Comment	File	Size	Author
#20	mail_alter_variables.patch.txt	12.35 KB	Gábor Hojtsy
#10	mail_alter_0.patch	11.97 KB	Steven
#10
#9	mail_alter.patch	11.75 KB	Steven
#9
#2	Drupal.mail_alter.2.patch.txt	10.74 KB	Gábor Hojtsy
	Drupal.mail_alter.patch.txt	8.7 KB	Gábor Hojtsy

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

Dries CreditAttribution: Dries commented 28 June 2006 at 21:34

I'm OK with this proposed change.

Comment #2

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 29 June 2006 at 11:39

File	Size
Drupal.mail_alter.2.patch.txt	10.74 KB

Maybe we should couple this change with renaming user_mail() to drupal_mail() and moving it out to common.inc. This is not a user specific mail sending function and never was. I remember searching for this back then when inspecting Drupal functionality and found it misleading that this is in user.module. It is used to send arbitarary mails by the contact module for example, and was designed to send arbitrary mails anyway.

Modified patch attached.

Comment #3

halkeye CreditAttribution: halkeye commented 29 June 2006 at 13:43

Is th array_merge for

+    $headers = array_merge($headers, array('From' => $mail['#from'], 'Reply-to' => $mail['#from'], 'Return-path' => $mail['#from'], 'Errors-to' => $mail['#from']));

really needed? I'm not sure about its efficiancy level, but wouldn't it be better to do:

+    $headers['From'] = $mail['#from'];
+    $headers['Reply-to'] = $mail['#from'];
+    $headers['Return-path'] = $mail['#from'];
+    $headers['Errors-to'] =; $mail['#from'];

Comment #4

moshe weitzman CreditAttribution: moshe weitzman commented 29 June 2006 at 19:57

hmmm. this is one nice way to let user/modules customize outbound mail. but i have always envisioned something different. i'd like to see all our emails be browsable by an admin and customizable through a standardized interface. to do that, we need a hook where modules register their emails and available substitutions. this mechanism can coexist with gabor's patch, but it gets a little hard to know whose customizations will prevail.

in any case, i support this patch for now. i will try to test tonight.

Comment #5

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 30 June 2006 at 06:33

@halkeye: We tend to prefer less repetition if it does not mean performance problems. This hook is not going to get called that much, and using array_merge() this way is a common tactic in Drupal, AFAIS.

@moshe: If I understand correctly, you envision some customization interface like how the user module mails work. User module already has this built in. The problem with it is that it is not language variant, and even with it being i18n-ized, it still keeps to be restricted to the subject and body of the mail. Adding additional headers like I have written above is not possible.

BTW I forget about another use case. I modified contact.module on a site to have a checkbox: "cc this to the site admins" (only visible to those having rights for it). So when I write some moderation comments to a user, I can check this, so all other admins get the notice that something happened. FAPI can add this checkbox, but I can only add the CC: header with hook_mail_alter(), without modifying core. (Now I have a hacked contact.module in place :)

Comment #6

moshe weitzman CreditAttribution: moshe weitzman commented 30 June 2006 at 12:48

Status:

Needs review

» Reviewed & tested by the community

i tested the patch and all works fine. RTBC

@goba - good point about i18n. i think though that this is easily solved by storing a template for each language. also, your cc: example could just as easily be solved with another submit handler on the contact form which reacts to your checkbox.

Comment #7

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 30 June 2006 at 19:47

@Moshe: sure, I can add a submit hook, but I need to send a different mail, and cannot add a CC header to the existing one. The contact submit function takes the user id from the URL, and loads the email address to send the mail to from the db. Adding a CC header is preferable, so the user who got the information on the moderation action can "reply all", and both the moderator and the admin group can review the progress of the situation.

Comment #8

drumm

he/him

NY, US

CreditAttribution: drumm commented 2 July 2006 at 01:26

Status:

Reviewed & tested by the community

» Needs work

Steven spotted a '=> =>' in the patch. Please be sure to test every form.

Comment #9

Steven CreditAttribution: Steven commented 2 July 2006 at 01:32

Status:

Needs work

» Needs review

File	Size
mail_alter.patch	11.75 KB

Here's an untested tweak.

I don't think we should use # marks for the keys. These are used in Form API only to disambiguate child elements from properties. They are unnecessary here.

I also added a call to mime_header_encode() for header values. If we're going to centralize header generation, we might as well centralize mime_header_encode() as well. It is a lossless transformation and protects against e-mail header injection. Think of it as the mysql_escape_string() of MIME.

Finally I spaced out those long drupal_mail() statements into multiple lines.

Comment #10

Steven CreditAttribution: Steven commented 2 July 2006 at 01:36

File	Size
mail_alter_0.patch	11.97 KB

Oops, that wasn't the final one :P.

Comment #11

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 2 July 2006 at 09:20

@Steven: I thought reusing existing metaphors from Drupal is fine. Drupal is not consistent in using objects (eg. node, user), arrays (eg. blocks, menus) or special marked arrays (eg. forms, node/comment links). I thought that some standardisation is underway to use form api like arrays, but using # is admittedly not neccessary in many cases.

I am fine with Steven's changes.

Comment #12

Dries CreditAttribution: Dries commented 2 July 2006 at 10:34

Looks good to me. Haven't tested it yet.

Comment #13

Steven CreditAttribution: Steven commented 2 July 2006 at 14:25

Goba: well I wasn't around when links were changed. I must admit I'm not too happy now that I've taken a closer look at its use of #. It implies a "Forms API"-like flexibility and recursion. But in theme_links(), you can see that it has no depth. The attributes are always on the second level and don't share a namespace with anything else. The # is also unnecessary... I'm going to propose a patch to remove them there at least.

Comment #14

Dries CreditAttribution: Dries commented 2 July 2006 at 20:08

+ drupal_mail(array(
+    'mail-id' => 'contact-page-mail',
+    'to' => $contact->recipients,
+    'subject' => $subject,
+    'body' => $body,
+    'from' => $from,
+  ));

All calls to drupal_mail() appear to use the exact same arguments. Why bother with the array? The arguments you can pass to PHP's mail() is well-defined. The more I look at this, the weirder it looks. Of course, I understand that it makes some underlying code easier, but maybe that shouldn't be exposed to the API? Just thinking up loud and playing the devil's advocate.

Steven: your last patch is not consistent with regard to coding style. Some long $header arrays are on a single line, other long $header arrays are not. Of course, this is a minor issue, but I figured I'd point it out.

Comment #15

Steven CreditAttribution: Steven commented 2 July 2006 at 23:40

Well, the array mechanism provides an easy and transparent way to add more MIME headers to the mix, if you need to.

In my opinion, it is more abstract of the underlying code. PHP's mail() is rather inconsistent. If you want to specify a From address, you need to mess with its $headers argument for example.

Comment #16

Dries CreditAttribution: Dries commented 3 July 2006 at 07:40

Then why don't we make the header-fields an array, and stick with normal non-array fields for the rest? I see no good reason to put 'to', 'subject' and 'body' into an array. I'm not really opposed to it either; it's just a bit awkward.

Comment #17

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 4 July 2006 at 20:32

@Dries, @Steven: Well, $mailid (or $mid?), $to, $subject, $body, $from could be parameters to the drupal_mail() function, like $headers, which should still be an associative array. That would fit an l() like behaviour with $headers playing the role of $attributes.

Comment #18

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 5 July 2006 at 08:16

Addendum: it definitely should not be $mid, since it is not a unique number, as this kind of naming scheme would suggest in Drupal. $mailkey might be better.

Comment #19

Dries CreditAttribution: Dries commented 7 July 2006 at 11:46

Status:

Needs review

» Needs work

Guess this needs more work then. :)

Comment #20

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 9 July 2006 at 20:50

Status:

Needs work

» Needs review

File	Size
mail_alter_variables.patch.txt	12.35 KB

Ok, here is an updated patch, which is synced to HEAD (lots of documentation on the mail function and parameter usage). It uses variables instead of the array. Please review.

Comment #21

Dries CreditAttribution: Dries commented 10 July 2006 at 08:12

Status:

Needs review

» Needs work

Committed to CVS HEAD. Marking this "code needs work" until Drupal's upgrade instructions have been updated.

Comment #22

kbahey CreditAttribution: kbahey commented 10 July 2006 at 14:40

I updated the module migration guide. Someone check it here http://drupal.org/node/64279

Comment #23

Gábor Hojtsy

he/him

Hungarian

Hungary

CreditAttribution: Gábor Hojtsy commented 11 July 2006 at 09:50

Status:

Needs work

» Fixed

Thanks kbahey, I have reviewed and extended your text. Plus added drupal_mail_wrapper() related update instructions, and documented hook_mail_alter() at contrib CVS. So it should also be up on api.drupal.org soon.