Page MenuHomePhabricator

D21273.id50658.diff
No OneTemporary

D21273.id50658.diff

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@
'core.pkg.js' => '845355f4',
'dark-console.pkg.js' => '187792c2',
'differential.pkg.css' => '42a2334f',
- 'differential.pkg.js' => '8f59bce2',
+ 'differential.pkg.js' => 'ff8ca085',
'diffusion.pkg.css' => '42c75c37',
'diffusion.pkg.js' => 'a98c0bf7',
'maniphest.pkg.css' => '35995d6d',
@@ -379,9 +379,9 @@
'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => 'a2ab19be',
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
- 'rsrc/js/application/diff/DiffChangeset.js' => '0c083409',
- 'rsrc/js/application/diff/DiffChangesetList.js' => 'db615898',
- 'rsrc/js/application/diff/DiffInline.js' => 'b00168c1',
+ 'rsrc/js/application/diff/DiffChangeset.js' => '6e5e03d2',
+ 'rsrc/js/application/diff/DiffChangesetList.js' => 'b51ba93a',
+ 'rsrc/js/application/diff/DiffInline.js' => '28e53a2c',
'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
@@ -774,9 +774,9 @@
'phabricator-darklog' => '3b869402',
'phabricator-darkmessage' => '26cd4b73',
'phabricator-dashboard-css' => '5a205b9d',
- 'phabricator-diff-changeset' => '0c083409',
- 'phabricator-diff-changeset-list' => 'db615898',
- 'phabricator-diff-inline' => 'b00168c1',
+ 'phabricator-diff-changeset' => '6e5e03d2',
+ 'phabricator-diff-changeset-list' => 'b51ba93a',
+ 'phabricator-diff-inline' => '28e53a2c',
'phabricator-diff-path-view' => '8207abf9',
'phabricator-diff-tree-view' => '5d83623b',
'phabricator-drag-and-drop-file-upload' => '4370900d',
@@ -1000,19 +1000,6 @@
'javelin-dom',
'phabricator-draggable-list',
),
- '0c083409' => array(
- 'javelin-dom',
- 'javelin-util',
- 'javelin-stratcom',
- 'javelin-install',
- 'javelin-workflow',
- 'javelin-router',
- 'javelin-behavior-device',
- 'javelin-vector',
- 'phabricator-diff-inline',
- 'phabricator-diff-path-view',
- 'phuix-button-view',
- ),
'0cf79f45' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1150,6 +1137,9 @@
'javelin-install',
'javelin-util',
),
+ '28e53a2c' => array(
+ 'javelin-dom',
+ ),
'29819b75' => array(
'phabricator-notification',
'javelin-stratcom',
@@ -1561,6 +1551,19 @@
'javelin-install',
'javelin-util',
),
+ '6e5e03d2' => array(
+ 'javelin-dom',
+ 'javelin-util',
+ 'javelin-stratcom',
+ 'javelin-install',
+ 'javelin-workflow',
+ 'javelin-router',
+ 'javelin-behavior-device',
+ 'javelin-vector',
+ 'phabricator-diff-inline',
+ 'phabricator-diff-path-view',
+ 'phuix-button-view',
+ ),
70245195 => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1935,9 +1938,6 @@
'javelin-behavior-device',
'javelin-vector',
),
- 'b00168c1' => array(
- 'javelin-dom',
- ),
'b105a3a6' => array(
'javelin-behavior',
'javelin-stratcom',
@@ -1970,6 +1970,11 @@
'b517bfa0' => array(
'phui-oi-list-view-css',
),
+ 'b51ba93a' => array(
+ 'javelin-install',
+ 'phuix-button-view',
+ 'phabricator-diff-tree-view',
+ ),
'b557770a' => array(
'javelin-install',
'javelin-util',
@@ -2119,11 +2124,6 @@
'javelin-uri',
'phabricator-notification',
),
- 'db615898' => array(
- 'javelin-install',
- 'phuix-button-view',
- 'phabricator-diff-tree-view',
- ),
'e150bd50' => array(
'javelin-behavior',
'javelin-stratcom',
diff --git a/src/infrastructure/diff/PhabricatorInlineCommentController.php b/src/infrastructure/diff/PhabricatorInlineCommentController.php
--- a/src/infrastructure/diff/PhabricatorInlineCommentController.php
+++ b/src/infrastructure/diff/PhabricatorInlineCommentController.php
@@ -39,7 +39,6 @@
private $isOnRight;
private $lineNumber;
private $lineLength;
- private $commentText;
private $operation;
private $commentID;
private $renderer;
@@ -99,16 +98,16 @@
$request = $this->getRequest();
$viewer = $this->getViewer();
+ if (!$request->validateCSRF()) {
+ return new Aphront404Response();
+ }
+
$this->readRequestParameters();
$op = $this->getOperation();
switch ($op) {
case 'hide':
case 'show':
- if (!$request->validateCSRF()) {
- return new Aphront404Response();
- }
-
$ids = $request->getStrList('ids');
if ($ids) {
if ($op == 'hide') {
@@ -120,9 +119,6 @@
return id(new AphrontAjaxResponse())->setContent(array());
case 'done':
- if (!$request->validateCSRF()) {
- return new Aphront404Response();
- }
$inline = $this->loadCommentForDone($this->getCommentID());
$is_draft_state = false;
@@ -158,10 +154,6 @@
case 'delete':
case 'undelete':
case 'refdelete':
- if (!$request->validateCSRF()) {
- return new Aphront404Response();
- }
-
// NOTE: For normal deletes, we just process the delete immediately
// and show an "Undo" action. For deletes by reference from the
// preview ("refdelete"), we prompt first (because the "Undo" may
@@ -193,15 +185,14 @@
return $this->buildEmptyResponse();
case 'edit':
+ case 'save':
$inline = $this->loadCommentByIDForEdit($this->getCommentID());
- $text = $this->getCommentText();
+ if ($op === 'save') {
+ $this->updateCommentContentState($inline);
- if ($request->isFormPost()) {
- if (strlen($text)) {
- $inline
- ->setContent($text)
- ->setIsEditing(false);
+ $inline->setIsEditing(false);
+ if (!$inline->isVoidComment($viewer)) {
$this->saveComment($inline);
return $this->buildRenderedCommentResponse(
@@ -216,7 +207,7 @@
}
} else {
// NOTE: At time of writing, the "editing" state of inlines is
- // preserved by simluating a click on "Edit" when the inline loads.
+ // preserved by simulating a click on "Edit" when the inline loads.
// In this case, we don't want to "saveComment()", because it
// recalculates object drafts and purges versioned drafts.
@@ -234,8 +225,8 @@
$is_dirty = true;
}
- if (strlen($text)) {
- $inline->setContent($text);
+ if ($this->hasContentState()) {
+ $this->updateCommentContentState($inline);
$is_dirty = true;
} else {
PhabricatorInlineComment::loadAndAttachVersionedDrafts(
@@ -262,13 +253,9 @@
// If the user uses "Undo" to get into an edited state ("AB"), then
// clicks cancel to return to the previous state ("A"), we also want
// to set the stored state back to "A".
- $text = $this->getCommentText();
- if (strlen($text)) {
- $inline->setContent($text);
- }
+ $this->updateCommentContentState($inline);
- $content = $inline->getContent();
- if (!strlen($content)) {
+ if ($inline->isVoidComment($viewer)) {
$inline->setIsDeleted(1);
}
@@ -283,11 +270,11 @@
$viewer->getPHID(),
$inline->getID());
- $text = $this->getCommentText();
-
- $versioned_draft
- ->setProperty('inline.text', $text)
- ->save();
+ $map = $this->getContentState();
+ foreach ($map as $key => $value) {
+ $versioned_draft->setProperty($key, $value);
+ }
+ $versioned_draft->save();
// We have to synchronize the draft engine after saving a versioned
// draft, because taking an inline comment from "no text, no draft"
@@ -318,11 +305,11 @@
->setIsNewFile($is_new)
->setLineNumber($number)
->setLineLength($length)
- ->setContent((string)$this->getCommentText())
->setReplyToCommentPHID($this->getReplyToCommentPHID())
->setIsEditing(true)
->setStartOffset($request->getInt('startOffset'))
- ->setEndOffset($request->getInt('endOffset'));
+ ->setEndOffset($request->getInt('endOffset'))
+ ->setContent('');
$document_engine_key = $request->getStr('documentEngineKey');
if ($document_engine_key !== null) {
@@ -338,6 +325,10 @@
}
}
+ if ($this->hasContentState()) {
+ $this->updateCommentContentState($inline);
+ }
+
$this->saveComment($inline);
$edit_dialog = $this->buildEditDialog($inline);
@@ -365,7 +356,6 @@
$this->isOnRight = $request->getBool('on_right');
$this->lineNumber = $request->getInt('number');
$this->lineLength = $request->getInt('length');
- $this->commentText = $request->getStr('text');
$this->commentID = $request->getInt('id');
$this->operation = $request->getStr('op');
$this->renderer = $request->getStr('renderer');
@@ -529,6 +519,36 @@
return $inline;
}
+ private function hasContentState() {
+ $request = $this->getRequest();
+ return (bool)$request->getBool('hasContentState');
+ }
+
+ private function getContentState() {
+ $request = $this->getRequest();
+
+ $comment_text = $request->getStr('text');
+
+ return array(
+ 'inline.text' => (string)$comment_text,
+ );
+ }
+
+ private function updateCommentContentState(PhabricatorInlineComment $inline) {
+ if (!$this->hasContentState()) {
+ throw new Exception(
+ pht(
+ 'Attempting to update comment content state, but request has no '.
+ 'content state.'));
+ }
+
+ $state = $this->getContentState();
+
+ $text = $state['inline.text'];
+
+ $inline->setContent($text);
+ }
+
private function saveComment(PhabricatorInlineComment $inline) {
$viewer = $this->getViewer();
$draft_engine = $this->newDraftEngine();
diff --git a/src/infrastructure/diff/interface/PhabricatorInlineComment.php b/src/infrastructure/diff/interface/PhabricatorInlineComment.php
--- a/src/infrastructure/diff/interface/PhabricatorInlineComment.php
+++ b/src/infrastructure/diff/interface/PhabricatorInlineComment.php
@@ -322,6 +322,11 @@
return $draft_text;
}
+ public function getContentState() {
+ return array(
+ 'text' => $this->getContent(),
+ );
+ }
/* -( PhabricatorMarkupInterface Implementation )-------------------------- */
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php
@@ -22,39 +22,12 @@
'sigil' => 'inline-edit-form',
),
array(
- $this->renderInputs(),
$this->renderBody(),
));
return $content;
}
- private function renderInputs() {
- $inputs = array();
- $inline = $this->getInlineComment();
-
- $inputs[] = array('op', 'edit');
- $inputs[] = array('id', $inline->getID());
-
- $inputs[] = array('on_right', $this->getIsOnRight());
- $inputs[] = array('renderer', $this->getRenderer());
-
- $out = array();
-
- foreach ($inputs as $input) {
- list($name, $value) = $input;
- $out[] = phutil_tag(
- 'input',
- array(
- 'type' => 'hidden',
- 'name' => $name,
- 'value' => $value,
- ));
- }
-
- return $out;
- }
-
private function renderBody() {
$buttons = array();
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentView.php
@@ -82,7 +82,6 @@
'number' => $inline->getLineNumber(),
'length' => $inline->getLineLength(),
'isNewFile' => (bool)$inline->getIsNewFile(),
- 'original' => $inline->getContent(),
'replyToCommentPHID' => $inline->getReplyToCommentPHID(),
'isDraft' => $inline->isDraft(),
'isFixed' => $is_fixed,
@@ -93,8 +92,8 @@
'documentEngineKey' => $inline->getDocumentEngineKey(),
'startOffset' => $inline->getStartOffset(),
'endOffset' => $inline->getEndOffset(),
-
'on_right' => $this->getIsOnRight(),
+ 'contentState' => $inline->getContentState(),
);
}
diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js
--- a/webroot/rsrc/js/application/diff/DiffChangeset.js
+++ b/webroot/rsrc/js/application/diff/DiffChangeset.js
@@ -761,14 +761,14 @@
return inline;
},
- newInlineReply: function(original, text) {
+ newInlineReply: function(original, state) {
var inline = new JX.DiffInline()
.setChangeset(this)
.bindToReply(original);
this._inlines.push(inline);
- inline.create(text);
+ inline.create(state);
return inline;
},
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -2410,7 +2410,7 @@
switch (action) {
case 'save':
- inline.save(e.getTarget());
+ inline.save();
break;
case 'cancel':
inline.cancel();
diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js
--- a/webroot/rsrc/js/application/diff/DiffInline.js
+++ b/webroot/rsrc/js/application/diff/DiffInline.js
@@ -19,7 +19,7 @@
_displaySide: null,
_isNewFile: null,
_replyToCommentPHID: null,
- _originalText: null,
+ _originalState: null,
_snippet: null,
_menuItems: null,
_documentEngineKey: null,
@@ -42,7 +42,7 @@
_editRow: null,
_undoRow: null,
_undoType: null,
- _undoText: null,
+ _undoState: null,
_draftRequest: null,
_skipFocus: false,
@@ -76,12 +76,7 @@
this._number = parseInt(data.number, 10);
this._length = parseInt(data.length, 10);
- var original = '' + data.original;
- if (original.length) {
- this._originalText = original;
- } else {
- this._originalText = null;
- }
+ this._originalState = data.contentState;
this._isNewFile = data.isNewFile;
this._replyToCommentPHID = data.replyToCommentPHID;
@@ -314,10 +309,6 @@
return this._hasMenuAction('collapse');
},
- getRawText: function() {
- return this._originalText;
- },
-
_newRow: function() {
var attributes = {
sigil: 'inline-row'
@@ -332,7 +323,7 @@
this._phid = null;
this._isCollapsed = false;
- this._originalText = null;
+ this._originalState = null;
return row;
},
@@ -404,7 +395,7 @@
this._didUpdate();
},
- create: function(text) {
+ create: function(content_state) {
var changeset = this.getChangeset();
if (!this._documentEngineKey) {
this._documentEngineKey = changeset.getResponseDocumentEngineKey();
@@ -412,7 +403,7 @@
var uri = this._getInlineURI();
var handler = JX.bind(this, this._oncreateresponse);
- var data = this._newRequestData('new', text);
+ var data = this._newRequestData('new', content_state);
this.setLoading(true);
@@ -421,22 +412,33 @@
.send();
},
+ _getContentState: function() {
+ var state;
+
+ if (this._editRow) {
+ state = this._readFormState(this._editRow);
+ } else {
+ state = this._originalState;
+ }
+
+ return state;
+ },
+
reply: function(with_quote) {
this._closeMenu();
- var text;
+ var content_state = this._newContentState();
if (with_quote) {
- text = this.getRawText();
+ var text = this._getContentState().text;
text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n';
- } else {
- text = '';
+ content_state.text = text;
}
var changeset = this.getChangeset();
- return changeset.newInlineReply(this, text);
+ return changeset.newInlineReply(this, content_state);
},
- edit: function(text, skip_focus) {
+ edit: function(content_state, skip_focus) {
this._closeMenu();
this._skipFocus = !!skip_focus;
@@ -456,7 +458,7 @@
var uri = this._getInlineURI();
var handler = JX.bind(this, this._oneditresponse);
- var data = this._newRequestData('edit', text || null);
+ var data = this._newRequestData('edit', content_state);
this.setLoading(true);
@@ -545,13 +547,12 @@
return this;
},
- _newRequestData: function(operation, text) {
+ _newRequestData: function(operation, content_state) {
var data = {
op: operation,
is_new: this.isNewFile(),
on_right: ((this.getDisplaySide() == 'right') ? 1 : 0),
- renderer: this.getChangeset().getRendererKey(),
- text: text || null
+ renderer: this.getChangeset().getRendererKey()
};
if (operation === 'new') {
@@ -574,6 +575,11 @@
JX.copy(data, edit_data);
}
+ if (content_state) {
+ data.hasContentState = 1;
+ JX.copy(data, content_state);
+ }
+
return data;
},
@@ -608,14 +614,14 @@
// If there's an existing editor, remove it. This happens when you
// delete a comment from the comment preview area. In this case, we
// read and preserve the text so "Undo" restores it.
- var text;
+ var state = null;
if (this._editRow) {
- text = this._readText(this._editRow);
+ state = this._readFormState(this._editRow);
JX.DOM.remove(this._editRow);
this._editRow = null;
}
- this._drawUndeleteRows(text);
+ this._drawUndeleteRows(state);
this.setLoading(false);
this.setDeleted(true);
@@ -623,21 +629,21 @@
this._didUpdate();
},
- _drawUndeleteRows: function(text) {
+ _drawUndeleteRows: function(content_state) {
this._undoType = 'undelete';
- this._undoText = text || null;
+ this._undoState = content_state || null;
return this._drawUndoRows('undelete', this._row);
},
- _drawUneditRows: function(text) {
+ _drawUneditRows: function(content_state) {
this._undoType = 'unedit';
- this._undoText = text;
+ this._undoState = content_state;
- return this._drawUndoRows('unedit', null, text);
+ return this._drawUndoRows('unedit', null);
},
- _drawUndoRows: function(mode, cursor, text) {
+ _drawUndoRows: function(mode, cursor) {
var templates = this.getChangeset().getUndoTemplates();
var template;
@@ -648,7 +654,7 @@
}
template = JX.$H(template).getNode();
- this._undoRow = this._drawRows(template, cursor, mode, text);
+ this._undoRow = this._drawRows(template, cursor, mode);
},
_drawContentRows: function(rows) {
@@ -660,7 +666,7 @@
this._editRow = this._drawRows(rows, null, 'edit');
},
- _drawRows: function(rows, cursor, type, text) {
+ _drawRows: function(rows, cursor, type) {
var first_row = JX.DOM.scry(rows, 'tr')[0];
var row = first_row;
var anchor = cursor || this._row;
@@ -713,14 +719,17 @@
return result_row;
},
- save: function(form) {
+ save: function() {
var handler = JX.bind(this, this._onsubmitresponse);
this.setLoading(true);
- JX.Workflow.newFromForm(form)
- .setHandler(handler)
- .start();
+ var uri = this._getInlineURI();
+ var data = this._newRequestData('save', this._getContentState());
+
+ new JX.Request(uri, handler)
+ .setData(data)
+ .send();
},
undo: function() {
@@ -740,8 +749,8 @@
.send();
}
- if (this._undoText !== null) {
- this.edit(this._undoText);
+ if (this._undoState !== null) {
+ this.edit(this._undoState);
}
},
@@ -751,18 +760,20 @@
},
cancel: function() {
- var text = this._readText(this._editRow);
+ var state = this._readFormState(this._editRow);
JX.DOM.remove(this._editRow);
this._editRow = null;
- if (text && text.length && (text != this._originalText)) {
- this._drawUneditRows(text);
+ var is_empty = this._isVoidContentState(state);
+ var is_same = this._isSameContentState(state, this._originalState);
+ if (!is_empty && !is_same) {
+ this._drawUneditRows(state);
}
// If this was an empty box and we typed some text and then hit cancel,
// don't show the empty concrete inline.
- if (!this._originalText) {
+ if (!this._isVoidContentState(this._originalState)) {
this.setInvisible(true);
} else {
this.setInvisible(false);
@@ -773,7 +784,7 @@
// text ("A") to the server as the current persistent state.
var uri = this._getInlineURI();
- var data = this._newRequestData('cancel', this._originalText);
+ var data = this._newRequestData('cancel', this._originalState);
var handler = JX.bind(this, this._onCancelResponse);
this.setLoading(true);
@@ -792,7 +803,7 @@
// If the comment was empty when we started editing it (there's no
// original text) and empty when we finished editing it (there's no
// undo row), just delete the comment.
- if (!this._originalText && !this.isUndo()) {
+ if (this._isVoidContentState(this._originalState) && !this.isUndo()) {
this.setDeleted(true);
JX.DOM.remove(this._row);
@@ -802,7 +813,7 @@
}
},
- _readText: function(row) {
+ _readFormState: function(row) {
var textarea;
try {
textarea = JX.DOM.find(
@@ -813,7 +824,9 @@
return null;
}
- return textarea.value;
+ return {
+ text: textarea.value
+ };
},
_onsubmitresponse: function(response) {
@@ -920,16 +933,19 @@
return null;
}
- var text = this._readText(this._editRow);
- if (text === null) {
+ var state = this._readFormState(this._editRow);
+ if (this._isVoidContentState(state)) {
return null;
}
- return {
+ var draft_data = {
op: 'draft',
id: this.getID(),
- text: text
};
+
+ JX.copy(draft_data, state);
+
+ return draft_data;
},
triggerDraft: function() {
@@ -1035,6 +1051,20 @@
if (this._menu) {
this._menu.close();
}
+ },
+
+ _isVoidContentState: function(state) {
+ return !state.text.length;
+ },
+
+ _isSameContentState: function(u, v) {
+ return (u.text === v.text);
+ },
+
+ _newContentState: function() {
+ return {
+ text: ''
+ };
}
}

File Metadata

Mime Type
text/plain
Expires
Wed, Mar 19, 12:22 AM (1 w, 12 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7342542
Default Alt Text
D21273.id50658.diff (23 KB)

Event Timeline