From b021da71a52ddd92ff47b08018f666ae4be47a20 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 14 May 2020 11:43:55 -0700
Subject: [PATCH] When users click headers to select diff UI elements, don't
 eat the events

Summary: Ref T13513. Currently, clicking inline or changeset headers eats the click events. This doesn't serve any clear purpose, and means these clicks do not clear text selections from the document, which is unusual.

Test Plan:
  - Selected some text in a diff.
  - Clicked a changeset header to select it.
  - Before patch: text selection and context menu were retained.
  - After patch: text selection was cleared and context menu was dismissed.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21255
---
 resources/celerity/map.php                    | 46 +++++++++----------
 .../rsrc/js/application/diff/DiffChangeset.js |  5 +-
 .../js/application/diff/DiffChangesetList.js  |  5 +-
 3 files changed, 29 insertions(+), 27 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index 91c1ab7247..7194b5c1ed 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -13,7 +13,7 @@ return array(
     'core.pkg.js' => '845355f4',
     'dark-console.pkg.js' => '187792c2',
     'differential.pkg.css' => 'b042ee8b',
-    'differential.pkg.js' => '5d560bda',
+    'differential.pkg.js' => 'e0600220',
     'diffusion.pkg.css' => '42c75c37',
     'diffusion.pkg.js' => 'a98c0bf7',
     'maniphest.pkg.css' => '35995d6d',
@@ -379,8 +379,8 @@ 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' => 'b6bb0240',
-    'rsrc/js/application/diff/DiffChangesetList.js' => '1e8658bb',
+    'rsrc/js/application/diff/DiffChangeset.js' => 'd721533b',
+    'rsrc/js/application/diff/DiffChangesetList.js' => '8b0eab21',
     'rsrc/js/application/diff/DiffInline.js' => '734d3c33',
     'rsrc/js/application/diff/DiffPathView.js' => '8207abf9',
     'rsrc/js/application/diff/DiffTreeView.js' => '5d83623b',
@@ -774,8 +774,8 @@ return array(
     'phabricator-darklog' => '3b869402',
     'phabricator-darkmessage' => '26cd4b73',
     'phabricator-dashboard-css' => '5a205b9d',
-    'phabricator-diff-changeset' => 'b6bb0240',
-    'phabricator-diff-changeset-list' => '1e8658bb',
+    'phabricator-diff-changeset' => 'd721533b',
+    'phabricator-diff-changeset-list' => '8b0eab21',
     'phabricator-diff-inline' => '734d3c33',
     'phabricator-diff-path-view' => '8207abf9',
     'phabricator-diff-tree-view' => '5d83623b',
@@ -1069,11 +1069,6 @@ return array(
       'javelin-behavior',
       'javelin-dom',
     ),
-    '1e8658bb' => array(
-      'javelin-install',
-      'phuix-button-view',
-      'phabricator-diff-tree-view',
-    ),
     '1ff278aa' => array(
       'phui-button-css',
     ),
@@ -1675,6 +1670,11 @@ return array(
       'javelin-dom',
       'phabricator-draggable-list',
     ),
+    '8b0eab21' => array(
+      'javelin-install',
+      'phuix-button-view',
+      'phabricator-diff-tree-view',
+    ),
     '8b5c7d65' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1979,19 +1979,6 @@ return array(
       'javelin-stratcom',
       'javelin-dom',
     ),
-    'b6bb0240' => 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',
-    ),
     'b7b73831' => array(
       'javelin-behavior',
       'javelin-dom',
@@ -2103,6 +2090,19 @@ return array(
     'd4cc2d2a' => array(
       'javelin-install',
     ),
+    'd721533b' => 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',
+    ),
     'd8a86cfb' => array(
       'javelin-behavior',
       'javelin-dom',
diff --git a/webroot/rsrc/js/application/diff/DiffChangeset.js b/webroot/rsrc/js/application/diff/DiffChangeset.js
index d6394a9217..8957827ccf 100644
--- a/webroot/rsrc/js/application/diff/DiffChangeset.js
+++ b/webroot/rsrc/js/application/diff/DiffChangeset.js
@@ -907,7 +907,10 @@ JX.install('DiffChangeset', {
         return;
       }
 
-      e.prevent();
+      // NOTE: Don't prevent or kill the event. If the user has text selected,
+      // clicking a header should clear the selection (and dismiss any inline
+      // context menu, if one exists) as clicking elsewhere in the document
+      // normally would.
 
       if (this._isSelected) {
         this.getChangesetList().selectChangeset(null);
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
index a0b093e21e..f380de4e75 100644
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -1131,9 +1131,8 @@ JX.install('DiffChangesetList', {
         return;
       }
 
-      // The user definitely clicked an inline, so we're going to handle the
-      // event.
-      e.kill();
+      // NOTE: Don't kill or prevent the event. In particular, we want this
+      // click to clear any text selection as it normally would.
 
       this.selectInline(inline);
     },