Page MenuHomePhabricator

HTML bodies are not passed to all emails
Closed, ResolvedPublic

Description

Currently, when something builds a PhabricatorMetaMTAMail, it must call both setBody() and setHTMLBody() to separately pass the body and HTML body. However, only some callsites do this. This causes some issues:

  • Some mail doesn't send with an HTML body, even if the user has requested HTML mail and we have an HTML body available.
  • After D10857, some mail generates with an empty HTML body, then has a To/CC footer appended. This can result in users receiving only the To/Cc part mail. A specific example of this is the mail generated by PhabricatorRepositoryPushMailWorker. (This only affects some mail, and only users who have opted into HTML mail, so it's not quite as broken as the screenshot looks.)

Screen_Shot_2014-11-17_at_1.29.46_PM.png (482×795 px, 28 KB)

To remedy this, we should:

  • Add some setMailBody() method which accepts a PhabricatorMetaMTAMailBody object and calls setBody() + setHTMLBody() from it.
  • Maybe rename setBody() + setHTMLBody() to setRawBody() + setRawHTMLBody().
  • When adding the To/Cc HTML, maybe make sure the mail already has an HTML body, so we never end up with a body that only has to/cc.
  • Go through all new PhabricatorMetaMTAMail() callsites and make sure they set HTML bodies if one is available.

Event Timeline

epriestley raised the priority of this task from to Normal.
epriestley updated the task description. (Show Details)
epriestley added projects: Support Impact, Mail.
epriestley added subscribers: epriestley, chad, btrahan.

When adding the To/Cc HTML, maybe make sure the mail already has an HTML body, so we never end up with a body that only has to/cc.

I'm going to throw up a quick patch for this since I have all the files open; the rest of this is "whenever we get to it".

epriestley lowered the priority of this task from Normal to Low.Nov 17 2014, 9:38 PM
epriestley removed a project: Support Impact.

...and not really important.

eadler moved this task from Backlog to Nits on the llvm board.

I've marked D19029 as resolving this since it fixes the last one of these that I'm aware of. HTML mail has been the default for a long time now so this presumably isn't really causing problems even if there are other cases.