Remove ajaxey comment magic in Pholio, and also some bugs

Summary:
Fixes T5424.

  - One concrete issue: drafts were not being cleared properly because `__draft__` was not set on submission. This (mostly) fixes phantom drafts.
  - This ajax comment magic feels weird and floaty and generally has problems. For example, if you add subscribers or inlines, all the stuff on the page which represents those won't update automatically. Instead, just reload. Maybe we'll ajax this stuff some day, but it feels like a net negative for now.
  - Also remove it from other applications where it's currently used.
  - Fix an issue with inline previews.

Test Plan: Made some comments on a mock, everything worked normally like I expected it to.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5424

Differential Revision: https://secure.phabricator.com/D9649

resources/celerity/map.php

@@ -8,7 +8,7 @@ return array(
   'names' =>
   array(
     'core.pkg.css' => '8feeb38a',
-    'core.pkg.js' => '07b01d4f',
+    'core.pkg.js' => 'b440d8d7',
     'darkconsole.pkg.js' => 'ca8671ce',
     'differential.pkg.css' => '4a93db37',
     'differential.pkg.js' => '5b252007',
@@ -418,8 +418,8 @@ return array(
     'rsrc/js/application/repository/repository-crossreference.js' => '8ab282be',
     'rsrc/js/application/search/behavior-reorder-queries.js' => '37871df4',
     'rsrc/js/application/slowvote/behavior-slowvote-embed.js' => 'a51fdb2e',
-    'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => '9084a36f',
+    'rsrc/js/application/transactions/behavior-transaction-comment-form.js' => '9f7309fb',
-    'rsrc/js/application/transactions/behavior-transaction-list.js' => 'cf656c84',
+    'rsrc/js/application/transactions/behavior-transaction-list.js' => 'fd6c2b32',
     'rsrc/js/application/uiexample/JavelinViewExample.js' => 'd4a14807',
     'rsrc/js/application/uiexample/ReactorButtonExample.js' => '44524435',
     'rsrc/js/application/uiexample/ReactorCheckboxExample.js' => '7ba325ee',
@@ -621,8 +621,8 @@ return array(
     'javelin-behavior-phabricator-search-typeahead' => 'fbeabd1e',
     'javelin-behavior-phabricator-show-all-transactions' => '7c273581',
     'javelin-behavior-phabricator-tooltips' => '48db4145',
-    'javelin-behavior-phabricator-transaction-comment-form' => '9084a36f',
+    'javelin-behavior-phabricator-transaction-comment-form' => '9f7309fb',
-    'javelin-behavior-phabricator-transaction-list' => 'cf656c84',
+    'javelin-behavior-phabricator-transaction-list' => 'fd6c2b32',
     'javelin-behavior-phabricator-watch-anchor' => '06e05112',
     'javelin-behavior-phame-post-preview' => '61d927ec',
     'javelin-behavior-pholio-mock-edit' => '1e1e8bb0',
@@ -1520,15 +1520,6 @@ return array(
       3 => 'javelin-mask',
       4 => 'phabricator-drag-and-drop-file-upload',
     ),
-    '9084a36f' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-dom',
-      2 => 'javelin-util',
-      3 => 'javelin-fx',
-      4 => 'javelin-request',
-      5 => 'phabricator-shaped-request',
-    ),
     '929d95eb' =>
     array(
       0 => 'javelin-behavior',
@@ -1595,6 +1586,14 @@ return array(
       2 => 'javelin-uri',
       3 => 'javelin-request',
     ),
+    '9f7309fb' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-dom',
+      2 => 'javelin-util',
+      3 => 'javelin-request',
+      4 => 'phabricator-shaped-request',
+    ),
     'a3e2244e' =>
     array(
       0 => 'javelin-behavior',
@@ -1851,16 +1850,6 @@ return array(
       0 => 'javelin-install',
       1 => 'javelin-typeahead-source',
     ),
-    'cf656c84' =>
-    array(
-      0 => 'javelin-behavior',
-      1 => 'javelin-stratcom',
-      2 => 'javelin-workflow',
-      3 => 'javelin-dom',
-      4 => 'javelin-fx',
-      5 => 'javelin-uri',
-      6 => 'phabricator-textareautils',
-    ),
     'cf76cfd5' =>
     array(
       0 => 'javelin-behavior',
@@ -2093,6 +2082,15 @@ return array(
       5 => 'javelin-util',
       6 => 'javelin-stratcom',
     ),
+    'fd6c2b32' =>
+    array(
+      0 => 'javelin-behavior',
+      1 => 'javelin-stratcom',
+      2 => 'javelin-workflow',
+      3 => 'javelin-dom',
+      4 => 'javelin-uri',
+      5 => 'phabricator-textareautils',
+    ),
     'fe2e0ba4' =>
     array(
       0 => 'javelin-behavior',

src/applications/files/controller/PhabricatorFileCommentController.php

@@ -58,7 +58,7 @@ final class PhabricatorFileCommentController
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)

src/applications/legalpad/controller/LegalpadDocumentCommentController.php

@@ -68,7 +68,7 @@ final class LegalpadDocumentCommentController extends LegalpadController {
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)

src/applications/macro/controller/PhabricatorMacroCommentController.php

@@ -55,7 +55,7 @@ final class PhabricatorMacroCommentController
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)

src/applications/paste/controller/PhabricatorPasteCommentController.php

@@ -58,7 +58,7 @@ final class PhabricatorPasteCommentController
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)

src/applications/pholio/controller/PholioMockCommentController.php

@@ -76,10 +76,14 @@ final class PholioMockCommentController extends PholioController {
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
+      $xaction_view = id(new PholioTransactionView())
+        ->setMock($mock);
+
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)
+        ->setTransactionView($xaction_view)
         ->setIsPreview($is_preview)
         ->setAnchorOffset($request->getStr('anchor'));
     } else {

src/applications/ponder/controller/PonderAnswerCommentController.php

@@ -56,7 +56,7 @@ final class PonderAnswerCommentController extends PonderController {
 //      $draft->replaceOrDelete();
 //    }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($viewer)
         ->setTransactions($xactions)

src/applications/ponder/controller/PonderQuestionCommentController.php

@@ -55,7 +55,7 @@ final class PonderQuestionCommentController extends PonderController {
 //      $draft->replaceOrDelete();
 //    }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($viewer)
         ->setTransactions($xactions)

src/applications/releeph/controller/request/ReleephRequestCommentController.php

@@ -55,7 +55,7 @@ final class ReleephRequestCommentController
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($viewer)
         ->setTransactions($xactions)

src/applications/slowvote/controller/PhabricatorSlowvoteCommentController.php

@@ -55,7 +55,7 @@ final class PhabricatorSlowvoteCommentController
       $draft->replaceOrDelete();
     }
 
-    if ($request->isAjax()) {
+    if ($request->isAjax() && $is_preview) {
       return id(new PhabricatorApplicationTransactionResponse())
         ->setViewer($user)
         ->setTransactions($xactions)

src/applications/transactions/response/PhabricatorApplicationTransactionResponse.php

@@ -7,6 +7,16 @@ final class PhabricatorApplicationTransactionResponse
   private $transactions;
   private $anchorOffset;
   private $isPreview;
+  private $transactionView;
+
+  public function setTransactionView($transaction_view) {
+    $this->transactionView = $transaction_view;
+    return $this;
+  }
+
+  public function getTransactionView() {
+    return $this->transactionView;
+  }
 
   protected function buildProxy() {
     return new AphrontAjaxResponse();
@@ -47,7 +57,9 @@ final class PhabricatorApplicationTransactionResponse
   }
 
   public function reduceProxyResponse() {
-    if ($this->getTransactions()) {
+    if ($this->transactionView) {
+      $view = $this->transactionView;
+    } else if ($this->getTransactions()) {
       $view = head($this->getTransactions())
         ->getApplicationTransactionViewObject();
     } else {

src/applications/transactions/view/PhabricatorApplicationTransactionCommentView.php

@@ -124,9 +124,6 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
         'showPreview'   => $this->getShowPreview(),
 
         'actionURI'     => $this->getAction(),
-        'draftKey'      => $this->getDraft()
-          ? $this->getDraft()->getDraftKey()
-          : null,
       ));
 
     $comment_box = id(new PHUIObjectBoxView())
@@ -146,8 +143,10 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
       '');
 
     $draft_comment = '';
+    $draft_key = null;
     if ($this->getDraft()) {
       $draft_comment = $this->getDraft()->getDraft();
+      $draft_key = $this->getDraft()->getDraftKey();
     }
 
     if (!$this->getObjectPHID()) {
@@ -164,6 +163,7 @@ class PhabricatorApplicationTransactionCommentView extends AphrontView {
         ))
       ->setAction($this->getAction())
       ->setID($this->getFormID())
+      ->addHiddenInput('__draft__', $draft_key)
       ->appendChild(
         id(new PhabricatorRemarkupControl())
           ->setID($this->getCommentID())

webroot/rsrc/js/application/transactions/behavior-transaction-comment-form.js

@@ -3,7 +3,6 @@
  * @requires javelin-behavior
  *           javelin-dom
  *           javelin-util
- *           javelin-fx
  *           javelin-request
  *           phabricator-shaped-request
  */
@@ -12,36 +11,9 @@ JX.behavior('phabricator-transaction-comment-form', function(config) {
 
   var form = JX.$(config.formID);
 
-  JX.DOM.listen(form, 'willSubmit', null, function (e) {
-    e.kill();
-    if (config.showPreview) {
-      var preview = JX.$(config.panelID);
-      preview.style.opacity = 0.5;
-    }
-  });
-
-  JX.DOM.listen(form, 'willClear', null, function(e) {
-    JX.$(config.commentID).value = '';
-
-    if (config.showPreview) {
-      var preview = JX.$(config.panelID);
-      new JX.FX(preview)
-        .setDuration(500)
-        .then(function () {
-          new JX.FX(preview).setDuration(1000).start({opacity: [0, 1]});
-        })
-        .start({opacity: [0.5, 0]});
-    }
-  });
-
   var getdata = function() {
     var obj = JX.DOM.convertFormToDictionary(form);
     obj.__preview__ = 1;
-
-    if (config.draftKey) {
-      obj.__draft__ = config.draftKey;
-    }
-
     return obj;
   };
 
@@ -70,8 +42,8 @@ JX.behavior('phabricator-transaction-comment-form', function(config) {
     JX.DOM.listen(form, 'keydown', null, trigger);
     var always_trigger = function() {
       new JX.Request(config.actionURI, onresponse)
-            .setData(getdata())
+        .setData(getdata())
-            .send();
+        .send();
     };
     JX.DOM.listen(form, 'shouldRefresh', null, always_trigger);
 

webroot/rsrc/js/application/transactions/behavior-transaction-list.js

@@ -4,7 +4,6 @@
  *           javelin-stratcom
  *           javelin-workflow
  *           javelin-dom
- *           javelin-fx
  *           javelin-uri
  *           phabricator-textareautils
  */
@@ -15,55 +14,6 @@ JX.behavior('phabricator-transaction-list', function(config) {
   var xaction_nodes = null;
   var next_anchor = config.nextAnchor;
 
-  function get_xaction_nodes() {
-    if (xaction_nodes === null) {
-      xaction_nodes = {};
-      var xactions = JX.DOM.scry(list, 'div', 'transaction');
-      for (var ii = 0; ii < xactions.length; ii++) {
-        xaction_nodes[JX.Stratcom.getData(xactions[ii]).phid] = xactions[ii];
-      }
-    }
-    return xaction_nodes;
-  }
-
-  function ontransactions(response) {
-    var fade_in = [];
-    var first_new = null;
-
-    var nodes = get_xaction_nodes();
-    for (var phid in response.xactions) {
-      var new_node = JX.$H(response.xactions[phid]).getFragment().firstChild;
-      fade_in.push(new_node);
-
-      if (nodes[phid]) {
-        JX.DOM.replace(nodes[phid], new_node);
-      } else {
-        if (first_new === null) {
-          first_new = new_node;
-        }
-        list.appendChild(new_node);
-
-        // Add a spacer after new transactions.
-        var spacer = JX.$H(response.spacer).getFragment().firstChild;
-        list.appendChild(spacer);
-        fade_in.push(spacer);
-
-        next_anchor++;
-      }
-      nodes[phid] = new_node;
-    }
-
-    // Scroll to the first new transaction, if transactions were added.
-    if (first_new) {
-      JX.DOM.scrollTo(first_new);
-    }
-
-    // Make any new or updated transactions fade in.
-    for (var ii = 0; ii < fade_in.length; ii++) {
-      new JX.FX(fade_in[ii]).setDuration(500).start({opacity: [0, 1]});
-    }
-  }
-
   JX.Stratcom.listen(
     'click',
     [['transaction-edit'], ['transaction-remove']],
@@ -120,61 +70,4 @@ JX.behavior('phabricator-transaction-list', function(config) {
         .start();
     });
 
-  JX.Stratcom.listen(
-    ['submit', 'didSyntheticSubmit'],
-    'transaction-append',
-    function(e) {
-      var form = e.getTarget();
-      if (JX.Stratcom.getData(form).objectPHID != config.objectPHID) {
-        // This indicates there are several forms on the page, and the user
-        // submitted a different one than the one we're in control of.
-        return;
-      }
-
-      e.kill();
-
-      JX.DOM.invoke(form, 'willSubmit');
-
-      JX.Workflow.newFromForm(form, { anchor : next_anchor })
-        .setHandler(function(response) {
-          ontransactions(response);
-
-          var e = JX.DOM.invoke(form, 'willClear');
-          if (!e.getPrevented()) {
-            var ii;
-            var textareas = JX.DOM.scry(form, 'textarea');
-            for (ii = 0; ii < textareas.length; ii++) {
-              textareas[ii].value = '';
-            }
-
-            var inputs = JX.DOM.scry(form, 'input');
-            for (ii = 0; ii < inputs.length; ii++) {
-            switch (inputs[ii].type) {
-              case 'password':
-              case 'text':
-                inputs[ii].value = '';
-                break;
-              case 'checkbox':
-              case 'radio':
-                inputs[ii].checked = false;
-                break;
-              }
-            }
-
-            var selects = JX.DOM.scry(form, 'select');
-            var jj;
-            for (ii = 0; ii < selects.length; ii++) {
-              if (selects[ii].type == 'select-one') {
-                selects[ii].selectedIndex = 0;
-              } else {
-               for (jj = 0; jj < selects[ii].options.length; jj++) {
-                 selects[ii].options[jj].selected = false;
-               }
-              }
-            }
-          }
-        })
-        .start();
-
-    });
 });