From c03297ab5a0aed1fc0823d92014ec28aeaaf04b7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 25 Mar 2015 07:14:12 -0700 Subject: [PATCH] Fix interaction between undo and inline comment placement Summary: Fixes T7658. Currently, we remove the "undo" before placing the comment, but that causes us to lose track of which row we should be examining. Instead, place the comment first, then remove the "undo". Test Plan: This stuff is hard to test comprehensively, but the original report reproduced easily and is now fixed. I wasn't able to break anything by adding/editing/deleting comments. Reviewers: btrahan, chad Reviewed By: chad Subscribers: epriestley Maniphest Tasks: T7658 Differential Revision: https://secure.phabricator.com/D12157 --- resources/celerity/map.php | 22 +++++++++---------- .../DifferentialInlineCommentEditor.js | 12 +++++----- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 4b71eebbf3..11764e1679 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -11,7 +11,7 @@ return array( 'core.pkg.js' => '75599122', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => '686ac058', - 'differential.pkg.js' => 'e1dd7634', + 'differential.pkg.js' => 'a7a75fd2', 'diffusion.pkg.css' => '591664fa', 'diffusion.pkg.js' => 'bfc0737b', 'maniphest.pkg.css' => '68d4dd3d', @@ -364,7 +364,7 @@ return array( 'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375', 'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63', 'rsrc/js/application/differential/ChangesetViewManager.js' => '58562350', - 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'f2431bc1', + 'rsrc/js/application/differential/DifferentialInlineCommentEditor.js' => 'cbaf4413', 'rsrc/js/application/differential/behavior-add-reviewers-and-ccs.js' => 'e10f8e18', 'rsrc/js/application/differential/behavior-comment-jump.js' => '4fdb476d', 'rsrc/js/application/differential/behavior-comment-preview.js' => '8e1389b5', @@ -522,7 +522,7 @@ return array( 'conpherence-widget-pane-css' => '9199d87c', 'differential-changeset-view-css' => '79c27a4c', 'differential-core-view-css' => '7ac3cabc', - 'differential-inline-comment-editor' => 'f2431bc1', + 'differential-inline-comment-editor' => 'cbaf4413', 'differential-results-table-css' => '181aa9d9', 'differential-revision-add-comment-css' => 'c478bcaa', 'differential-revision-comment-css' => '48186045', @@ -1758,6 +1758,14 @@ return array( 'javelin-stratcom', 'phabricator-phtize', ), + 'cbaf4413' => array( + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-install', + 'javelin-request', + 'javelin-workflow', + ), 'd19198c8' => array( 'javelin-install', 'javelin-dom', @@ -1905,14 +1913,6 @@ return array( 'javelin-install', 'javelin-util', ), - 'f2431bc1' => array( - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-install', - 'javelin-request', - 'javelin-workflow', - ), 'f24f3253' => array( 'javelin-behavior', 'javelin-dom', diff --git a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js index d0c60f62fb..b37a4bca2d 100644 --- a/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js +++ b/webroot/rsrc/js/application/differential/DifferentialInlineCommentEditor.js @@ -158,6 +158,12 @@ JX.install('DifferentialInlineCommentEditor', { _didCompleteWorkflow : function(response) { var op = this.getOperation(); + // We don't get any markup back if the user deletes a comment, or saves + // an empty comment (which effects a delete). + if (response.markup) { + this._draw(JX.$H(response.markup).getNode()); + } + if (op == 'delete' || op == 'refdelete') { this._undoText = null; this._drawUndo(); @@ -165,12 +171,6 @@ JX.install('DifferentialInlineCommentEditor', { this._removeUndoLink(); } - // We don't get any markup back if the user deletes a comment, or saves - // an empty comment (which effects a delete). - if (response.markup) { - this._draw(JX.$H(response.markup).getNode()); - } - // These operations remove the old row (edit adds a new row first). var remove_old = (op == 'edit' || op == 'delete' || op == 'refdelete'); if (remove_old) {