Skip to content

Commit 99034ef

Browse files
author
epriestley
committedOct 5, 2018
Make Pholio mail render without a ton of over-escaped HTML
Summary: Ref T13202. See PHI900. Fixes T12814. Pholio currently builds HTML comments in an older way that can dump a bunch of over-escaped HTML into mail bodies. Update the logic to be more similar to the Differential rendering logic and stop over-escaping things. The result isn't perfect, but is dramatically less broken. Test Plan: {F5919860} Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13202, T12814 Differential Revision: https://secure.phabricator.com/D19733
1 parent c6c1893 commit 99034ef

File tree

4 files changed

+78
-41
lines changed

4 files changed

+78
-41
lines changed
 

‎src/applications/differential/editor/DifferentialTransactionEditor.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -636,8 +636,8 @@ protected function buildMailBody(
636636

637637
$viewer = $this->requireActor();
638638

639-
$body = new PhabricatorMetaMTAMailBody();
640-
$body->setViewer($this->requireActor());
639+
$body = id(new PhabricatorMetaMTAMailBody())
640+
->setViewer($viewer);
641641

642642
$revision_uri = $object->getURI();
643643
$revision_uri = PhabricatorEnv::getProductionURI($revision_uri);

‎src/applications/pholio/editor/PholioMockEditor.php

+63-39
Original file line numberDiff line numberDiff line change
@@ -128,59 +128,83 @@ protected function buildMailBody(
128128
PhabricatorLiskDAO $object,
129129
array $xactions) {
130130

131-
$body = new PhabricatorMetaMTAMailBody();
132-
$headers = array();
133-
$comments = array();
134-
$inline_comments = array();
131+
$viewer = $this->requireActor();
135132

136-
foreach ($xactions as $xaction) {
137-
if ($xaction->shouldHide()) {
138-
continue;
139-
}
140-
$comment = $xaction->getComment();
141-
switch ($xaction->getTransactionType()) {
142-
case PholioMockInlineTransaction::TRANSACTIONTYPE:
143-
if ($comment && strlen($comment->getContent())) {
144-
$inline_comments[] = $comment;
145-
}
146-
break;
147-
case PhabricatorTransactions::TYPE_COMMENT:
148-
if ($comment && strlen($comment->getContent())) {
149-
$comments[] = $comment->getContent();
150-
}
151-
// fallthrough
152-
default:
153-
$headers[] = id(clone $xaction)
154-
->setRenderingTarget('text')
155-
->getTitle();
156-
break;
157-
}
158-
}
133+
$body = id(new PhabricatorMetaMTAMailBody())
134+
->setViewer($viewer);
159135

160-
$body->addRawSection(implode("\n", $headers));
136+
$mock_uri = $object->getURI();
137+
$mock_uri = PhabricatorEnv::getProductionURI($mock_uri);
161138

162-
foreach ($comments as $comment) {
163-
$body->addRawSection($comment);
164-
}
139+
$this->addHeadersAndCommentsToMailBody(
140+
$body,
141+
$xactions,
142+
pht('View Mock'),
143+
$mock_uri);
165144

166-
if ($inline_comments) {
167-
$body->addRawSection(pht('INLINE COMMENTS'));
168-
foreach ($inline_comments as $comment) {
169-
$text = pht(
170-
'Image %d: %s',
171-
$comment->getImageID(),
172-
$comment->getContent());
173-
$body->addRawSection($text);
145+
$type_inline = PholioMockInlineTransaction::TRANSACTIONTYPE;
146+
147+
$inlines = array();
148+
foreach ($xactions as $xaction) {
149+
if ($xaction->getTransactionType() == $type_inline) {
150+
$inlines[] = $xaction;
174151
}
175152
}
176153

154+
$this->appendInlineCommentsForMail($object, $inlines, $body);
155+
177156
$body->addLinkSection(
178157
pht('MOCK DETAIL'),
179158
PhabricatorEnv::getProductionURI('/M'.$object->getID()));
180159

181160
return $body;
182161
}
183162

163+
private function appendInlineCommentsForMail(
164+
$object,
165+
array $inlines,
166+
PhabricatorMetaMTAMailBody $body) {
167+
168+
if (!$inlines) {
169+
return;
170+
}
171+
172+
$viewer = $this->requireActor();
173+
174+
$header = pht('INLINE COMMENTS');
175+
$body->addRawPlaintextSection($header);
176+
$body->addRawHTMLSection(phutil_tag('strong', array(), $header));
177+
178+
$image_ids = array();
179+
foreach ($inlines as $inline) {
180+
$comment = $inline->getComment();
181+
$image_id = $comment->getImageID();
182+
$image_ids[$image_id] = $image_id;
183+
}
184+
185+
$images = id(new PholioImageQuery())
186+
->setViewer($viewer)
187+
->withIDs($image_ids)
188+
->execute();
189+
$images = mpull($images, null, 'getID');
190+
191+
foreach ($inlines as $inline) {
192+
$comment = $inline->getComment();
193+
$content = $comment->getContent();
194+
$image_id = $comment->getImageID();
195+
$image = idx($images, $image_id);
196+
if ($image) {
197+
$image_name = $image->getName();
198+
} else {
199+
$image_name = pht('Unknown (ID %d)', $image_id);
200+
}
201+
202+
$body->addRemarkupSection(
203+
pht('Image "%s":', $image_name),
204+
$content);
205+
}
206+
}
207+
184208
protected function getMailSubjectPrefix() {
185209
return PhabricatorEnv::getEnvConfig('metamta.pholio.subject-prefix');
186210
}

‎src/applications/pholio/storage/PholioMock.php

+4
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,10 @@ public function getMonogram() {
5858
return 'M'.$this->getID();
5959
}
6060

61+
public function getURI() {
62+
return '/'.$this->getMonogram();
63+
}
64+
6165
protected function getConfiguration() {
6266
return array(
6367
self::CONFIG_AUX_PHID => true,

‎src/applications/pholio/storage/PholioTransaction.php

+9
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,13 @@ public function getMailTags() {
5353
return $tags;
5454
}
5555

56+
public function isInlineCommentTransaction() {
57+
switch ($this->getTransactionType()) {
58+
case PholioMockInlineTransaction::TRANSACTIONTYPE:
59+
return true;
60+
}
61+
62+
return parent::isInlineCommentTransaction();
63+
}
64+
5665
}

0 commit comments

Comments
 (0)
Failed to load comments.