diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1778,6 +1778,7 @@ 'PhabricatorMetaMTAMail' => 'applications/metamta/storage/PhabricatorMetaMTAMail.php', 'PhabricatorMetaMTAMailBody' => 'applications/metamta/view/PhabricatorMetaMTAMailBody.php', 'PhabricatorMetaMTAMailBodyTestCase' => 'applications/metamta/view/__tests__/PhabricatorMetaMTAMailBodyTestCase.php', + 'PhabricatorMetaMTAMailSection' => 'applications/metamta/view/PhabricatorMetaMTAMailSection.php', 'PhabricatorMetaMTAMailTestCase' => 'applications/metamta/storage/__tests__/PhabricatorMetaMTAMailTestCase.php', 'PhabricatorMetaMTAMailgunReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php', 'PhabricatorMetaMTAMailingList' => 'applications/mailinglists/storage/PhabricatorMetaMTAMailingList.php', 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,20 +1164,21 @@ $config_attach = PhabricatorEnv::getEnvConfig($config_key_attach); if ($config_inline || $config_attach) { - $patch = $this->renderPatchForMail($diff); - $lines = count(phutil_split_lines($patch)); + $patchSection = $this->renderPatchForMail($diff); + $lines = count(phutil_split_lines($patchSection->getPlaintext())); if ($config_inline && ($lines <= $config_inline)) { $body->addTextSection( pht('CHANGE DETAILS'), - $patch); + $patchSection); } if ($config_attach) { $name = pht('D%s.%s.patch', $object->getID(), $diff->getID()); $mime_type = 'text/x-patch; charset=utf-8'; $body->addAttachment( - new PhabricatorMetaMTAAttachment($patch, $name, $mime_type)); + new PhabricatorMetaMTAAttachment( + $patchSection->getPlaintext(), $name, $mime_type)); } } } @@ -1298,7 +1299,7 @@ $hunk_parser = new DifferentialHunkParser(); } - $result = array(); + $section = new PhabricatorMetaMTAMailSection(); foreach ($inline_groups as $changeset_id => $group) { $changeset = idx($changesets, $changeset_id); if (!$changeset) { @@ -1319,25 +1320,27 @@ $inline_content = $comment->getContent(); if (!$show_context) { - $result[] = "{$file}:{$range} {$inline_content}"; + $section->addFragment("{$file}:{$range} {$inline_content}"); } else { - $result[] = '================'; - $result[] = 'Comment at: '.$file.':'.$range; - $result[] = $hunk_parser->makeContextDiff( + $patch = $hunk_parser->makeContextDiff( $changeset->getHunks(), $comment->getIsNewFile(), $comment->getLineNumber(), $comment->getLineLength(), 1); - $result[] = '----------------'; - $result[] = $inline_content; - $result[] = null; + $section->addFragment('================') + ->addFragment('Comment at: '.$file.':'.$range) + ->addPlaintextFragment($patch) + ->addHTMLFragment($this->renderPatchHTMLForMail($patch)) + ->addFragment('----------------') + ->addFragment($inline_content) + ->addFragment(null); } } } - return implode("\n", $result); + return $section; } private function loadDiff($phid, $need_changesets = false) { @@ -1694,14 +1697,47 @@ return implode("\n", $filenames); } + private function renderPatchHTMLForMail($patch) { + // 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); + } + + return "
".implode(
+      '', $patch_html_lines).'
'; + + } 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(); + + $section = new PhabricatorMetaMTAMailSection(); + $section->addHTMLFragment($this->renderPatchHTMLForMail($patch)); + $section->addPlaintextFragment($patch); + + return $section; } } 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 = idx($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,7 +26,25 @@ */ 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; } @@ -42,11 +61,34 @@ * @return this * @task compose */ - public function addTextSection($header, $text) { + public function addTextSection($header, $section) { + if ($section instanceof PhabricatorMetaMTAMailSection) { + $plaintext = $section->getPlaintext(); + $html = $section->getHTML(); + } else { + $plaintext = $section; + $html = phutil_escape_html_newlines(phutil_tag('div', array(), $section)); + } + + $this->addPlaintextSection($header, $plaintext); + $this->addHTMLSection($header, $html); + 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 +157,9 @@ return implode("\n\n", $this->sections)."\n"; } + public function renderHTML() { + return implode('

', $this->htmlSections).'
'; + } /** * Retrieve attachments. diff --git a/src/applications/metamta/view/PhabricatorMetaMTAMailSection.php b/src/applications/metamta/view/PhabricatorMetaMTAMailSection.php new file mode 100644 --- /dev/null +++ b/src/applications/metamta/view/PhabricatorMetaMTAMailSection.php @@ -0,0 +1,40 @@ +htmlFragments); + } + + public function getPlaintext() { + return implode('\n', $this->plaintextFragments); + } + + public function addHTMLFragment($fragment) { + $this->htmlFragments[] = $fragment; + return $this; + + } + + public function addPlaintextFragment($fragment) { + $this->plaintextFragments[] = $fragment; + return $this; + } + + public function addFragment($fragment) { + $this->plaintextFragments[] = $fragment; + $this->htmlFragments[] = + phutil_escape_html_newlines(phutil_tag('div', array(), $fragment)); + + return $this; + } +} 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 @@ -1811,7 +1811,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); @@ -1999,6 +2000,7 @@ array $xactions) { $headers = array(); + $html_headers = array(); $comments = array(); foreach ($xactions as $xaction) { @@ -2009,6 +2011,9 @@ $header = $xaction->getTitleForMail(); if ($header !== null) { $headers[] = $header; + $html_headers[] = phutil_tag('span', array( + 'style' => 'color:'.$xaction->getColor() + ), $header); } $comment = $xaction->getBodyForMail(); @@ -2018,7 +2023,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);