The example code in mail.inc suggests that a mail template should set the body as a string rather than an array.

$message['body'] = t("Dear !username\n\nThere is new content available on the site.", $variables, $language->language);

This is bad practice, and should be changed to make the $message['body'] in to an array so that other modules can attach additional content to the outbound emails.

$message['body'][] = t("Dear !username\n\nThere is new content available on the site.", $variables, $language->language);

Modules such as mollom and webform (from a quick check) already expect $messages['body'] to be an array. Other modules such as authorcontact have followed the above example code from the api.drupal.org site and thus cause mollom.module to crash due to $message['body'] not being an array.

A spot check on other core modules indicates this rule of using an array is already obeyed, so I presume it is just a documentation fix required in mail.inc

CommentFileSizeAuthor
#9 durpal-mail-D6.patch788 bytesandypost
#2 drupal_455172_2.patch806 bytesRoboPhred
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

brianV’s picture

Issue tags: +Novice

Setting as a novice issue

RoboPhred’s picture

Assigned: Unassigned » RoboPhred
FileSize
806 bytes

Jumping into core.

The documentation already mentions that the message body should be an array...

Any module can modify the composed e-mail message array using hook_mail_alter().

...so it looks that only the example code needs to be changed.

RoboPhred’s picture

Status: Active » Needs review
brianV’s picture

Status: Needs review » Reviewed & tested by the community

Looks good!

Dries’s picture

Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks!

andypost’s picture

Status: Fixed » Needs work
Issue tags: +Drupal 6 porting, +drupal_mail

Take a closer look to drupal_mail()

  // Concatenate and wrap the e-mail body.
  $message['body'] = is_array($message['body']) ? drupal_wrap_mail(implode("\n\n", $message['body'])) : drupal_wrap_mail($message['body']);

body can be an array and a plain text - it's not generalized, suppose bug?

So this docs are not valid!

RoboPhred’s picture

Is there any case where a module should explicitly deny other modules from adding content to the message? I feel that the documentation is correct and that only an array should be accepted so that modules will always play nice with each other.

It's a very minor change anyway, the coder module could easily take care of it when converting 6-7.

I will roll a patch for whatever is decided, I'm just the novice here.

RoboPhred’s picture

API site also needs to be updated.
http://api.drupal.org/api/function/drupal_mail/6

andypost’s picture

Status: Needs work » Needs review
FileSize
788 bytes

So here patch to change this for D6 at api

Status: Needs review » Needs work

The last submitted patch failed testing.

andypost’s picture

Version: 7.x-dev » 6.x-dev

forget to change version

andypost’s picture

Just a tag

RoboPhred’s picture

Assigned: RoboPhred » Unassigned
andypost’s picture

Status: Needs work » Needs review
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

The patch on #9 above applies cleanly to the D6 branch, and in my opinion should be applied. It conforms to the change already applied to D7.

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

Committed this doc change to Drupal 6. However I believe that since the core API allows either string or array, modules should be able to work with both. Those altering email can still alter the string to be an array and go on with that. At least in Drupal 6, it is not an option to convert to supporting arrays only.

Dave Reid’s picture

Dave Reid’s picture

Version: 6.x-dev » 7.x-dev
Status: Fixed » Needs work

We still need to fix drupal_mail() in 7.x:
$message['body'] = is_array($message['body']) ? drupal_wrap_mail(implode("\n\n", $message['body'])) : drupal_wrap_mail($message['body']);

Status: Needs work » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Dave Reid’s picture

Component: documentation » base system
Assigned: Unassigned » Dave Reid
Status: Closed (fixed) » Needs work

Re-opening to fix drupal_mail() to only accept arrays.

jhodgdon’s picture

Status: Needs work » Fixed

The original issue filed here has been fixed in both Drupal 6 and Drupal 7.

If you'd like to change the API to only allow arrays, can you file a separate issue against Drupal 7?

Status: Fixed » Closed (fixed)
Issue tags: -Needs backport to D6, -Novice, -drupal_mail

Automatically closed -- issue fixed for 2 weeks with no activity.