Skip to content

Commit 8b550ce

Browse files
author
epriestley
committedNov 26, 2018
Don't allow the middle mouse button to start an inline comment
Summary: Ref T13216. See PHI985. When you click a line number to start an inline comment, we intend to initiate the action only if you used the left mouse button (desktop) or a touch (tablet/device). We currently have a `not right` condition for doing this, but it only excludes right clicks, not middle clicks (or other nth-button clicks). The `not right` condition was sligthly easier to write, but use an `is left` condition instead of a `not right` condition. Test Plan: - In Safari, Firefox and Chrome: - Used left click to start an inline. - Used middle click to do nothing (previously: started an inline). Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13216 Differential Revision: https://secure.phabricator.com/D19836
1 parent 5343b1f commit 8b550ce

File tree

3 files changed

+31
-14
lines changed

3 files changed

+31
-14
lines changed
 

‎resources/celerity/map.php

+13-13
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
'conpherence.pkg.css' => 'e68cf1fa',
1111
'conpherence.pkg.js' => '15191c65',
1212
'core.pkg.css' => 'cff4ff6f',
13-
'core.pkg.js' => 'b5a949ca',
13+
'core.pkg.js' => '4bde473b',
1414
'differential.pkg.css' => '06dc617c',
15-
'differential.pkg.js' => 'c1cfa143',
15+
'differential.pkg.js' => 'ef0b989b',
1616
'diffusion.pkg.css' => 'a2d17c7d',
1717
'diffusion.pkg.js' => '6134c5a1',
1818
'maniphest.pkg.css' => '4845691a',
@@ -207,7 +207,7 @@
207207
'rsrc/externals/font/lato/lato-regular.ttf' => 'e270165b',
208208
'rsrc/externals/font/lato/lato-regular.woff' => '13d39fe2',
209209
'rsrc/externals/font/lato/lato-regular.woff2' => '57a9f742',
210-
'rsrc/externals/javelin/core/Event.js' => '2ee659ce',
210+
'rsrc/externals/javelin/core/Event.js' => 'ef7e057f',
211211
'rsrc/externals/javelin/core/Stratcom.js' => '327f418a',
212212
'rsrc/externals/javelin/core/__tests__/event-stop-and-kill.js' => '717554e4',
213213
'rsrc/externals/javelin/core/__tests__/install.js' => 'c432ee85',
@@ -373,7 +373,7 @@
373373
'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '453c5375',
374374
'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => 'd4eecc63',
375375
'rsrc/js/application/diff/DiffChangeset.js' => 'b49b59d6',
376-
'rsrc/js/application/diff/DiffChangesetList.js' => 'e0b984b5',
376+
'rsrc/js/application/diff/DiffChangesetList.js' => '0a84bcc1',
377377
'rsrc/js/application/diff/DiffInline.js' => 'e83d28f3',
378378
'rsrc/js/application/diff/behavior-preview-link.js' => '051c7832',
379379
'rsrc/js/application/differential/behavior-comment-preview.js' => '51c5ad07',
@@ -688,7 +688,7 @@
688688
'javelin-diffusion-locate-file-source' => '00676f00',
689689
'javelin-dom' => '4976858c',
690690
'javelin-dynval' => 'f6555212',
691-
'javelin-event' => '2ee659ce',
691+
'javelin-event' => 'ef7e057f',
692692
'javelin-fx' => '54b612ba',
693693
'javelin-history' => 'd4505101',
694694
'javelin-install' => '05270951',
@@ -750,7 +750,7 @@
750750
'phabricator-darkmessage' => 'c48cccdd',
751751
'phabricator-dashboard-css' => 'fe5b1869',
752752
'phabricator-diff-changeset' => 'b49b59d6',
753-
'phabricator-diff-changeset-list' => 'e0b984b5',
753+
'phabricator-diff-changeset-list' => '0a84bcc1',
754754
'phabricator-diff-inline' => 'e83d28f3',
755755
'phabricator-drag-and-drop-file-upload' => '58dea2fa',
756756
'phabricator-draggable-list' => 'bea6e7f4',
@@ -944,6 +944,10 @@
944944
'javelin-dom',
945945
'javelin-router',
946946
),
947+
'0a84bcc1' => array(
948+
'javelin-install',
949+
'phuix-button-view',
950+
),
947951
'0f764c35' => array(
948952
'javelin-install',
949953
'javelin-util',
@@ -1049,9 +1053,6 @@
10491053
'javelin-install',
10501054
'javelin-event',
10511055
),
1052-
'2ee659ce' => array(
1053-
'javelin-install',
1054-
),
10551056
'31420f77' => array(
10561057
'javelin-behavior',
10571058
),
@@ -2009,10 +2010,6 @@
20092010
'phuix-icon-view',
20102011
'phabricator-prefab',
20112012
),
2012-
'e0b984b5' => array(
2013-
'javelin-install',
2014-
'phuix-button-view',
2015-
),
20162013
'e1d25dfb' => array(
20172014
'javelin-behavior',
20182015
'javelin-stratcom',
@@ -2094,6 +2091,9 @@
20942091
'javelin-behavior',
20952092
'javelin-uri',
20962093
),
2094+
'ef7e057f' => array(
2095+
'javelin-install',
2096+
),
20972097
'efe49472' => array(
20982098
'javelin-install',
20992099
'javelin-util',

‎webroot/rsrc/externals/javelin/core/Event.js

+14
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,20 @@ JX.install('Event', {
133133
return r.which == 3 || r.button == 2;
134134
},
135135

136+
137+
/**
138+
* Get whether the mouse button associated with the mouse event is the
139+
* left-side button in a browser-agnostic way.
140+
*
141+
* @return bool
142+
* @task info
143+
*/
144+
isLeftButton: function() {
145+
var r = this.getRawEvent();
146+
return (r.which == 1 || r.button == 0);
147+
},
148+
149+
136150
/**
137151
* Determine if a mouse event is a normal event (left mouse button, no
138152
* modifier keys).

‎webroot/rsrc/js/application/diff/DiffChangesetList.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -1261,7 +1261,10 @@ JX.install('DiffChangesetList', {
12611261
_onrangedown: function(e) {
12621262
// NOTE: We're allowing "mousedown" from a touch event through so users
12631263
// can leave inlines on a single line.
1264-
if (e.isRightButton()) {
1264+
1265+
// See PHI985. We want to exclude both right-mouse and middle-mouse
1266+
// clicks from continuing.
1267+
if (!e.isLeftButton()) {
12651268
return;
12661269
}
12671270

0 commit comments

Comments
 (0)
Failed to load comments.