diff --git a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php --- a/src/applications/audit/editor/PhabricatorAuditCommentEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditCommentEditor.php @@ -420,7 +420,7 @@ $threading = self::getMailThreading($repository, $commit); list($thread_id, $thread_topic) = $threading; - $body = $this->renderMailBody( + $body = $this->buildMailBody( $comment, "{$name}: {$summary}", $handle, @@ -480,7 +480,8 @@ ->setRelatedPHID($commit->getPHID()) ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setIsBulk(true) - ->setBody($body); + ->setBody($body->render()) + ->setHTMLBody($body->renderHTML()); $mails = $reply_handler->multiplexMail( $template, @@ -509,7 +510,7 @@ return $reply_handler; } - private function renderMailBody( + private function buildMailBody( PhabricatorAuditComment $comment, $cname, PhabricatorObjectHandle $handle, @@ -562,7 +563,7 @@ PhabricatorEnv::getProductionURI($handle->getURI())); $body->addReplySection($reply_handler->getReplyHandlerInstructions()); - return $body->render(); + return $body; } } diff --git a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php --- a/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php +++ b/src/applications/config/option/PhabricatorMetaMTAConfigOptions.php @@ -331,7 +331,21 @@ 'in bytes.')) ->setSummary(pht('Global cap for size of generated emails (bytes).')) ->addExample(524288, pht('Truncate at 512KB')) - ->addExample(1048576, pht('Truncate at 1MB')) + ->addExample(1048576, pht('Truncate at 1MB')), + $this->newOption('metamta.html-emails', 'bool', false) + ->setBoolOptions( + array( + pht('Enable HTML emails'), + pht('Plaintext emails only'), + )) + ->setSummary( + pht( + 'Phabricator can send prettier emails when HTML is enabled '. + 'reply addresses.')) + ->setDescription( + pht('HTML emails offer prettier emails (colorized diffs, links etc)'. + ', however it may reduce the reliability of reply-by-email')), + ); } diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -1164,13 +1164,16 @@ $config_attach = PhabricatorEnv::getEnvConfig($config_key_attach); if ($config_inline || $config_attach) { - $patch = $this->renderPatchForMail($diff); + list($patch, $html_patch) = $this->renderPatchForMail($diff); $lines = count(phutil_split_lines($patch)); if ($config_inline && ($lines <= $config_inline)) { - $body->addTextSection( + $body->addPlaintextSection( pht('CHANGE DETAILS'), $patch); + $body->addHTMLSection( + pht('CHANGE DETAILS'), + $html_patch); } if ($config_attach) { @@ -1697,11 +1700,38 @@ private function renderPatchForMail(DifferentialDiff $diff) { $format = PhabricatorEnv::getEnvConfig('metamta.differential.patch-format'); - return id(new DifferentialRawDiffRenderer()) + $patch = id(new DifferentialRawDiffRenderer()) ->setViewer($this->getActor()) ->setFormat($format) ->setChangesets($diff->getChangesets()) ->buildPatch(); + + // some basic formatting (switch to a PhutilSyntaxHighlighterEngine?) + $patch_html_lines = array(); + foreach (phutil_split_lines($patch) as $line) { + $len = strlen($line); + $color = null; + if ($len >= 1 && $line[0] == '+') { + $color = 'green'; + } else if ($len >= 1 && $line[0] == '-') { + $color = 'red'; + } else if ($len >= 2 && substr($line, 0, 2) == '@@') { + $color = 'purple'; + } + + $attrib = array(); + + if ($color) { + $attrib['style'] = "color:{$color};"; + } + + $patch_html_lines[] = phutil_tag('span', $attrib, $line); + } + + $html_patch = "
".
+      implode('', $patch_html_lines).'
'; + + return array($patch, $html_patch); } } diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationAdapter.php @@ -8,9 +8,9 @@ abstract public function addCCs(array $emails); abstract public function addAttachment($data, $filename, $mimetype); abstract public function addHeader($header_name, $header_value); - abstract public function setBody($body); + abstract public function setBody($plaintext_body); + abstract public function setHTMLBody($html_body); abstract public function setSubject($subject); - abstract public function setIsHTML($is_html); /** * Some mailers, notably Amazon SES, do not support us setting a specific diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationMailgunAdapter.php @@ -57,13 +57,13 @@ return $this; } - public function setSubject($subject) { - $this->params['subject'] = $subject; + public function setHTMLBody($html_body) { + $this->params['html-body'] = $html_body; return $this; } - public function setIsHTML($is_html) { - $this->params['is-html'] = $is_html; + public function setSubject($subject) { + $this->params['subject'] = $subject; return $this; } @@ -78,11 +78,10 @@ $params['to'] = implode(', ', idx($this->params, 'tos', array())); $params['subject'] = idx($this->params, 'subject'); + $params['text'] = idx($this->params, 'body'); - if (idx($this->params, 'is-html')) { - $params['html'] = idx($this->params, 'body'); - } else { - $params['text'] = idx($this->params, 'body'); + if (idx($this->params, 'html-body')) { + $params['html'] = idx($this->params, 'html-body'); } $from = idx($this->params, 'from'); diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerAdapter.php @@ -91,17 +91,19 @@ } public function setBody($body) { + $this->mailer->IsHTML(false); $this->mailer->Body = $body; return $this; } - public function setSubject($subject) { - $this->mailer->Subject = $subject; + public function setHTMLBody($html_body) { + $this->mailer->IsHTML(true); + $this->mailer->Body = $html_body; return $this; } - public function setIsHTML($is_html) { - $this->mailer->IsHTML($is_html); + public function setSubject($subject) { + $this->mailer->Subject = $subject; return $this; } diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerLiteAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerLiteAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerLiteAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationPHPMailerLiteAdapter.php @@ -70,16 +70,23 @@ public function setBody($body) { $this->mailer->Body = $body; + $this->mailer->IsHTML(false); return $this; } - public function setSubject($subject) { - $this->mailer->Subject = $subject; + + /** Note: phpmailer-lite does NOT support sending messages with mixed version + * (plaintext and html). So for now lets just use HTML if it's available. + * @param $html + */ + public function setHTMLBody($html_body) { + $this->mailer->Body = $html_body; + $this->mailer->IsHTML(true); return $this; } - public function setIsHTML($is_html) { - $this->mailer->IsHTML($is_html); + public function setSubject($subject) { + $this->mailer->Subject = $subject; return $this; } diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationSendGridAdapter.php @@ -56,13 +56,14 @@ return $this; } - public function setSubject($subject) { - $this->params['subject'] = $subject; + public function setHTMLBody($body) { + $this->params['html-body'] = $body; return $this; } - public function setIsHTML($is_html) { - $this->params['is-html'] = $is_html; + + public function setSubject($subject) { + $this->params['subject'] = $subject; return $this; } @@ -89,10 +90,10 @@ } $params['subject'] = idx($this->params, 'subject'); - if (idx($this->params, 'is-html')) { - $params['html'] = idx($this->params, 'body'); - } else { - $params['text'] = idx($this->params, 'body'); + $params['text'] = idx($this->params, 'body'); + + if (idx($this->params, 'html-body')) { + $params['html'] = idx($this->params, 'html-body'); } $params['from'] = idx($this->params, 'from'); diff --git a/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php b/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php --- a/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php +++ b/src/applications/metamta/adapter/PhabricatorMailImplementationTestAdapter.php @@ -64,13 +64,13 @@ return $this; } - public function setSubject($subject) { - $this->guts['subject'] = $subject; + public function setHTMLBody($html_body) { + $this->guts['html-body'] = $html_body; return $this; } - public function setIsHTML($is_html) { - $this->guts['is-html'] = $is_html; + public function setSubject($subject) { + $this->guts['subject'] = $subject; return $this; } diff --git a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php --- a/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php +++ b/src/applications/metamta/management/PhabricatorMailManagementSendTestWorkflow.php @@ -105,7 +105,6 @@ $subject = $args->getArg('subject'); $tags = $args->getArg('tag'); $attach = $args->getArg('attach'); - $is_html = $args->getArg('html'); $is_bulk = $args->getArg('bulk'); $console->writeErr("%s\n", pht('Reading message body from stdin...')); @@ -116,12 +115,17 @@ ->addTos($tos) ->addCCs($ccs) ->setSubject($subject) - ->setBody($body) ->setOverrideNoSelfMailPreference(true) - ->setIsHTML($is_html) ->setIsBulk($is_bulk) ->setMailTags($tags); + if ($args->getArg('html')) { + $mail->setBody('plaintext email'); + $mail->setHTMLBody($body); + } else { + $mail->setBody($body); + } + if ($from) { $mail->setFrom($from->getPHID()); } diff --git a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php --- a/src/applications/metamta/storage/PhabricatorMetaMTAMail.php +++ b/src/applications/metamta/storage/PhabricatorMetaMTAMail.php @@ -213,13 +213,12 @@ return $this; } - public function getBody() { - return $this->getParam('body'); + public function setHTMLBody($html) { + $this->setParam('html-body', $html); } - public function setIsHTML($html) { - $this->setParam('is-html', $html); - return $this; + public function getBody() { + return $this->getParam('body'); } public function setIsErrorEmail($is_error) { @@ -425,15 +424,6 @@ $attachment->getMimeType()); } break; - case 'body': - $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); - if (strlen($value) > $max) { - $value = phutil_utf8_shorten($value, $max); - $value .= "\n"; - $value .= pht('(This email was truncated at %d bytes.)', $max); - } - $mailer->setBody($value); - break; case 'subject': // Only try to use preferences if everything is multiplexed, so we // get consistent behavior. @@ -499,11 +489,6 @@ $mailer->setSubject(implode(' ', array_filter($subject))); break; - case 'is-html': - if ($value) { - $mailer->setIsHTML(true); - } - break; case 'is-bulk': if ($value) { if (PhabricatorEnv::getEnvConfig('metamta.precedence-bulk')) { @@ -551,6 +536,20 @@ } } + $body = $params['body']; + $max = PhabricatorEnv::getEnvConfig('metamta.email-body-limit'); + if (strlen($body) > $max) { + $body = phutil_utf8_shorten($body, $max); + $body .= "\n"; + $body .= pht('(This email was truncated at %d bytes.)', $max); + } + $mailer->setBody($body); + + if (PhabricatorEnv::getEnvConfig('metamta.html-emails') && + isset($params['html-body'])) { + $mailer->setHTMLBody($params['html-body']); + } + if (!$add_to && !$add_cc) { $this->setStatus(self::STATUS_VOID); $this->setMessage( diff --git a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php --- a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php +++ b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php @@ -10,6 +10,7 @@ final class PhabricatorMetaMTAMailBody { private $sections = array(); + private $htmlSections = array(); private $attachments = array(); @@ -25,11 +26,28 @@ */ public function addRawSection($text) { if (strlen($text)) { - $this->sections[] = rtrim($text); + $text = rtrim($text); + $this->sections[] = $text; + $this->htmlSections[] = phutil_escape_html_newlines( + phutil_tag('div', array(), $text)); } return $this; } + public function addRawPlaintextSection($text) { + if (strlen($text)) { + $text = rtrim($text); + $this->sections[] = $text; + } + return $this; + } + + public function addRawHTMLSection($html) { + if (strlen($html)) { + $this->htmlSections[] = $html; + } + return $this; + } /** * Add a block of text with a section header. This is rendered like this: @@ -43,10 +61,26 @@ * @task compose */ public function addTextSection($header, $text) { + $this->addPlaintextSection($header, $text); + $this->addHTMLSection($header, + phutil_escape_html_newlines(phutil_tag('div', array(), $text))); + return $this; + } + + public function addPlaintextSection($header, $text) { $this->sections[] = $header."\n".$this->indent($text); return $this; } + public function addHTMLSection($header, $html_fragment) { + $this->htmlSections[] = phutil_tag('div', + array( + 'style' => 'font-weight:800;' + ), + $header).$html_fragment; + + return $this; + } /** * Add a Herald section with a rule management URI and a transcript URI. @@ -115,6 +149,9 @@ return implode("\n\n", $this->sections)."\n"; } + public function renderHTML() { + return implode('

', $this->htmlSections).'
'; + } /** * Retrieve attachments. diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -1810,7 +1810,8 @@ ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setMailTags($mail_tags) ->setIsBulk(true) - ->setBody($body->render()); + ->setBody($body->render()) + ->setHTMLBody($body->renderHTML()); foreach ($body->getAttachments() as $attachment) { $template->addAttachment($attachment); @@ -1998,6 +1999,7 @@ array $xactions) { $headers = array(); + $html_headers = array(); $comments = array(); foreach ($xactions as $xaction) { @@ -2008,6 +2010,9 @@ $header = $xaction->getTitleForMail(); if ($header !== null) { $headers[] = $header; + $html_headers[] = phutil_tag('span', array( + 'style' => 'color:'.$xaction->getColor() + ), $header); } $comment = $xaction->getBodyForMail(); @@ -2017,7 +2022,8 @@ } $body = new PhabricatorMetaMTAMailBody(); - $body->addRawSection(implode("\n", $headers)); + $body->addRawPlaintextSection(implode("\n", $headers)); + $body->addRawHTMLSection(implode('
', $html_headers)); foreach ($comments as $comment) { $body->addRawSection($comment);