From 0c0cbb1c09e4bc5a27f1246181754ab22f13499c Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 14 Jan 2019 09:52:41 -0800
Subject: [PATCH] Fix an issue where transactions in mail were always rendered
 as text

Summary:
Fixes T12921. Currently, we call `getTitleForHTMLMail()`, but that calls `getTitleForMail()` which forces us into text rendering mode.

Instead, have `getTitleForHTML/TextMail()` force the rendering mode, then call `getTitleForMail()` with the desired rendering mode.

This causes stories like "epriestely added dependent tasks: x, y." to appear as links in email instead of plain text.

Test Plan: Used `bin/mail show-outbound --id ... --dump-html > out.html` to verify HTML mail.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12921

Differential Revision: https://secure.phabricator.com/D19968
---
 ...habricatorApplicationTransactionEditor.php |  4 ++--
 .../PhabricatorApplicationTransaction.php     | 21 +++++++++++++++++--
 .../storage/PhabricatorModularTransaction.php |  9 --------
 3 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index cd5125d3b3..64f375fd88 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -3265,7 +3265,7 @@ abstract class PhabricatorApplicationTransactionEditor
       }
 
       if (!$is_comment || !$seen_comment) {
-        $header = $xaction->getTitleForMail();
+        $header = $xaction->getTitleForTextMail();
         if ($header !== null) {
           $headers[] = $header;
         }
@@ -3350,7 +3350,7 @@ abstract class PhabricatorApplicationTransactionEditor
       // If this is not the first comment in the mail, add the header showing
       // who wrote the comment immediately above the comment.
       if (!$is_initial) {
-        $header = $xaction->getTitleForMail();
+        $header = $xaction->getTitleForTextMail();
         if ($header !== null) {
           $body->addRawPlaintextSection($header);
         }
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index 725178d6ca..515fd87394 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -763,12 +763,29 @@ abstract class PhabricatorApplicationTransaction
     return $this->shouldHideForFeed();
   }
 
+  private function getTitleForMailWithRenderingTarget($new_target) {
+    $old_target = $this->getRenderingTarget();
+    try {
+      $this->setRenderingTarget($new_target);
+      $result = $this->getTitleForMail();
+    } catch (Exception $ex) {
+      $this->setRenderingTarget($old_target);
+      throw $ex;
+    }
+    $this->setRenderingTarget($old_target);
+    return $result;
+  }
+
   public function getTitleForMail() {
-    return id(clone $this)->setRenderingTarget('text')->getTitle();
+    return $this->getTitle();
+  }
+
+  public function getTitleForTextMail() {
+    return $this->getTitleForMailWithRenderingTarget(self::TARGET_TEXT);
   }
 
   public function getTitleForHTMLMail() {
-    $title = $this->getTitleForMail();
+    $title = $this->getTitleForMailWithRenderingTarget(self::TARGET_HTML);
     if ($title === null) {
       return null;
     }
diff --git a/src/applications/transactions/storage/PhabricatorModularTransaction.php b/src/applications/transactions/storage/PhabricatorModularTransaction.php
index 38b9d1835d..59466ee472 100644
--- a/src/applications/transactions/storage/PhabricatorModularTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorModularTransaction.php
@@ -150,15 +150,6 @@ abstract class PhabricatorModularTransaction
     return parent::getActionStrength();
   }
 
-  public function getTitleForMail() {
-    $old_target = $this->getRenderingTarget();
-    $new_target = self::TARGET_TEXT;
-    $this->setRenderingTarget($new_target);
-    $title = $this->getTitle();
-    $this->setRenderingTarget($old_target);
-    return $title;
-  }
-
   /* final */ public function getTitleForFeed() {
     $title = $this->getTransactionImplementation()->getTitleForFeed();
     if ($title !== null) {