(stable) Promote 2021 Week 21

resources/celerity/map.php

@@ -10,10 +10,10 @@ return array(
     'conpherence.pkg.css' => '0e3cf785',
     'conpherence.pkg.js' => '020aebcf',
     'core.pkg.css' => '0ae696de',
-    'core.pkg.js' => 'ab3502fe',
+    'core.pkg.js' => '68f29322',
     'dark-console.pkg.js' => '187792c2',
     'differential.pkg.css' => 'ffb69e3d',
-    'differential.pkg.js' => '5986f349',
+    'differential.pkg.js' => '8deec4cd',
     'diffusion.pkg.css' => '42c75c37',
     'diffusion.pkg.js' => '78c9885d',
     'maniphest.pkg.css' => '35995d6d',
@@ -253,7 +253,7 @@ return array(
     'rsrc/externals/javelin/lib/Mask.js' => '7c4d8998',
     'rsrc/externals/javelin/lib/Quicksand.js' => 'd3799cb4',
     'rsrc/externals/javelin/lib/Request.js' => '84e6891f',
-    'rsrc/externals/javelin/lib/Resource.js' => '740956e1',
+    'rsrc/externals/javelin/lib/Resource.js' => '20514cc2',
     'rsrc/externals/javelin/lib/Routable.js' => '6a18c42e',
     'rsrc/externals/javelin/lib/Router.js' => '32755edb',
     'rsrc/externals/javelin/lib/Scrollbar.js' => 'a43ae2ae',
@@ -383,9 +383,10 @@ return array(
     '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' => '3b6e1fde',
+    'rsrc/js/application/diff/DiffChangeset.js' => 'd7d3ba75',
     'rsrc/js/application/diff/DiffChangesetList.js' => 'cc2c5de5',
-    'rsrc/js/application/diff/DiffInline.js' => '511a1315',
+    'rsrc/js/application/diff/DiffInline.js' => '9c775532',
+    'rsrc/js/application/diff/DiffInlineContentState.js' => 'aa51efb4',
     'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
     'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
     'rsrc/js/application/differential/behavior-diff-radios.js' => '925fe8cd',
@@ -733,7 +734,7 @@ return array(
     'javelin-reactor-node-calmer' => '225bbb98',
     'javelin-reactornode' => '72960bc1',
     'javelin-request' => '84e6891f',
-    'javelin-resource' => '740956e1',
+    'javelin-resource' => '20514cc2',
     'javelin-routable' => '6a18c42e',
     'javelin-router' => '32755edb',
     'javelin-scrollbar' => 'a43ae2ae',
@@ -785,9 +786,10 @@ return array(
     'phabricator-darklog' => '3b869402',
     'phabricator-darkmessage' => '26cd4b73',
     'phabricator-dashboard-css' => '5a205b9d',
-    'phabricator-diff-changeset' => '3b6e1fde',
+    'phabricator-diff-changeset' => 'd7d3ba75',
     'phabricator-diff-changeset-list' => 'cc2c5de5',
-    'phabricator-diff-inline' => '511a1315',
+    'phabricator-diff-inline' => '9c775532',
+    'phabricator-diff-inline-content-state' => 'aa51efb4',
     'phabricator-diff-path-view' => '8207abf9',
     'phabricator-diff-tree-view' => '5d83623b',
     'phabricator-drag-and-drop-file-upload' => '4370900d',
@@ -1104,6 +1106,11 @@ return array(
       'javelin-behavior',
       'javelin-request',
     ),
+    '20514cc2' => array(
+      'javelin-util',
+      'javelin-uri',
+      'javelin-install',
+    ),
     '225bbb98' => array(
       'javelin-install',
       'javelin-reactor',
@@ -1248,20 +1255,6 @@ return array(
       'javelin-behavior',
       'phabricator-prefab',
     ),
-    '3b6e1fde' => 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',
-      'javelin-external-editor-link-engine',
-    ),
     '3dc5ad43' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1413,9 +1406,6 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
-    '511a1315' => array(
-      'javelin-dom',
-    ),
     '5202e831' => array(
       'javelin-install',
       'javelin-dom',
@@ -1605,11 +1595,6 @@ return array(
       'javelin-stratcom',
       'phabricator-tooltip',
     ),
-    '740956e1' => array(
-      'javelin-util',
-      'javelin-uri',
-      'javelin-install',
-    ),
     74446546 => array(
       'javelin-behavior',
       'javelin-dom',
@@ -1831,6 +1816,10 @@ return array(
       'javelin-dom',
       'javelin-workflow',
     ),
+    '9c775532' => array(
+      'javelin-dom',
+      'phabricator-diff-inline-content-state',
+    ),
     '9cec214e' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1921,6 +1910,9 @@ return array(
       'javelin-typeahead-ondemand-source',
       'javelin-dom',
     ),
+    'aa51efb4' => array(
+      'javelin-dom',
+    ),
     'aa6d2308' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2129,6 +2121,20 @@ return array(
     'd4cc2d2a' => array(
       'javelin-install',
     ),
+    'd7d3ba75' => 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',
+      'javelin-external-editor-link-engine',
+    ),
     'd8a86cfb' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2441,6 +2447,7 @@ return array(
       'javelin-behavior-phabricator-object-selector',
       'javelin-behavior-repository-crossreference',
       'javelin-behavior-aphront-more',
+      'phabricator-diff-inline-content-state',
       'phabricator-diff-inline',
       'phabricator-diff-changeset',
       'phabricator-diff-changeset-list',

resources/celerity/packages.php

@@ -212,6 +212,7 @@ return array(
 
     'javelin-behavior-aphront-more',
 
+    'phabricator-diff-inline-content-state',
     'phabricator-diff-inline',
     'phabricator-diff-changeset',
     'phabricator-diff-changeset-list',

src/applications/differential/render/DifferentialChangesetRenderer.php

@@ -718,6 +718,9 @@ abstract class DifferentialChangesetRenderer extends Phobject {
 
     foreach ($views as $key => $view) {
       $scaffold = $this->getRowScaffoldForInline($view);
+
+      $scaffold->setIsUndoTemplate(true);
+
       $views[$key] = id(new PHUIDiffInlineCommentTableScaffold())
         ->addRowScaffold($scaffold);
     }

src/applications/phid/PhabricatorObjectHandle.php

@@ -402,6 +402,10 @@ final class PhabricatorObjectHandle
   }
 
   public function hasCapabilities() {
+    if (!$this->isComplete()) {
+      return false;
+    }
+
     return ($this->getType() === PhabricatorPeopleUserPHIDType::TYPECONST);
   }
 

src/infrastructure/diff/PhabricatorInlineCommentController.php

@@ -172,7 +172,9 @@ abstract class PhabricatorInlineCommentController
         $inline = $this->loadCommentByIDForEdit($this->getCommentID());
 
         if ($is_delete) {
-          $inline->setIsDeleted(1);
+          $inline
+            ->setIsEditing(false)
+            ->setIsDeleted(1);
         } else {
           $inline->setIsDeleted(0);
         }
@@ -180,30 +182,27 @@ abstract class PhabricatorInlineCommentController
         $this->saveComment($inline);
 
         return $this->buildEmptyResponse();
-      case 'edit':
       case 'save':
         $inline = $this->loadCommentByIDForEdit($this->getCommentID());
-        if ($op === 'save') {
+
         $this->updateCommentContentState($inline);
 
-          $inline->setIsEditing(false);
+        $inline
+          ->setIsEditing(false)
+          ->setIsDeleted(0);
 
-          if (!$inline->isVoidComment($viewer)) {
+        // Since we're saving the comment, update the committed state.
-            $inline->setIsDeleted(0);
+        $active_state = $inline->getContentState();
+        $inline->setCommittedContentState($active_state);
 
         $this->saveComment($inline);
 
         return $this->buildRenderedCommentResponse(
           $inline,
           $this->getIsOnRight());
-          } else {
+      case 'edit':
-            $inline->setIsDeleted(1);
+        $inline = $this->loadCommentByIDForEdit($this->getCommentID());
 
-            $this->saveComment($inline);
-
-            return $this->buildEmptyResponse();
-          }
-        } else {
         // NOTE: At time of writing, the "editing" state of inlines is
         // preserved by simulating a click on "Edit" when the inline loads.
 
@@ -238,7 +237,6 @@ abstract class PhabricatorInlineCommentController
         if ($is_dirty) {
           $this->saveComment($inline);
         }
-        }
 
         $edit_dialog = $this->buildEditDialog($inline)
           ->setTitle(pht('Edit Inline Comment'));
@@ -256,10 +254,6 @@ abstract class PhabricatorInlineCommentController
         // to set the stored state back to "A".
         $this->updateCommentContentState($inline);
 
-        if ($inline->isVoidComment($viewer)) {
-          $inline->setIsDeleted(1);
-        }
-
         $this->saveComment($inline);
 
         return $this->buildEmptyResponse();
@@ -328,11 +322,31 @@ abstract class PhabricatorInlineCommentController
           $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);
 
         // Reload the inline to attach context.
         $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->setInitialContentState($state);
+        $inline->setContentState($state);
+
+        $inline->setIsDeleted(0);
+
+        $this->saveComment($inline);
+
         $edit_dialog = $this->buildEditDialog($inline);
 
         if ($this->getOperation() == 'reply') {
@@ -456,6 +470,7 @@ abstract class PhabricatorInlineCommentController
     PhabricatorInlineComment $inline,
     $view,
     $is_edit) {
+    $viewer = $this->getViewer();
 
     if ($inline->getReplyToCommentPHID()) {
       $can_suggest = false;
@@ -464,18 +479,15 @@ abstract class PhabricatorInlineCommentController
     }
 
     if ($is_edit) {
-      $viewer = $this->getViewer();
+      $state = $inline->getContentStateMapForEdit($viewer);
-      $content_state = $inline->getContentStateForEdit($viewer);
     } else {
-      $content_state = $inline->getContentState();
+      $state = $inline->getContentStateMap();
     }
 
-    $state_map = $content_state->newStorageMap();
-
     $response = array(
       'inline' => array(
         'id' => $inline->getID(),
-        'contentState' => $state_map,
+        'state' => $state,
         'canSuggestEdit' => $can_suggest,
       ),
       'view' => hsprintf('%s', $view),

src/infrastructure/diff/interface/PhabricatorInlineComment.php

@@ -336,13 +336,29 @@ abstract class PhabricatorInlineComment
     return $this->newContentState()->readFromRequest($request);
   }
 
-  public function getContentState() {
+  public function getInitialContentState() {
-    $state = $this->newContentState();
+    return $this->getNamedContentState('inline.state.initial');
+  }
 
-    $storage = $this->getStorageObject();
+  public function setInitialContentState(
-    $storage_map = $storage->getAttribute('inline.state');
+    PhabricatorInlineCommentContentState $state) {
-    if (is_array($storage_map)) {
+    return $this->setNamedContentState('inline.state.initial', $state);
-      $state->readStorageMap($storage_map);
+  }
+
+  public function getCommittedContentState() {
+    return $this->getNamedContentState('inline.state.committed');
+  }
+
+  public function setCommittedContentState(
+    PhabricatorInlineCommentContentState $state) {
+    return $this->setNamedContentState('inline.state.committed', $state);
+  }
+
+  public function getContentState() {
+    $state = $this->getNamedContentState('inline.state');
+
+    if (!$state) {
+      $state = $this->newContentState();
     }
 
     $state->setContentText($this->getContent());
@@ -351,11 +367,31 @@ abstract class PhabricatorInlineComment
   }
 
   public function setContentState(PhabricatorInlineCommentContentState $state) {
+    $this->setContent($state->getContentText());
+
+    return $this->setNamedContentState('inline.state', $state);
+  }
+
+  private function getNamedContentState($key) {
+    $storage = $this->getStorageObject();
+
+    $storage_map = $storage->getAttribute($key);
+    if (!is_array($storage_map)) {
+      return null;
+    }
+
+    $state = $this->newContentState();
+    $state->readStorageMap($storage_map);
+    return $state;
+  }
+
+  private function setNamedContentState(
+    $key,
+    PhabricatorInlineCommentContentState $state) {
+
     $storage = $this->getStorageObject();
     $storage_map = $state->newStorageMap();
-    $storage->setAttribute('inline.state', $storage_map);
+    $storage->setAttribute($key, $storage_map);
-
-    $this->setContent($state->getContentText());
 
     return $this;
   }
@@ -364,6 +400,55 @@ abstract class PhabricatorInlineComment
     return $this->getStorageObject()->getInlineContext();
   }
 
+  public function getContentStateMapForEdit(PhabricatorUser $viewer) {
+    return $this->getWireContentStateMap(true, $viewer);
+  }
+
+  public function getContentStateMap() {
+    return $this->getWireContentStateMap(false, null);
+  }
+
+  private function getWireContentStateMap(
+    $is_edit,
+    PhabricatorUser $viewer = null) {
+
+    $initial_state = $this->getInitialContentState();
+    $committed_state = $this->getCommittedContentState();
+
+    if ($is_edit) {
+      $active_state = $this->getContentStateForEdit($viewer);
+    } else {
+      $active_state = $this->getContentState();
+    }
+
+    return array(
+      'initial' => $this->getWireContentState($initial_state),
+      'committed' => $this->getWireContentState($committed_state),
+      'active' => $this->getWireContentState($active_state),
+    );
+  }
+
+  private function getWireContentState($content_state) {
+    if ($content_state === null) {
+      return null;
+    }
+
+    return $content_state->newStorageMap();
+  }
+
+  public function getDefaultSuggestionText() {
+    $context = $this->getInlineContext();
+
+    if (!$context) {
+      return null;
+    }
+
+    $default = $context->getBodyLines();
+    $default = implode('', $default);
+
+    return $default;
+  }
+
 
 /* -(  PhabricatorMarkupInterface Implementation  )-------------------------- */
 

src/infrastructure/diff/view/PHUIDiffInlineCommentEditView.php

@@ -114,9 +114,6 @@ final class PHUIDiffInlineCommentEditView
     $main = $state->getContentSuggestionText();
     $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
     // any actual leading newlines in the content are preserved.
     $main = "\n".$main;
@@ -127,9 +124,6 @@ final class PHUIDiffInlineCommentEditView
         'class' => 'inline-suggestion-input PhabricatorMonospaced',
         'rows' => max(3, $main_count + 1),
         'sigil' => 'inline-content-suggestion',
-        'meta' => array(
-          'defaultText' => $default,
-        ),
       ),
       $main);
 

src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php

@@ -10,6 +10,16 @@
 abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
 
   private $views = array();
+  private $isUndoTemplate;
+
+  final public function setIsUndoTemplate($is_undo_template) {
+    $this->isUndoTemplate = $is_undo_template;
+    return $this;
+  }
+
+  final public function getIsUndoTemplate() {
+    return $this->isUndoTemplate;
+  }
 
   public function getInlineViews() {
     return $this->views;
@@ -21,13 +31,30 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
   }
 
   protected function getRowAttributes() {
+    $is_undo_template = $this->getIsUndoTemplate();
+
     $is_hidden = false;
+    if ($is_undo_template) {
+
+      // NOTE: When this scaffold is turned into an "undo" template, it is
+      // important it not have any metadata: the metadata reference will be
+      // copied to each instance of the row. This is a complicated mess; for
+      // now, just sneak by without generating metadata when rendering undo
+      // templates.
+
+      $metadata = null;
+    } else {
       foreach ($this->getInlineViews() as $view) {
         if ($view->isHidden()) {
           $is_hidden = true;
         }
       }
 
+      $metadata = array(
+        'hidden' => $is_hidden,
+      );
+    }
+
     $classes = array();
     $classes[] = 'inline';
     if ($is_hidden) {
@@ -37,9 +64,7 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
     $result = array(
       'class' => implode(' ', $classes),
       'sigil' => 'inline-row',
-      'meta' => array(
+      'meta' => $metadata,
-        'hidden' => $is_hidden,
-      ),
     );
 
     return $result;

src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php

@@ -27,7 +27,7 @@ final class PHUIDiffInlineCommentUndoView
       array(
         'class' => 'differential-inline-undo',
       ),
-      array(pht('Changes discarded. '), $link));
+      array(pht('Changes discarded.'), ' ', $link));
   }
 
 }

src/infrastructure/diff/view/PHUIDiffInlineCommentView.php

@@ -93,7 +93,7 @@ abstract class PHUIDiffInlineCommentView extends AphrontView {
       'startOffset' => $inline->getStartOffset(),
       'endOffset' => $inline->getEndOffset(),
       'on_right' => $this->getIsOnRight(),
-      'contentState' => $inline->getContentState()->newStorageMap(),
+      'state' => $inline->getContentStateMap(),
     );
   }
 

webroot/rsrc/externals/javelin/lib/Resource.js

@@ -149,6 +149,10 @@ JX.install('Resource', {
         }
       }
 
+      for (var jj = 0; jj < errors.length; jj++) {
+        JX.log(errors[jj]);
+      }
+
       if (errors.length) {
         throw errors[0];
       }

webroot/rsrc/js/application/diff/DiffChangeset.js

@@ -671,9 +671,6 @@ JX.install('DiffChangeset', {
       // Code shared by autoload and context responses.
 
       this._loadChangesetState(response);
-
-      JX.Stratcom.invoke('differential-inline-comment-refresh');
-
       this._rebuildAllInlines();
 
       JX.Stratcom.invoke('resize');
@@ -846,9 +843,7 @@ JX.install('DiffChangeset', {
     },
 
     _rebuildAllInlines: function() {
-      if (this._inlines === null) {
       this._inlines = [];
-      }
 
       var rows = JX.DOM.scry(this._node, 'tr');
       var ii;

webroot/rsrc/js/application/diff/DiffInline.js

@@ -1,12 +1,14 @@
 /**
  * @provides phabricator-diff-inline
  * @requires javelin-dom
+ *           phabricator-diff-inline-content-state
  * @javelin
  */
 
 JX.install('DiffInline', {
 
   construct : function() {
+    this._state = {};
   },
 
   members: {
@@ -19,7 +21,6 @@ JX.install('DiffInline', {
     _displaySide: null,
     _isNewFile: null,
     _replyToCommentPHID: null,
-    _originalState: null,
     _snippet: null,
     _menuItems: null,
     _documentEngineKey: null,
@@ -53,6 +54,8 @@ JX.install('DiffInline', {
     _isSelected: false,
     _canSuggestEdit: false,
 
+    _state: null,
+
     bindToRow: function(row) {
       this._row = row;
 
@@ -323,8 +326,6 @@ JX.install('DiffInline', {
       this._phid = null;
       this._isCollapsed = false;
 
-      this._originalState = null;
-
       return row;
     },
 
@@ -412,25 +413,12 @@ JX.install('DiffInline', {
         .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 content_state = this._newContentState();
       if (with_quote) {
-        var text = this._getContentState().text;
+        var text = this._getActiveContentState().getTextForQuote();
-        text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n';
         content_state.text = text;
       }
 
@@ -455,21 +443,12 @@ JX.install('DiffInline', {
         this._undoText = null;
       }
 
-      var uri = this._getInlineURI();
+      this._applyEdit(content_state);
-      var handler = JX.bind(this, this._oneditresponse);
-
-      var data = this._newRequestData('edit', content_state);
-
-      this.setLoading(true);
-
-      new JX.Request(uri, handler)
-        .setData(data)
-        .send();
     },
 
     delete: function(is_ref) {
       var uri = this._getInlineURI();
-      var handler = JX.bind(this, this._ondeleteresponse);
+      var handler = JX.bind(this, this._ondeleteresponse, false);
 
       // NOTE: This may be a direct delete (the user clicked on the inline
       // itself) or a "refdelete" (the user clicked somewhere else, like the
@@ -589,7 +568,6 @@ JX.install('DiffInline', {
       this._readInlineState(response.inline);
       this._drawEditRows(rows);
 
-      this.setLoading(false);
       this.setInvisible(true);
     },
 
@@ -602,11 +580,26 @@ JX.install('DiffInline', {
 
     _readInlineState: function(state) {
       this._id = state.id;
-      this._originalState = state.contentState;
+
+      this._state = {
+        initial: this._newContentStateFromWireFormat(state.state.initial),
+        committed: this._newContentStateFromWireFormat(state.state.committed),
+        active: this._newContentStateFromWireFormat(state.state.active)
+      };
+
       this._canSuggestEdit = state.canSuggestEdit;
     },
 
-    _ondeleteresponse: function() {
+    _newContentStateFromWireFormat: function(map) {
+      if (map === null) {
+        return null;
+      }
+
+      return new JX.DiffInlineContentState().readWireFormat(map);
+    },
+
+    _ondeleteresponse: function(prevent_undo) {
+      if (!prevent_undo) {
         // If there's an existing "unedit" undo element, remove it.
         if (this._undoRow) {
           JX.DOM.remove(this._undoRow);
@@ -618,12 +611,13 @@ JX.install('DiffInline', {
         // read and preserve the text so "Undo" restores it.
         var state = null;
         if (this._editRow) {
-        state = this._readFormState(this._editRow);
+          state = this._getActiveContentState().getWireFormat();
           JX.DOM.remove(this._editRow);
           this._editRow = null;
         }
 
         this._drawUndeleteRows(state);
+      }
 
       this.setLoading(false);
       this.setDeleted(true);
@@ -668,7 +662,10 @@ JX.install('DiffInline', {
       this._editRow = this._drawRows(rows, null, 'edit');
 
       this._drawSuggestionState(this._editRow);
-      this.setHasSuggestion(this._originalState.hasSuggestion);
+
+      // TODO: We're just doing this for the rendering side effect of drawing
+      // the button text.
+      this.setHasSuggestion(this.getHasSuggestion());
     },
 
     _drawRows: function(rows, cursor, type) {
@@ -677,7 +674,6 @@ JX.install('DiffInline', {
       var anchor = cursor || this._row;
       cursor = cursor || this._row.nextSibling;
 
-
       var result_row;
       var next_row;
       while (row) {
@@ -762,31 +758,41 @@ JX.install('DiffInline', {
 
       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._editRow) {
           var node = this._getSuggestionNode(this._editRow);
           if (node) {
-            if (!node.value.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);
           }
         }
       }
-        }
-      }
 
       // Save the "hasSuggestion" part of the content state.
       this.triggerDraft();
     },
 
+    _getActiveContentState: function() {
+      var state = this._state.active;
+
+      if (this._editRow) {
+        state.readForm(this._editRow);
+      }
+
+      return state;
+    },
+
+    _getCommittedContentState: function() {
+      return this._state.committed;
+    },
+
+    _getInitialContentState: function() {
+      return this._state.initial;
+    },
+
     setHasSuggestion: function(has_suggestion) {
-      this._hasSuggestion = has_suggestion;
+      var state = this._getActiveContentState();
+      state.setHasSuggestion(has_suggestion);
 
       var button = this._getSuggestionButton();
       var pht = this.getChangeset().getChangesetList().getTranslations();
@@ -806,20 +812,121 @@ JX.install('DiffInline', {
     },
 
     getHasSuggestion: function() {
-      return this._hasSuggestion;
+      return this._getActiveContentState().getHasSuggestion();
     },
 
     save: function() {
-      var handler = JX.bind(this, this._onsubmitresponse);
+      if (this._shouldDeleteOnSave()) {
+        JX.DOM.remove(this._editRow);
+        this._editRow = null;
+
+        this._applyDelete(true);
+        return;
+      }
+
+      this._applySave();
+    },
+
+    _shouldDeleteOnSave: function() {
+      var active = this._getActiveContentState();
+      var initial = this._getInitialContentState();
+
+      // When a user clicks "Save", it counts as a "delete" if the content
+      // of the comment is functionally empty.
+
+      // This isn't a delete if there's any text. Even if the text is a
+      // quote (so the state is the same as the initial state), we preserve
+      // it when the user clicks "Save".
+      if (!active.isTextEmpty()) {
+        return false;
+      }
+
+      // This isn't a delete if there's a suggestion and that suggestion is
+      // different from the initial state. (This means that an inline which
+      // purely suggests a block of code should be deleted is non-empty.)
+      if (active.getHasSuggestion()) {
+        if (!active.isSuggestionSimilar(initial)) {
+          return false;
+        }
+      }
+
+      // Otherwise, this comment is functionally empty, so we can just treat
+      // a "Save" as a "delete".
+      return true;
+    },
+
+    _shouldUndoOnCancel: function() {
+      var committed = this._getCommittedContentState();
+      var active = this._getActiveContentState();
+      var initial = this._getInitialContentState();
+
+      // When a user clicks "Cancel", we only offer to let them "Undo" the
+      // action if the undo would be substantive.
+
+      // The undo is substantive if the text is nonempty, and not similar to
+      // the last state.
+      var versus = committed || initial;
+      if (!active.isTextEmpty() && !active.isTextSimilar(versus)) {
+        return true;
+      }
+
+      // The undo is substantive if there's a suggestion, and the suggestion
+      // is not similar to the last state.
+      if (active.getHasSuggestion()) {
+        if (!active.isSuggestionSimilar(versus)) {
+          return true;
+        }
+      }
+
+      return false;
+    },
+
+    _applySave: function() {
+      var handler = JX.bind(this, this._onsaveresponse);
+
+      var state = this._getActiveContentState();
+      var data = this._newRequestData('save', state.getWireFormat());
+
+      this._applyCall(handler, data);
+    },
+
+    _applyDelete: function(prevent_undo) {
+      var handler = JX.bind(this, this._ondeleteresponse, prevent_undo);
+
+      var data = this._newRequestData('delete');
+
+      this._applyCall(handler, data);
+    },
+
+    _applyCancel: function(state) {
+      var handler = JX.bind(this, this._onCancelResponse);
+
+      var data = this._newRequestData('cancel', state);
+
+      this._applyCall(handler, data);
+    },
+
+    _applyEdit: function(state) {
+      var handler = JX.bind(this, this._oneditresponse);
+
+      var data = this._newRequestData('edit', state);
+
+      this._applyCall(handler, data);
+    },
+
+    _applyCall: function(handler, data) {
+      var uri = this._getInlineURI();
+
+      var callback = JX.bind(this, function() {
+        this.setLoading(false);
+        handler.apply(null, arguments);
+      });
 
       this.setLoading(true);
 
-      var uri = this._getInlineURI();
+      new JX.Workflow(uri, data)
-      var data = this._newRequestData('save', this._getContentState());
+        .setHandler(callback)
-
+        .start();
-      new JX.Request(uri, handler)
-        .setData(data)
-        .send();
     },
 
     undo: function() {
@@ -850,79 +957,45 @@ JX.install('DiffInline', {
     },
 
     cancel: function() {
-      var state = this._readFormState(this._editRow);
+      // NOTE: Read the state before we remove the editor. Otherwise, we might
+      // miss text the user has entered into the textarea.
+      var state = this._getActiveContentState().getWireFormat();
 
       JX.DOM.remove(this._editRow);
       this._editRow = null;
 
-      var is_empty = this._isVoidContentState(state);
+      // When a user clicks "Cancel", we delete the comment if it has never
-      var is_same = this._isSameContentState(state, this._originalState);
+      // been saved: we don't have a non-empty display state to revert to.
-      if (!is_empty && !is_same) {
+      var is_delete = (this._getCommittedContentState() === null);
-        this._drawUneditRows(state);
-      }
 
-      // If this was an empty box and we typed some text and then hit cancel,
+      var is_undo = this._shouldUndoOnCancel();
-      // don't show the empty concrete inline.
-      if (this._isVoidContentState(this._originalState)) {
-        this.setInvisible(true);
-      } else {
-        this.setInvisible(false);
-      }
 
       // If you "undo" to restore text ("AB") and then "Cancel", we put you
       // back in the original text state ("A"). We also send the original
       // text ("A") to the server as the current persistent state.
 
-      var uri = this._getInlineURI();
+      if (is_undo) {
-      var data = this._newRequestData('cancel', this._originalState);
+        this._drawUneditRows(state);
-      var handler = JX.bind(this, this._onCancelResponse);
+      }
 
-      this.setLoading(true);
+      if (is_delete) {
+        // NOTE: We're always suppressing the undo from "delete". We want to
+        // use the "undo" we just added above instead, which will get us
+        // back to the ephemeral, client-side editor state.
+        this._applyDelete(true);
+      } else {
+        this.setEditing(false);
+        this.setInvisible(false);
 
-      new JX.Request(uri, handler)
+        var old_state = this._getCommittedContentState();
-        .setData(data)
+        this._applyCancel(old_state.getWireFormat());
-        .send();
 
         this._didUpdate(true);
+      }
     },
 
     _onCancelResponse: function(response) {
-      this.setEditing(false);
+      // Nothing to do.
-      this.setLoading(false);
-
-      // 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._isVoidContentState(this._originalState) && !this.isUndo()) {
-        this.setDeleted(true);
-
-        JX.DOM.remove(this._row);
-        this._row = null;
-
-        this._didUpdate();
-      }
-    },
-
-    _readFormState: function(row) {
-      var state = this._newContentState();
-
-      var node;
-
-      try {
-        node = JX.DOM.find(row, 'textarea', 'inline-content-text');
-        state.text = node.value;
-      } catch (ex) {
-        // Ignore.
-      }
-
-      node = this._getSuggestionNode(row);
-      if (node) {
-        state.suggestionText = node.value;
-      }
-
-      state.hasSuggestion = this.getHasSuggestion();
-
-      return state;
     },
 
     _getSuggestionNode: function(row) {
@@ -933,40 +1006,18 @@ JX.install('DiffInline', {
       }
     },
 
-    _onsubmitresponse: function(response) {
+    _onsaveresponse: function(response) {
       if (this._editRow) {
         JX.DOM.remove(this._editRow);
         this._editRow = null;
       }
 
-      this.setLoading(false);
-      this.setInvisible(false);
       this.setEditing(false);
+      this.setInvisible(false);
 
-      this._onupdate(response);
+      var new_row = this._drawContentRows(JX.$H(response.view).getNode());
-    },
-
-    _onupdate: function(response) {
-      var new_row;
-      if (response.view) {
-        new_row = this._drawContentRows(JX.$H(response.view).getNode());
-      }
-
-      // TODO: Save the old row so the action it's undo-able if it was a
-      // delete.
-      var remove_old = true;
-      if (remove_old) {
       JX.DOM.remove(this._row);
-      }
-
-      // If you delete the content on a comment and save it, it acts like a
-      // delete: the server does not return a new row.
-      if (new_row) {
       this.bindToRow(new_row);
-      } else {
-        this.setDeleted(true);
-        this._row = null;
-      }
 
       this._didUpdate();
     },
@@ -1037,8 +1088,8 @@ JX.install('DiffInline', {
         return null;
       }
 
-      var state = this._readFormState(this._editRow);
+      var state = this._getActiveContentState();
-      if (this._isVoidContentState(state)) {
+      if (state.isStateEmpty()) {
         return null;
       }
 
@@ -1047,7 +1098,7 @@ JX.install('DiffInline', {
         id: this.getID(),
       };
 
-      JX.copy(draft_data, state);
+      JX.copy(draft_data, state.getWireFormat());
 
       return draft_data;
     },
@@ -1163,19 +1214,8 @@ JX.install('DiffInline', {
         suggestionText: '',
         hasSuggestion: false
       };
-    },
-
-    _isVoidContentState: function(state) {
-      return (!state.text.length && !state.suggestionText.length);
-    },
-
-    _isSameContentState: function(u, v) {
-      return (
-        ((u === null) === (v === null)) &&
-        (u.text === v.text) &&
-        (u.suggestionText === v.suggestionText) &&
-        (u.hasSuggestion === v.hasSuggestion));
     }
+
   }
 
 });

webroot/rsrc/js/application/diff/DiffInlineContentState.js

@@ -0,0 +1,137 @@
+/**
+ * @provides phabricator-diff-inline-content-state
+ * @requires javelin-dom
+ * @javelin
+ */
+
+JX.install('DiffInlineContentState', {
+
+  construct : function() {
+
+  },
+
+  properties: {
+    text: null,
+    suggestionText: null,
+    hasSuggestion: false
+  },
+
+  members: {
+    readForm: function(row) {
+      var node;
+
+      try {
+        node = JX.DOM.find(row, 'textarea', 'inline-content-text');
+        this.setText(node.value);
+      } catch (ex) {
+        this.setText(null);
+      }
+
+      node = this._getSuggestionNode(row);
+      if (node) {
+        this.setSuggestionText(node.value);
+      } else {
+        this.setSuggestionText(null);
+      }
+
+      return this;
+    },
+
+    getWireFormat: function() {
+      return {
+        text: this.getText(),
+        suggestionText: this.getSuggestionText(),
+        hasSuggestion: this.getHasSuggestion()
+      };
+    },
+
+    readWireFormat: function(map) {
+      this.setText(map.text || null);
+      this.setSuggestionText(map.suggestionText || null);
+      this.setHasSuggestion(!!map.hasSuggestion);
+
+      return this;
+    },
+
+    getTextForQuote: function() {
+      var text = this.getText();
+      text = '> ' + text.replace(/\n/g, '\n> ') + '\n\n';
+      return text;
+    },
+
+    isStateEmpty: function() {
+      return (this.isTextEmpty() && this.isSuggestionEmpty());
+    },
+
+    isTextEmpty: function() {
+      var text = this.getText();
+      if (text === null) {
+        return true;
+      }
+
+      if (this._isStringSimilar(text, '')) {
+        return true;
+      }
+
+      return false;
+    },
+
+    isSuggestionEmpty: function() {
+      if (!this.getHasSuggestion()) {
+        return true;
+      }
+
+      var suggestion = this.getSuggestionText();
+      if (suggestion === null) {
+        return true;
+      }
+
+      if (this._isStringSimilar(suggestion, '')) {
+        return true;
+      }
+
+      return false;
+    },
+
+    isTextSimilar: function(v) {
+      if (!v) {
+        return false;
+      }
+
+      var us = this.getText();
+      var vs = v.getText();
+
+      return this._isStringSimilar(us, vs);
+    },
+
+    isSuggestionSimilar: function(v) {
+      // If we don't have a comparison state, treat them as dissimilar. This
+      // is expected to occur in old inline comments that did not save an
+      // initial state.
+
+      if (!v) {
+        return false;
+      }
+
+      var us = this.getSuggestionText();
+      var vs = v.getSuggestionText();
+
+      return this._isStringSimilar(us, vs);
+    },
+
+    _isStringSimilar: function(u, v) {
+      u = u || '';
+      v = v || '';
+      return (u === v);
+    },
+
+    _getSuggestionNode: function(row) {
+      try {
+        return JX.DOM.find(row, 'textarea', 'inline-content-suggestion');
+      } catch (ex) {
+        return null;
+      }
+    }
+  }
+
+});