I have a case where reply notifications are enabled for a comment that has no email address entered. If one replies to that comment, comment_notify still calls drupal_mail with an empty "to" email address. Calling drupal_mail with an empty "to" email address causes hook_mail_alter to be called with an empty "to" email address, which causes problems with other modules that expect the address to exist.

Patch attached.

CommentFileSizeAuthor
comment_notify_empty_mail.patch587 bytesemilymoi
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

emilymoi’s picture

emilymoi CreditAttribution: emilymoi commented
Status: Active » Needs review

Status: Needs review » Needs work

The last submitted patch, comment_notify_empty_mail.patch, failed testing.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented
Status: Needs work » Needs review

comment_notify_empty_mail.patch queued for re-testing.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

I have a case where reply notifications are enabled for a comment that has no email address entered.

Can you explain how that happened? I just want to know so we can add a comment to explain why this check is necessary.

Status: Needs review » Needs work

The last submitted patch, comment_notify_empty_mail.patch, failed testing.

greggles’s picture

greggles
he/him
Primary language English
Location Denver, Colorado, USA
CreditAttribution: greggles commented

Probably just needs a reroll - sorry about that.

So, I think that this new check should be split into its own if with a clear comment explaining how it might happen. Having all these conditions stacked together makes it hard to document when/why they might happen.

@bfcam - if you can do that it would be great. Otherwise I'll get around to it sometime :)