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: '' + }; } }