From 42f1a95a1255fc3c8062611ceb36551c05b3c0a5 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 14 May 2020 11:34:33 -0700
Subject: [PATCH] Fix a flash of document selection when "oncopy" and "inline
 on range" behaviors interact

Summary:
Ref T13513. In Safari, do this:

  - view a 2-up diff with content on both sides;
  - select more than one line on the right side; and
  - use your mouse to click "New Inline Comment" in the context menu that pops up.

The mousedown event for the "New Inline Comment" click removes the "copy selection" behavior and creates a flash where both sides of the diff are selected.

This doesn't happen with (most) normal clicks because mouse down on a non-grabbable element clears the document selection.

To avoid this, don't reset the copy selection behavior if the user mouses down on an "<a />". This might not be robust, but seems simple and plausible as a fix.

Test Plan:
  - See above.
  - Before patch: flash of overbroad selection when clicking "New Inline Comment".
  - After patch: no selection flash.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21254
---
 resources/celerity/map.php              | 14 +++++++-------
 webroot/rsrc/js/core/behavior-oncopy.js | 11 +++++++++++
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/resources/celerity/map.php b/resources/celerity/map.php
index d83eb4813e..91c1ab7247 100644
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@@ -10,7 +10,7 @@ return array(
     'conpherence.pkg.css' => '0e3cf785',
     'conpherence.pkg.js' => '020aebcf',
     'core.pkg.css' => 'a560707d',
-    'core.pkg.js' => '0efaf0ac',
+    'core.pkg.js' => '845355f4',
     'dark-console.pkg.js' => '187792c2',
     'differential.pkg.css' => 'b042ee8b',
     'differential.pkg.js' => '5d560bda',
@@ -488,7 +488,7 @@ return array(
     'rsrc/js/core/behavior-linked-container.js' => '74446546',
     'rsrc/js/core/behavior-more.js' => '506aa3f4',
     'rsrc/js/core/behavior-object-selector.js' => '98ef467f',
-    'rsrc/js/core/behavior-oncopy.js' => 'b475aae5',
+    'rsrc/js/core/behavior-oncopy.js' => 'da8f5259',
     'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '54262396',
     'rsrc/js/core/behavior-read-only-warning.js' => 'b9109f8f',
     'rsrc/js/core/behavior-redirect.js' => '407ee861',
@@ -648,7 +648,7 @@ return array(
     'javelin-behavior-phabricator-line-linker' => '590e6527',
     'javelin-behavior-phabricator-notification-example' => '29819b75',
     'javelin-behavior-phabricator-object-selector' => '98ef467f',
-    'javelin-behavior-phabricator-oncopy' => 'b475aae5',
+    'javelin-behavior-phabricator-oncopy' => 'da8f5259',
     'javelin-behavior-phabricator-remarkup-assist' => '54262396',
     'javelin-behavior-phabricator-reveal-content' => 'b105a3a6',
     'javelin-behavior-phabricator-search-typeahead' => '1cb7d027',
@@ -1952,10 +1952,6 @@ return array(
       'javelin-workboard-card-template',
       'javelin-workboard-order-template',
     ),
-    'b475aae5' => array(
-      'javelin-behavior',
-      'javelin-dom',
-    ),
     'b49fd60c' => array(
       'multirow-row-manager',
       'trigger-rule',
@@ -2116,6 +2112,10 @@ return array(
     'da15d3dc' => array(
       'phui-oi-list-view-css',
     ),
+    'da8f5259' => array(
+      'javelin-behavior',
+      'javelin-dom',
+    ),
     'dae2d55b' => array(
       'javelin-behavior',
       'javelin-uri',
diff --git a/webroot/rsrc/js/core/behavior-oncopy.js b/webroot/rsrc/js/core/behavior-oncopy.js
index 10a8c901d0..8c2aa7808f 100644
--- a/webroot/rsrc/js/core/behavior-oncopy.js
+++ b/webroot/rsrc/js/core/behavior-oncopy.js
@@ -11,6 +11,17 @@ JX.behavior('phabricator-oncopy', function() {
   function onstartselect(e) {
     var target = e.getTarget();
 
+    // See T13513. If the user selects multiple lines in a 2-up diff and then
+    // clicks "New Inline Comment" in the context menu that pops up, the
+    // mousedown causes us to arrive here and remove the "selectable" CSS
+    // styles, and creates a flash of selected content across both sides of
+    // the diff, which is distracting. To attempt to avoid this, bail out if
+    // the user clicked a link.
+
+    if (JX.DOM.isType(target, 'a')) {
+      return;
+    }
+
     var container;
     try {
       // NOTE: For now, all elements with custom oncopy behavior are tables,