From 1b438a8bd106ebf222ac7ec348b9dd353f77d11b Mon Sep 17 00:00:00 2001 From: lkassianik Date: Mon, 17 Nov 2014 18:26:18 -0800 Subject: [PATCH] Process Remarkup in text and HTML email bodies appropriately Summary: Ref T6343, adding HTMLMailMode to remarkup, and most objects should now be processed and appear pretty in emails. Test Plan: Add a comment to a Maniphest task containing a mention of an object like '{T1}' or 'T1'. Emails should show a styled version of the object similar to how the object looks in the context of the Maniphest task in the UI. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: chad, Korvin, epriestley Maniphest Tasks: T6343, T2617 Differential Revision: https://secure.phabricator.com/D10859 --- .../markup/DivinerSymbolRemarkupRule.php | 4 + .../markup/PhabricatorIconRemarkupRule.php | 7 +- .../PhabricatorImageMacroRemarkupRule.php | 2 + .../markup/PhabricatorMemeRemarkupRule.php | 4 + .../view/PhabricatorMetaMTAMailBody.php | 42 +++++++++++ .../markup/PhabricatorMentionRemarkupRule.php | 47 ++++++++---- .../markup/PhrictionRemarkupRule.php | 5 +- ...habricatorApplicationTransactionEditor.php | 3 +- .../PhabricatorNavigationRemarkupRule.php | 22 ++++-- .../rule/PhabricatorObjectRemarkupRule.php | 73 +++++++++++++++++-- .../rule/PhabricatorYoutubeRemarkupRule.php | 4 +- 11 files changed, 181 insertions(+), 32 deletions(-) diff --git a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php index 422209fca0..bb4fafd11e 100644 --- a/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php +++ b/src/applications/diviner/markup/DivinerSymbolRemarkupRule.php @@ -140,6 +140,10 @@ final class DivinerSymbolRemarkupRule extends PhutilRemarkupRule { $link = $title; } } else if ($href) { + if ($this->getEngine()->isHTMLMailMode()) { + $href = PhabricatorEnv::getProductionURI($href); + } + $link = $this->newTag( 'a', array( diff --git a/src/applications/macro/markup/PhabricatorIconRemarkupRule.php b/src/applications/macro/markup/PhabricatorIconRemarkupRule.php index 7fd9f6b6be..9daf9b6f0c 100644 --- a/src/applications/macro/markup/PhabricatorIconRemarkupRule.php +++ b/src/applications/macro/markup/PhabricatorIconRemarkupRule.php @@ -14,7 +14,11 @@ final class PhabricatorIconRemarkupRule extends PhutilRemarkupRule { } public function markupIcon($matches) { - if (!$this->isFlatText($matches[0])) { + $engine = $this->getEngine(); + $text_mode = $engine->isTextMode(); + $mail_mode = $engine->isHTMLMailMode(); + + if (!$this->isFlatText($matches[0]) || $text_mode || $mail_mode) { return $matches[0]; } @@ -69,6 +73,7 @@ final class PhabricatorIconRemarkupRule extends PhutilRemarkupRule { $icon_view = id(new PHUIIconView()) ->setIconFont('fa-'.$icon, $color); + return $this->getEngine()->storeText($icon_view); } diff --git a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php index 081f2bd5c5..965064b4a4 100644 --- a/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php +++ b/src/applications/macro/markup/PhabricatorImageMacroRemarkupRule.php @@ -109,6 +109,8 @@ final class PhabricatorImageMacroRemarkupRule extends PhutilRemarkupRule { $result = $spec['original'].' <'.$src_uri.'>'; $engine->overwriteStoredText($spec['token'], $result); continue; + } else if ($this->getEngine()->isHTMLMailMode()) { + $src_uri = PhabricatorEnv::getProductionURI($src_uri); } $file_data = $file->getMetadata(); diff --git a/src/applications/macro/markup/PhabricatorMemeRemarkupRule.php b/src/applications/macro/markup/PhabricatorMemeRemarkupRule.php index 8e6e0f5639..0b65226e09 100644 --- a/src/applications/macro/markup/PhabricatorMemeRemarkupRule.php +++ b/src/applications/macro/markup/PhabricatorMemeRemarkupRule.php @@ -34,6 +34,10 @@ final class PhabricatorMemeRemarkupRule extends PhutilRemarkupRule { ->alter('uppertext', $options['above']) ->alter('lowertext', $options['below']); + if ($this->getEngine()->isHTMLMailMode()) { + $uri = PhabricatorEnv::getProductionURI($uri); + } + if ($this->getEngine()->isTextMode()) { $img = ($options['above'] != '' ? "\"{$options['above']}\"\n" : ''). diff --git a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php index e1ef4d6373..9e1a41f1c0 100644 --- a/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php +++ b/src/applications/metamta/view/PhabricatorMetaMTAMailBody.php @@ -12,6 +12,15 @@ final class PhabricatorMetaMTAMailBody { private $htmlSections = array(); private $attachments = array(); + private $viewer; + + public function getViewer() { + return $this->viewer; + } + + public function setViewer($viewer) { + $this->viewer = $viewer; + } /* -( Composition )-------------------------------------------------------- */ @@ -33,6 +42,39 @@ final class PhabricatorMetaMTAMailBody { return $this; } + public function addRemarkupSection($text) { + try { + $engine = PhabricatorMarkupEngine::newMarkupEngine(array()); + $engine->setConfig('viewer', $this->getViewer()); + $engine->setMode(PhutilRemarkupEngine::MODE_TEXT); + $styled_text = $engine->markupText($text); + $this->sections[] = $styled_text; + } catch (Exception $ex) { + phlog($ex); + $this->sections[] = $text; + } + + try { + $mail_engine = PhabricatorMarkupEngine::newMarkupEngine(array()); + $mail_engine->setConfig('viewer', $this->getViewer()); + $mail_engine->setMode(PhutilRemarkupEngine::MODE_HTML_MAIL); + $mail_engine->setConfig( + 'uri.base', + PhabricatorEnv::getProductionURI('/')); + $html = $mail_engine->markupText($text); + $this->htmlSections[] = $html; + } catch (Exception $ex) { + phlog($ex); + $this->htmlSections[] = phutil_escape_html_newlines( + phutil_tag( + 'div', + array(), + $text)); + } + + return $this; + } + public function addRawPlaintextSection($text) { if (strlen($text)) { $text = rtrim($text); diff --git a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php index 49f56251f0..622b28998e 100644 --- a/src/applications/people/markup/PhabricatorMentionRemarkupRule.php +++ b/src/applications/people/markup/PhabricatorMentionRemarkupRule.php @@ -100,22 +100,41 @@ final class PhabricatorMentionRemarkupRule extends PhutilRemarkupRule { $user = $actual_users[$username]; Javelin::initBehavior('phabricator-hovercards'); - $tag = id(new PHUITagView()) - ->setType(PHUITagView::TYPE_PERSON) - ->setPHID($user->getPHID()) - ->setName('@'.$user->getUserName()) - ->setHref('/p/'.$user->getUserName().'/'); + $user_href = '/p/'.$user->getUserName().'/'; - if (!$user->isUserActivated()) { - $tag->setDotColor(PHUITagView::COLOR_GREY); + if ($engine->isHTMLMailMode()) { + $user_href = PhabricatorEnv::getProductionURI($user_href); + $tag = phutil_tag( + 'a', + array( + 'href' => $user_href, + 'style' => 'background-color: #f1f7ff; + border-color: #f1f7ff; + border: 1px solid transparent; + border-radius: 3px; + color: #19558d; + font-weight: bold; + padding: 0 4px;', + ), + '@'.$user->getUserName()); } else { - $status = idx($user_statuses, $user->getPHID()); - if ($status) { - $status = $status->getStatus(); - if ($status == PhabricatorCalendarEvent::STATUS_AWAY) { - $tag->setDotColor(PHUITagView::COLOR_RED); - } else if ($status == PhabricatorCalendarEvent::STATUS_AWAY) { - $tag->setDotColor(PHUITagView::COLOR_ORANGE); + $tag = id(new PHUITagView()) + ->setType(PHUITagView::TYPE_PERSON) + ->setPHID($user->getPHID()) + ->setName('@'.$user->getUserName()) + ->setHref($user_href); + + if (!$user->isUserActivated()) { + $tag->setDotColor(PHUITagView::COLOR_GREY); + } else { + $status = idx($user_statuses, $user->getPHID()); + if ($status) { + $status = $status->getStatus(); + if ($status == PhabricatorCalendarEvent::STATUS_AWAY) { + $tag->setDotColor(PHUITagView::COLOR_RED); + } else if ($status == PhabricatorCalendarEvent::STATUS_AWAY) { + $tag->setDotColor(PHUITagView::COLOR_ORANGE); + } } } } diff --git a/src/applications/phriction/markup/PhrictionRemarkupRule.php b/src/applications/phriction/markup/PhrictionRemarkupRule.php index 48eb9cc3ee..ba740e5f7b 100644 --- a/src/applications/phriction/markup/PhrictionRemarkupRule.php +++ b/src/applications/phriction/markup/PhrictionRemarkupRule.php @@ -29,9 +29,12 @@ final class PhrictionRemarkupRule extends PhutilRemarkupRule { $slug = PhrictionDocument::getSlugURI($slug); $href = (string)id(new PhutilURI($slug))->setFragment($fragment); + $text_mode = $this->getEngine()->isTextMode(); + $mail_mode = $this->getEngine()->isHTMLMailMode(); + if ($this->getEngine()->getState('toc')) { $text = $name; - } else if ($this->getEngine()->isTextMode()) { + } else if ($text_mode || $mail_mode) { return PhabricatorEnv::getProductionURI($href); } else { $text = $this->newTag( diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index 6de4b46ff5..ae225915b3 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -2159,10 +2159,11 @@ abstract class PhabricatorApplicationTransactionEditor } $body = new PhabricatorMetaMTAMailBody(); + $body->setViewer($this->requireActor()); $body->addRawSection(implode("\n", $headers)); foreach ($comments as $comment) { - $body->addRawSection($comment); + $body->addRemarkupSection($comment); } if ($object instanceof PhabricatorCustomFieldInterface) { diff --git a/src/infrastructure/markup/rule/PhabricatorNavigationRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorNavigationRemarkupRule.php index d0bf3a69b1..2f32950f4e 100644 --- a/src/infrastructure/markup/rule/PhabricatorNavigationRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorNavigationRemarkupRule.php @@ -80,20 +80,30 @@ final class PhabricatorNavigationRemarkupRule extends PhutilRemarkupRule { $out[] = $tag; } + if ($this->getEngine()->isHTMLMailMode()) { + $arrow_attr = array( + 'style' => 'color: #92969D;', + ); + $nav_attr = array(); + } else { + $arrow_attr = array( + 'class' => 'remarkup-nav-sequence-arrow', + ); + $nav_attr = array( + 'class' => 'remarkup-nav-sequence', + ); + } + $joiner = phutil_tag( 'span', - array( - 'class' => 'remarkup-nav-sequence-arrow', - ), + $arrow_attr, " \xE2\x86\x92 "); $out = phutil_implode_html($joiner, $out); $out = phutil_tag( 'span', - array( - 'class' => 'remarkup-nav-sequence', - ), + $nav_attr, $out); return $this->getEngine()->storeText($out); diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php index 1e60bde7dc..3f8688897e 100644 --- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php @@ -44,7 +44,11 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { return $handle->getURI(); } - protected function renderObjectRef($object, $handle, $anchor, $id) { + protected function renderObjectRefForAnyMedia ( + $object, + $handle, + $anchor, + $id) { $href = $this->getObjectHref($object, $handle, $id); $text = $this->getObjectNamePrefix().$id; @@ -55,10 +59,25 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { if ($this->getEngine()->isTextMode()) { return PhabricatorEnv::getProductionURI($href); + } else if ($this->getEngine()->isHTMLMailMode()) { + $href = PhabricatorEnv::getProductionURI($href); + return $this->renderObjectTagForMail($text, $href, $handle); } + return $this->renderObjectRef($object, $handle, $anchor, $id); + + } + + protected function renderObjectRef($object, $handle, $anchor, $id) { + $href = $this->getObjectHref($object, $handle, $id); + $text = $this->getObjectNamePrefix().$id; $status_closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; + if ($anchor) { + $href = $href.'#'.$anchor; + $text = $text.'#'.$anchor; + } + $attr = array( 'phid' => $handle->getPHID(), 'closed' => ($handle->getStatus() == $status_closed), @@ -67,15 +86,24 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { return $this->renderHovertag($text, $href, $attr); } + protected function renderObjectEmbedForAnyMedia($object, $handle, $options) { + $name = $handle->getFullName(); + $href = $handle->getURI(); + + if ($this->getEngine()->isTextMode()) { + return $name.' <'.PhabricatorEnv::getProductionURI($href).'>'; + } else if ($this->getEngine()->isHTMLMailMode()) { + $href = PhabricatorEnv::getProductionURI($href); + return $this->renderObjectTagForMail($name, $href, $handle); + } + + return $this->renderObjectEmbed($object, $handle, $options); + } + protected function renderObjectEmbed($object, $handle, $options) { $name = $handle->getFullName(); $href = $handle->getURI(); $status_closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; - - if ($this->getEngine()->isTextMode()) { - return $name.' <'.PhabricatorEnv::getProductionURI($href).'>'; - } - $attr = array( 'phid' => $handle->getPHID(), 'closed' => ($handle->getStatus() == $status_closed), @@ -84,6 +112,31 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { return $this->renderHovertag($name, $href, $attr); } + protected function renderObjectTagForMail( + $text, + $href, + $handle) { + + $status_closed = PhabricatorObjectHandleStatus::STATUS_CLOSED; + $strikethrough = $handle->getStatus() == $status_closed ? + 'text-decoration: line-through;' : + 'text-decoration: none;'; + + return phutil_tag( + 'a', + array( + 'href' => $href, + 'style' => 'background-color: #e7e7e7; + border-color: #e7e7e7; + border-radius: 3px; + padding: 0 4px; + font-weight: bold; + color: black;' + .$strikethrough, + ), + $text); + } + protected function renderHovertag($name, $href, array $attr = array()) { return id(new PHUITagView()) ->setName($name) @@ -282,7 +335,8 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { $object = $objects[$spec['id']]; switch ($spec['type']) { case 'ref': - $view = $this->renderObjectRef( + + $view = $this->renderObjectRefForAnyMedia( $object, $handle, $spec['anchor'], @@ -290,7 +344,10 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { break; case 'embed': $spec['options'] = $this->assertFlatText($spec['options']); - $view = $this->renderObjectEmbed($object, $handle, $spec['options']); + $view = $this->renderObjectEmbedForAnyMedia( + $object, + $handle, + $spec['options']); break; } $engine->overwriteStoredText($spec['token'], $view); diff --git a/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php index 6528783aed..43af8d3f01 100644 --- a/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorYoutubeRemarkupRule.php @@ -20,8 +20,10 @@ final class PhabricatorYoutubeRemarkupRule extends PhutilRemarkupRule { public function markupYoutubeLink() { $v = idx($this->uri->getQueryParams(), 'v'); + $text_mode = $this->getEngine()->isTextMode(); + $mail_mode = $this->getEngine()->isHTMLMailMode(); - if ($this->getEngine()->isTextMode()) { + if ($text_mode || $mail_mode) { return $this->getEngine()->storeText('http://youtu.be/'.$v); }