1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 00:42:41 +01:00

When creating an inline comment, populate the content state with the default suggestion text

Summary:
Ref T13559. Currently, the default text for inline comment side-loads in a bizarre way. Instead, when a user creates an inline comment, load the inline context and set it as part of the initial content state.

This allows the side channel (and the code that puts the text in place at the last second on the client) to be removed.

Test Plan: Created inlines, clicked "Suggest Edit". See followup changes.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21652
This commit is contained in:
epriestley 2021-03-25 10:55:56 -07:00
parent 5efe7fb4c1
commit b75517918d
5 changed files with 39 additions and 24 deletions

View file

@ -13,7 +13,7 @@ return array(
'core.pkg.js' => '68f29322', 'core.pkg.js' => '68f29322',
'dark-console.pkg.js' => '187792c2', 'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => 'ffb69e3d', 'differential.pkg.css' => 'ffb69e3d',
'differential.pkg.js' => 'bbf6d742', 'differential.pkg.js' => 'fbde899f',
'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => '78c9885d', 'diffusion.pkg.js' => '78c9885d',
'maniphest.pkg.css' => '35995d6d', 'maniphest.pkg.css' => '35995d6d',
@ -385,7 +385,7 @@ return array(
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75', 'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75',
'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5', 'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5',
'rsrc/js/application/diff/DiffInline.js' => 'c794b624', 'rsrc/js/application/diff/DiffInline.js' => '62fff8eb',
'rsrc/js/application/diff/DiffInlineContentState.js' => '68e6339d', 'rsrc/js/application/diff/DiffInlineContentState.js' => '68e6339d',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9', 'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b', 'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
@ -788,7 +788,7 @@ return array(
'phabricator-dashboard-css' => '5a205b9d', 'phabricator-dashboard-css' => '5a205b9d',
'phabricator-diff-changeset' => 'd7d3ba75', 'phabricator-diff-changeset' => 'd7d3ba75',
'phabricator-diff-changeset-list' => 'cc2c5de5', 'phabricator-diff-changeset-list' => 'cc2c5de5',
'phabricator-diff-inline' => 'c794b624', 'phabricator-diff-inline' => '62fff8eb',
'phabricator-diff-inline-content-state' => '68e6339d', 'phabricator-diff-inline-content-state' => '68e6339d',
'phabricator-diff-path-view' => '8207abf9', 'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b', 'phabricator-diff-tree-view' => '5d83623b',
@ -1532,6 +1532,10 @@ return array(
'javelin-request', 'javelin-request',
'javelin-uri', 'javelin-uri',
), ),
'62fff8eb' => array(
'javelin-dom',
'phabricator-diff-inline-content-state',
),
'65bb0011' => array( '65bb0011' => array(
'javelin-behavior', 'javelin-behavior',
'javelin-dom', 'javelin-dom',
@ -2088,10 +2092,6 @@ return array(
'javelin-workflow', 'javelin-workflow',
'javelin-json', 'javelin-json',
), ),
'c794b624' => array(
'javelin-dom',
'phabricator-diff-inline-content-state',
),
'cc2c5de5' => array( 'cc2c5de5' => array(
'javelin-install', 'javelin-install',
'phuix-button-view', 'phuix-button-view',

View file

@ -316,11 +316,28 @@ abstract class PhabricatorInlineCommentController
$this->updateCommentContentState($inline); $this->updateCommentContentState($inline);
} }
// NOTE: We're writing the comment as "deleted", then reloading to
// pick up context and undeleting it. This is silly -- we just want
// to load and attach context -- but just loading context is currently
// complicated (for example, context relies on cache keys that expect
// the inline to have an ID).
$inline->setIsDeleted(1);
$this->saveComment($inline); $this->saveComment($inline);
// Reload the inline to attach context. // Reload the inline to attach context.
$inline = $this->loadCommentByIDForEdit($inline->getID()); $inline = $this->loadCommentByIDForEdit($inline->getID());
// Now, we can read the source file and set the initial state.
$state = $inline->getContentState();
$default_suggestion = $inline->getDefaultSuggestionText();
$state->setContentSuggestionText($default_suggestion);
$inline->setContentState($state);
$inline->setIsDeleted(0);
$this->saveComment($inline);
$edit_dialog = $this->buildEditDialog($inline); $edit_dialog = $this->buildEditDialog($inline);
if ($this->getOperation() == 'reply') { if ($this->getOperation() == 'reply') {

View file

@ -364,6 +364,19 @@ abstract class PhabricatorInlineComment
return $this->getStorageObject()->getInlineContext(); return $this->getStorageObject()->getInlineContext();
} }
public function getDefaultSuggestionText() {
$context = $this->getInlineContext();
if (!$context) {
return null;
}
$default = $context->getBodyLines();
$default = implode('', $default);
return $default;
}
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */ /* -( PhabricatorMarkupInterface Implementation )-------------------------- */

View file

@ -114,9 +114,6 @@ final class PHUIDiffInlineCommentEditView
$main = $state->getContentSuggestionText(); $main = $state->getContentSuggestionText();
$main_count = count(phutil_split_lines($main)); $main_count = count(phutil_split_lines($main));
$default = $context->getBodyLines();
$default = implode('', $default);
// Browsers ignore one leading newline in text areas. Add one so that // Browsers ignore one leading newline in text areas. Add one so that
// any actual leading newlines in the content are preserved. // any actual leading newlines in the content are preserved.
$main = "\n".$main; $main = "\n".$main;
@ -127,9 +124,6 @@ final class PHUIDiffInlineCommentEditView
'class' => 'inline-suggestion-input PhabricatorMonospaced', 'class' => 'inline-suggestion-input PhabricatorMonospaced',
'rows' => max(3, $main_count + 1), 'rows' => max(3, $main_count + 1),
'sigil' => 'inline-content-suggestion', 'sigil' => 'inline-content-suggestion',
'meta' => array(
'defaultText' => $default,
),
), ),
$main); $main);

View file

@ -772,21 +772,12 @@ JX.install('DiffInline', {
this.setHasSuggestion(!this.getHasSuggestion()); this.setHasSuggestion(!this.getHasSuggestion());
// The first time the user actually clicks the button and enables // Resize the suggestion input for size of the text.
// suggestions for a given editor state, fill the input with the
// underlying text if there isn't any text yet.
if (this.getHasSuggestion()) { if (this.getHasSuggestion()) {
if (this._editRow) { if (this._editRow) {
var node = this._getSuggestionNode(this._editRow); var node = this._getSuggestionNode(this._editRow);
if (node) { if (node) {
if (!node.value.length) { node.rows = Math.max(3, node.value.split('\n').length);
var data = JX.Stratcom.getData(node);
if (!data.hasSetDefault) {
data.hasSetDefault = true;
node.value = data.defaultText;
node.rows = Math.max(3, node.value.split('\n').length);
}
}
} }
} }
} }