1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-09-20 01:08:50 +02:00

Don't consider file references inside quoted text blocks to grant "attachment intent"

Summary:
Ref T13682. This is a marginal case, but if you quote someone else's text and it has a file reference embedded in it, that isn't good enough to generate "attachment intent".

Since you need both a reference and an explicit attachment, this should never actually affect any user-visible behavior today, but makes the ruleset more thorough.

Test Plan: Dragged and dropped a file, referencing it in either a quoted or unquoted block. Saw it attach only for a quoted block.

Maniphest Tasks: T13682

Differential Revision: https://secure.phabricator.com/D21833
This commit is contained in:
epriestley 2022-05-20 11:17:44 -07:00
parent 2a0feb3de0
commit 631c36aee3
6 changed files with 79 additions and 8 deletions

View file

@ -5,7 +5,7 @@ final class PhabricatorEmbedFileRemarkupRule
private $viewer; private $viewer;
const KEY_EMBED_FILE_PHIDS = 'phabricator.embedded-file-phids'; const KEY_ATTACH_INTENT_FILE_PHIDS = 'files.attach-intent';
protected function getObjectNamePrefix() { protected function getObjectNamePrefix() {
return 'F'; return 'F';
@ -23,13 +23,44 @@ final class PhabricatorEmbedFileRemarkupRule
PhabricatorFileThumbnailTransform::TRANSFORM_PREVIEW, PhabricatorFileThumbnailTransform::TRANSFORM_PREVIEW,
)) ))
->execute(); ->execute();
$objects = mpull($objects, null, 'getID');
$phids_key = self::KEY_EMBED_FILE_PHIDS;
$phids = $engine->getTextMetadata($phids_key, array()); // Identify files embedded in the block with "attachment intent", i.e.
foreach (mpull($objects, 'getPHID') as $phid) { // those files which the user appears to want to attach to the object.
$phids[] = $phid; // Files referenced inside quoted blocks are not considered to have this
// attachment intent.
$metadata_key = self::KEY_RULE_OBJECT.'.'.$this->getObjectNamePrefix();
$metadata = $engine->getTextMetadata($metadata_key, array());
$attach_key = self::KEY_ATTACH_INTENT_FILE_PHIDS;
$attach_phids = $engine->getTextMetadata($attach_key, array());
foreach ($metadata as $item) {
// If this reference was inside a quoted block, don't count it. Quoting
// someone else doesn't establish an intent to attach a file.
$depth = idx($item, 'quote.depth');
if ($depth > 0) {
continue;
}
$id = $item['id'];
$file = idx($objects, $id);
if (!$file) {
continue;
}
$attach_phids[] = $file->getPHID();
} }
$engine->setTextMetadata($phids_key, $phids);
$attach_phids = array_fuse($attach_phids);
$attach_phids = array_keys($attach_phids);
$engine->setTextMetadata($attach_key, $attach_phids);
return $objects; return $objects;
} }

View file

@ -632,7 +632,7 @@ final class PhabricatorMarkupEngine extends Phobject {
foreach ($content_blocks as $content_block) { foreach ($content_blocks as $content_block) {
$engine->markupText($content_block); $engine->markupText($content_block);
$phids = $engine->getTextMetadata( $phids = $engine->getTextMetadata(
PhabricatorEmbedFileRemarkupRule::KEY_EMBED_FILE_PHIDS, PhabricatorEmbedFileRemarkupRule::KEY_ATTACH_INTENT_FILE_PHIDS,
array()); array());
foreach ($phids as $phid) { foreach ($phids as $phid) {
$files[$phid] = $phid; $files[$phid] = $phid;

View file

@ -43,6 +43,14 @@ abstract class PhutilRemarkupBlockRule extends Phobject {
return; return;
} }
public function willMarkupChildBlocks() {
return;
}
public function didMarkupChildBlocks() {
return;
}
final public function setEngine(PhutilRemarkupEngine $engine) { final public function setEngine(PhutilRemarkupEngine $engine) {
$this->engine = $engine; $this->engine = $engine;
$this->updateRules(); $this->updateRules();

View file

@ -7,6 +7,22 @@ abstract class PhutilRemarkupQuotedBlockRule
return true; return true;
} }
public function willMarkupChildBlocks() {
$engine = $this->getEngine();
$depth = $engine->getQuoteDepth();
$depth = $depth + 1;
$engine->setQuoteDepth($depth);
}
public function didMarkupChildBlocks() {
$engine = $this->getEngine();
$depth = $engine->getQuoteDepth();
$depth = $depth - 1;
$engine->setQuoteDepth($depth);
}
final protected function normalizeQuotedBody($text) { final protected function normalizeQuotedBody($text) {
$text = phutil_split_lines($text, true); $text = phutil_split_lines($text, true);
foreach ($text as $key => $line) { foreach ($text as $key => $line) {

View file

@ -42,6 +42,14 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine {
return $this->mode & self::MODE_HTML_MAIL; return $this->mode & self::MODE_HTML_MAIL;
} }
public function getQuoteDepth() {
return $this->getConfig('runtime.quote.depth', 0);
}
public function setQuoteDepth($depth) {
return $this->setConfig('runtime.quote.depth', $depth);
}
public function setBlockRules(array $rules) { public function setBlockRules(array $rules) {
assert_instances_of($rules, 'PhutilRemarkupBlockRule'); assert_instances_of($rules, 'PhutilRemarkupBlockRule');
@ -255,18 +263,24 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine {
} }
private function markupBlock(array $block) { private function markupBlock(array $block) {
$rule = $block['rule'];
$rule->willMarkupChildBlocks();
$children = array(); $children = array();
foreach ($block['children'] as $child) { foreach ($block['children'] as $child) {
$children[] = $this->markupBlock($child); $children[] = $this->markupBlock($child);
} }
$rule->didMarkupChildBlocks();
if ($children) { if ($children) {
$children = $this->flattenOutput($children); $children = $this->flattenOutput($children);
} else { } else {
$children = null; $children = null;
} }
return $block['rule']->markupText($block['text'], $children); return $rule->markupText($block['text'], $children);
} }
private function flattenOutput(array $output) { private function flattenOutput(array $output) {

View file

@ -318,6 +318,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
'id' => $matches[1], 'id' => $matches[1],
'options' => idx($matches, 2), 'options' => idx($matches, 2),
'original' => $matches[0], 'original' => $matches[0],
'quote.depth' => $engine->getQuoteDepth(),
)); ));
} }
@ -337,6 +338,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule {
'id' => $matches[1], 'id' => $matches[1],
'anchor' => idx($matches, 2), 'anchor' => idx($matches, 2),
'original' => $matches[0], 'original' => $matches[0],
'quote.depth' => $engine->getQuoteDepth(),
)); ));
} }