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 @@ -1728,6 +1728,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', 'PhabricatorMetaMTAMailableDatasource' => 'applications/metamta/typeahead/PhabricatorMetaMTAMailableDatasource.php', 'PhabricatorMetaMTAMailgunReceiveController' => 'applications/metamta/controller/PhabricatorMetaMTAMailgunReceiveController.php', 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,20 @@ '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', true) + ->setBoolOptions( + array( + pht('Enable HTML emails'), + pht('Plaintext emails only'), + )) + ->setSummary( + pht( + 'Phabricator can send prettier emails when HTML is enabled ')) + ->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 @@ -1179,20 +1179,21 @@ $config_attach = PhabricatorEnv::getEnvConfig($config_key_attach); if ($config_inline || $config_attach) { - $patch = $this->renderPatchForMail($diff); - $lines = count(phutil_split_lines($patch)); + $patch_section = $this->renderPatchForMail($diff); + $lines = count(phutil_split_lines($patch_section->getPlaintext())); if ($config_inline && ($lines <= $config_inline)) { $body->addTextSection( pht('CHANGE DETAILS'), - $patch); + $patch_section); } 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( + $patch_section->getPlaintext(), $name, $mime_type)); } } } @@ -1330,7 +1331,7 @@ $hunk_parser = new DifferentialHunkParser(); } - $result = array(); + $section = new PhabricatorMetaMTAMailSection(); foreach ($inline_groups as $changeset_id => $group) { $changeset = idx($changesets, $changeset_id); if (!$changeset) { @@ -1351,25 +1352,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) { @@ -1762,14 +1765,25 @@ return implode("\n", $filenames); } + private function renderPatchHTMLForMail($patch) { + return phutil_tag('pre', + array('style' => 'font-family: monospace;'), $patch); + } + 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,24 @@ 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...')); @@ -117,10 +116,19 @@ ->addCCs($ccs) ->setSubject($subject) ->setBody($body) - ->setIsHTML($is_html) ->setIsBulk($is_bulk) ->setMailTags($tags); + if ($args->getArg('html')) { + $mail->setBody( + pht('(This is a placeholder plaintext email body for a test message '. + 'sent with --html.)')); + + $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 @@ -212,13 +212,13 @@ return $this; } - public function getBody() { - return $this->getParam('body'); + public function setHTMLBody($html) { + $this->setParam('html-body', $html); + return $this; } - public function setIsHTML($html) { - $this->setParam('is-html', $html); - return $this; + public function getBody() { + return $this->getParam('body'); } public function setIsErrorEmail($is_error) { @@ -377,6 +377,32 @@ $add_cc = array(); $add_to = array(); + // Only try to use preferences if everything is multiplexed, so we + // get consistent behavior. + $use_prefs = self::shouldMultiplexAllMail(); + + $prefs = null; + if ($use_prefs) { + + // If multiplexing is enabled, some recipients will be in "Cc" + // rather than "To". We'll move them to "To" later (or supply a + // dummy "To") but need to look for the recipient in either the + // "To" or "Cc" fields here. + $target_phid = head(idx($params, 'to', array())); + if (!$target_phid) { + $target_phid = head(idx($params, 'cc', array())); + } + + if ($target_phid) { + $user = id(new PhabricatorUser())->loadOneWhere( + 'phid = %s', + $target_phid); + if ($user) { + $prefs = $user->loadPreferences(); + } + } + } + foreach ($params as $key => $value) { switch ($key) { case 'from': @@ -444,42 +470,7 @@ $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. - $use_prefs = self::shouldMultiplexAllMail(); - - $prefs = null; - if ($use_prefs) { - - // If multiplexing is enabled, some recipients will be in "Cc" - // rather than "To". We'll move them to "To" later (or supply a - // dummy "To") but need to look for the recipient in either the - // "To" or "Cc" fields here. - $target_phid = head(idx($params, 'to', array())); - if (!$target_phid) { - $target_phid = head(idx($params, 'cc', array())); - } - - if ($target_phid) { - $user = id(new PhabricatorUser())->loadOneWhere( - 'phid = %s', - $target_phid); - if ($user) { - $prefs = $user->loadPreferences(); - } - } - } - $subject = array(); if ($is_threaded) { @@ -518,11 +509,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')) { @@ -570,6 +556,26 @@ } } + $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); + + $html_emails = PhabricatorEnv::getEnvConfig('metamta.html-emails'); + if ($use_prefs && $prefs) { + $html_emails = $prefs->getPreference( + PhabricatorUserPreferences::PREFERENCE_HTML_EMAILS, + $html_emails); + } + + if ($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 @@ -9,6 +9,7 @@ final class PhabricatorMetaMTAMailBody { private $sections = array(); + private $htmlSections = array(); private $attachments = array(); @@ -24,7 +25,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; } @@ -41,11 +60,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. @@ -114,6 +156,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/settings/panel/PhabricatorSettingsPanelEmailFormat.php b/src/applications/settings/panel/PhabricatorSettingsPanelEmailFormat.php --- a/src/applications/settings/panel/PhabricatorSettingsPanelEmailFormat.php +++ b/src/applications/settings/panel/PhabricatorSettingsPanelEmailFormat.php @@ -22,6 +22,7 @@ $pref_re_prefix = PhabricatorUserPreferences::PREFERENCE_RE_PREFIX; $pref_vary = PhabricatorUserPreferences::PREFERENCE_VARY_SUBJECT; + $prefs_html_email = PhabricatorUserPreferences::PREFERENCE_HTML_EMAILS; $errors = array(); if ($request->isFormPost()) { @@ -42,6 +43,14 @@ $pref_vary, $request->getBool($pref_vary)); } + + if ($request->getStr($prefs_html_email) == 'default') { + $preferences->unsetPreference($prefs_html_email); + } else { + $preferences->setPreference( + $prefs_html_email, + $request->getBool($prefs_html_email)); + } } $preferences->save(); @@ -58,6 +67,10 @@ ? pht('Vary') : pht('Do Not Vary'); + $html_emails_default = PhabricatorEnv::getEnvConfig('metamta.html-emails') + ? pht('HTML') + : pht('plaintext'); + $re_prefix_value = $preferences->getPreference($pref_re_prefix); if ($re_prefix_value === null) { $re_prefix_value = 'default'; @@ -76,11 +89,30 @@ : 'false'; } + $html_emails_value = $preferences->getPreference($prefs_html_email); + if ($html_emails_value === null) { + $html_emails_value = 'default'; + } else { + $html_emails_value = $html_emails_value + ? 'true' + : 'false'; + } + $form = new AphrontFormView(); $form ->setUser($user); if (PhabricatorMetaMTAMail::shouldMultiplexAllMail()) { + $html_email_control = id(new AphrontFormSelectControl()) + ->setName($prefs_html_email) + ->setOptions( + array( + 'default' => pht('Use Server Default (%s)', $html_emails_default), + 'true' => pht('HTML emails'), + 'false' => pht('Plaintext emails'), + )) + ->setValue($html_emails_value); + $re_control = id(new AphrontFormSelectControl()) ->setName($pref_re_prefix) ->setOptions( @@ -101,6 +133,9 @@ )) ->setValue($vary_value); } else { + $html_email_control = id(new AphrontFormStaticControl()) + ->setValue('Server Default ('.$html_emails_default.')'); + $re_control = id(new AphrontFormStaticControl()) ->setValue('Server Default ('.$re_prefix_default.')'); @@ -124,6 +159,7 @@ } $form + ->appendChild($html_email_control) ->appendRemarkupInstructions('') ->appendRemarkupInstructions( pht( diff --git a/src/applications/settings/storage/PhabricatorUserPreferences.php b/src/applications/settings/storage/PhabricatorUserPreferences.php --- a/src/applications/settings/storage/PhabricatorUserPreferences.php +++ b/src/applications/settings/storage/PhabricatorUserPreferences.php @@ -15,6 +15,7 @@ const PREFERENCE_NO_MAIL = 'no-mail'; const PREFERENCE_MAILTAGS = 'mailtags'; const PREFERENCE_VARY_SUBJECT = 'vary-subject'; + const PREFERENCE_HTML_EMAILS = 'html-emails'; const PREFERENCE_SEARCHBAR_JUMP = 'searchbar-jump'; const PREFERENCE_SEARCH_SHORTCUT = 'search-shortcut'; 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 @@ -1865,7 +1865,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); @@ -2096,6 +2097,7 @@ array $xactions) { $headers = array(); + $html_headers = array(); $comments = array(); foreach ($xactions as $xaction) { @@ -2106,6 +2108,9 @@ $header = $xaction->getTitleForMail(); if ($header !== null) { $headers[] = $header; + $html_headers[] = phutil_tag('span', array( + 'style' => 'color:'.$xaction->getColor() + ), $header); } $comment = $xaction->getBodyForMail(); @@ -2115,7 +2120,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);