@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Behold! Copy text from either side of a diff!

Summary:
Ref T12822. Ref T13161. By default, when users select text from a diff and copy it to the clipboard, they get both sides of the diff and all the line numbers. This is usually not what they intended to copy.

As of D20188, we use `content: attr(...)` to render line numbers. No browser copies this text, so that fixes line numbers.

We can use "user-select" CSS to visually prevent selection of line numbers and other stuff we don't want to copy. In Firefox and Chrome, "user-select" also applies to copied text, so getting "user-select" on the right nodes is largely good enough to do what we want.

In Safari, "user-select" is only visual, so we always need to crawl the DOM to figure out what text to pull out of it anyway.

In all browsers, we likely want to crawl the DOM anyway because this will let us show one piece of text and copy a different piece of text. We probably want to do this in the future to preserve "\t" tabs, and possibly to let us render certain character codes in one way but copy their original values. For example, we could render "\x07" as "␇".

Finally, we have to figure out which side of the diff we're copying from. The rule here is:

- If you start the selection by clicking somewhere on the left or right side of the diff, that's what you're copying.
- Otherwise, use normal document copy rules.

So the overall flow here is:

- Listen for clicks.
- When the user clicks the left or right side of the diff, store what they clicked.
- When a selection starts, and something is actually selected, check if it was initiated by clicking a diff. If it was, apply a visual effect to get "user-select" where it needs to go and show the user what we think they're doing and what we're going to copy.
- (Then, try to handle a bunch of degenerate cases where you start a selection and then click inside that selection.)
- When a user clicks elsewhere or ends the selection with nothing selected, clear the selection mode.
- When a user copies text, if we have an active selection mode, pull all the selected nodes out of the DOM and filter out the ones we don't want to copy, then stitch the text back together. Although I believe this didn't work well in ~2010, it appears to work well today.

Test Plan: This mostly seems to work in Safari, Chrome, and Firefox. T12822 has some errata. I haven't tested touch events but am satisfied if the touch event story is anything better than "permanently destroys data".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161, T12822

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

+306 -65
+13 -13
resources/celerity/map.php
··· 10 10 'conpherence.pkg.css' => '3c8a0668', 11 11 'conpherence.pkg.js' => '020aebcf', 12 12 'core.pkg.css' => '261ee8cf', 13 - 'core.pkg.js' => '5ace8a1e', 14 - 'differential.pkg.css' => 'fcc82bc0', 13 + 'core.pkg.js' => '5ba0b6d7', 14 + 'differential.pkg.css' => 'd1b29c9c', 15 15 'differential.pkg.js' => '0e2b0e2c', 16 16 'diffusion.pkg.css' => '42c75c37', 17 17 'diffusion.pkg.js' => '91192d85', ··· 61 61 'rsrc/css/application/dashboard/dashboard.css' => '4267d6c6', 62 62 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 63 63 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', 64 - 'rsrc/css/application/differential/changeset-view.css' => '58236820', 64 + 'rsrc/css/application/differential/changeset-view.css' => 'e2b81e85', 65 65 'rsrc/css/application/differential/core.css' => 'bdb93065', 66 66 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 67 67 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', ··· 473 473 'rsrc/js/core/behavior-linked-container.js' => '74446546', 474 474 'rsrc/js/core/behavior-more.js' => '506aa3f4', 475 475 'rsrc/js/core/behavior-object-selector.js' => 'a4af0b4a', 476 - 'rsrc/js/core/behavior-oncopy.js' => '418f6684', 476 + 'rsrc/js/core/behavior-oncopy.js' => 'f20d66c1', 477 477 'rsrc/js/core/behavior-phabricator-nav.js' => 'f166c949', 478 478 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '2f80333f', 479 479 'rsrc/js/core/behavior-read-only-warning.js' => 'b9109f8f', ··· 541 541 'conpherence-thread-manager' => 'aec8e38c', 542 542 'conpherence-transaction-css' => '3a3f5e7e', 543 543 'd3' => 'd67475f5', 544 - 'differential-changeset-view-css' => '58236820', 544 + 'differential-changeset-view-css' => 'e2b81e85', 545 545 'differential-core-view-css' => 'bdb93065', 546 546 'differential-revision-add-comment-css' => '7e5900d9', 547 547 'differential-revision-comment-css' => '7dbc8d1d', ··· 636 636 'javelin-behavior-phabricator-nav' => 'f166c949', 637 637 'javelin-behavior-phabricator-notification-example' => '29819b75', 638 638 'javelin-behavior-phabricator-object-selector' => 'a4af0b4a', 639 - 'javelin-behavior-phabricator-oncopy' => '418f6684', 639 + 'javelin-behavior-phabricator-oncopy' => 'f20d66c1', 640 640 'javelin-behavior-phabricator-remarkup-assist' => '2f80333f', 641 641 'javelin-behavior-phabricator-reveal-content' => 'b105a3a6', 642 642 'javelin-behavior-phabricator-search-typeahead' => '1cb7d027', ··· 1222 1222 'javelin-behavior', 1223 1223 'javelin-uri', 1224 1224 ), 1225 - '418f6684' => array( 1226 - 'javelin-behavior', 1227 - 'javelin-dom', 1228 - ), 1229 1225 '42c7a5a7' => array( 1230 1226 'javelin-install', 1231 1227 'javelin-dom', ··· 1379 1375 'javelin-stratcom', 1380 1376 'javelin-vector', 1381 1377 'javelin-typeahead-static-source', 1382 - ), 1383 - 58236820 => array( 1384 - 'phui-inline-comment-view-css', 1385 1378 ), 1386 1379 '5902260c' => array( 1387 1380 'javelin-util', ··· 2039 2032 'javelin-dom', 2040 2033 'javelin-stratcom', 2041 2034 ), 2035 + 'e2b81e85' => array( 2036 + 'phui-inline-comment-view-css', 2037 + ), 2042 2038 'e562708c' => array( 2043 2039 'javelin-install', 2044 2040 ), ··· 2089 2085 'javelin-vector', 2090 2086 'javelin-request', 2091 2087 'javelin-util', 2088 + ), 2089 + 'f20d66c1' => array( 2090 + 'javelin-behavior', 2091 + 'javelin-dom', 2092 2092 ), 2093 2093 'f340a484' => array( 2094 2094 'javelin-install',
+1 -1
src/applications/differential/render/DifferentialChangesetHTMLRenderer.php
··· 436 436 'table', 437 437 array( 438 438 'class' => implode(' ', $classes), 439 - 'sigil' => 'differential-diff', 439 + 'sigil' => 'differential-diff intercept-copy', 440 440 ), 441 441 array( 442 442 $this->renderColgroup(),
+12 -2
src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php
··· 319 319 320 320 $html[] = phutil_tag('tr', array(), array( 321 321 $old_number, 322 - phutil_tag('td', array('class' => $o_classes), $o_text), 322 + phutil_tag( 323 + 'td', 324 + array( 325 + 'class' => $o_classes, 326 + 'data-copy-mode' => 'copy-l', 327 + ), 328 + $o_text), 323 329 $new_number, 324 330 $n_copy, 325 331 phutil_tag( 326 332 'td', 327 - array('class' => $n_classes, 'colspan' => $n_colspan), 333 + array( 334 + 'class' => $n_classes, 335 + 'colspan' => $n_colspan, 336 + 'data-copy-mode' => 'copy-r', 337 + ), 328 338 $n_text), 329 339 $n_cov, 330 340 ));
+30 -6
webroot/rsrc/css/application/differential/changeset-view.css
··· 176 176 cursor: pointer; 177 177 border-right: 1px solid {$thinblueborder}; 178 178 overflow: hidden; 179 - 180 - -moz-user-select: -moz-none; 181 - -khtml-user-select: none; 182 - -webkit-user-select: none; 183 - -ms-user-select: none; 184 - user-select: none; 185 179 } 186 180 187 181 .differential-diff td.n::before { ··· 430 424 .diff-banner-buttons { 431 425 float: right; 432 426 } 427 + 428 + /* In Firefox, making the table unselectable and then making cells selectable 429 + does not work: the cells remain unselectable. Narrowly mark the cells as 430 + unselectable. */ 431 + 432 + .differential-diff.copy-l > tbody > tr > td, 433 + .differential-diff.copy-r > tbody > tr > td { 434 + -moz-user-select: -moz-none; 435 + -khtml-user-select: none; 436 + -ms-user-select: none; 437 + -webkit-user-select: none; 438 + user-select: none; 439 + } 440 + 441 + .differential-diff.copy-l > tbody > tr > td, 442 + .differential-diff.copy-r > tbody > tr > td { 443 + opacity: 0.5; 444 + } 445 + 446 + .differential-diff.copy-l > tbody > tr > td:nth-child(2) { 447 + -webkit-user-select: auto; 448 + user-select: auto; 449 + opacity: 1; 450 + } 451 + 452 + .differential-diff.copy-r > tbody > tr > td:nth-child(5) { 453 + -webkit-user-select: auto; 454 + user-select: auto; 455 + opacity: 1; 456 + }
+250 -43
webroot/rsrc/js/core/behavior-oncopy.js
··· 4 4 * javelin-dom 5 5 */ 6 6 7 - /** 8 - * Tools like Paste and Differential don't normally respond to the clipboard 9 - * 'copy' operation well, because when a user copies text they'll get line 10 - * numbers and other metadata. 11 - * 12 - * To improve this behavior, applications can embed markers that delimit 13 - * metadata (left of the marker) from content (right of the marker). When 14 - * we get a copy event, we strip out all the metadata and just copy the 15 - * actual text. 16 - */ 17 7 JX.behavior('phabricator-oncopy', function() { 8 + var copy_root; 9 + var copy_mode; 18 10 19 - var zws = '\u200B'; // Unicode Zero-Width Space 11 + function onstartselect(e) { 12 + var target = e.getTarget(); 13 + 14 + var container; 15 + try { 16 + // NOTE: For now, all elements with custom oncopy behavior are tables, 17 + // so this tag selection will hit everything we need it to. 18 + container = JX.DOM.findAbove(target, 'table', 'intercept-copy'); 19 + } catch (ex) { 20 + container = null; 21 + } 20 22 21 - JX.enableDispatch(document.body, 'copy'); 22 - JX.Stratcom.listen( 23 - ['copy'], 24 - null, 25 - function(e) { 23 + var old_mode = copy_mode; 24 + clear_selection_mode(); 26 25 27 - var selection; 28 - var text; 29 - if (window.getSelection) { 30 - selection = window.getSelection(); 31 - text = selection.toString(); 26 + if (!container) { 27 + return; 28 + } 29 + 30 + // If the potential selection is starting inside an inline comment, 31 + // don't do anything special. 32 + try { 33 + if (JX.DOM.findAbove(target, 'div', 'differential-inline-comment')) { 34 + return; 35 + } 36 + } catch (ex) { 37 + // Continue. 38 + } 39 + 40 + // Find the row and cell we're copying from. If we don't find anything, 41 + // don't do anything special. 42 + var row; 43 + var cell; 44 + try { 45 + // The target may be the cell we're after, particularly if you click 46 + // in the white area to the right of the text, towards the end of a line. 47 + if (JX.DOM.isType(target, 'td')) { 48 + cell = target; 32 49 } else { 33 - selection = document.selection; 34 - text = selection.createRange().text; 50 + cell = JX.DOM.findAbove(target, 'td'); 51 + } 52 + row = JX.DOM.findAbove(target, 'tr'); 53 + } catch (ex) { 54 + return; 55 + } 56 + 57 + // If the row doesn't have enough nodes, bail out. Note that it's okay 58 + // to begin a selection in the whitespace on the opposite side of an inline 59 + // comment. For example, if there's an inline comment on the right side of 60 + // a diff, it's okay to start selecting the left side of the diff by 61 + // clicking the corresponding empty space on the left side. 62 + if (row.childNodes.length < 4) { 63 + return; 64 + } 65 + 66 + // If the selection's cell is in the "old" diff or the "new" diff, we'll 67 + // activate an appropriate copy mode. 68 + var mode; 69 + if (cell === row.childNodes[1]) { 70 + mode = 'copy-l'; 71 + } else if ((row.childNodes.length >= 4) && (cell === row.childNodes[4])) { 72 + mode = 'copy-r'; 73 + } else { 74 + return; 75 + } 76 + 77 + // We found a copy mode, so set it as the current active mode. 78 + copy_root = container; 79 + copy_mode = mode; 80 + 81 + // If the user makes a selection, then clicks again inside the same 82 + // selection, browsers retain the selection. This is because the user may 83 + // want to drag-and-drop the text to another window. 84 + 85 + // Handle special cases when the click is inside an existing selection. 86 + 87 + var ranges = get_selected_ranges(); 88 + if (ranges.length) { 89 + // We'll have an existing selection if the user selects text on the right 90 + // side of a diff, then clicks the selection on the left side of the 91 + // diff, even if the second click is clicking part of the selection 92 + // range where the selection highlight is currently invisible because 93 + // of CSS rules. 94 + 95 + // This behavior looks and feels glitchy: an invisible selection range 96 + // suddenly pops into existence and there's a bunch of flicker. If we're 97 + // switching selection modes, clear the old selection to avoid this: 98 + // assume the user is not trying to drag-and-drop text which is not 99 + // visually selected. 100 + 101 + if (old_mode !== copy_mode) { 102 + window.getSelection().removeAllRanges(); 103 + } 104 + 105 + // In the more mundane case, if the user selects some text on one side 106 + // of a diff and then clicks that same selection in a normal way (in 107 + // the visible part of the highlighted text), we may either be altering 108 + // the selection range or may be initiating a text drag depending on how 109 + // long they hold the button for. Regardless of what we're doing, we're 110 + // still in a selection mode, so keep the visual hints active. 111 + 112 + JX.DOM.alterClass(copy_root, copy_mode, true); 113 + } 114 + 115 + // We've chosen a mode and saved it now, but we don't actually update to 116 + // apply any visual changes until the user actually starts making some 117 + // kind of selection. 118 + } 119 + 120 + // When the selection range changes, apply CSS classes if the selection is 121 + // nonempty. We don't want to make visual changes to the document immediately 122 + // when the user press the mouse button, since we aren't yet sure that 123 + // they are starting a selection: instead, wait for them to actually select 124 + // something. 125 + function onchangeselect() { 126 + if (!copy_mode) { 127 + return; 128 + } 129 + 130 + var ranges = get_selected_ranges(); 131 + JX.DOM.alterClass(copy_root, copy_mode, !!ranges.length); 132 + } 133 + 134 + // When the user releases the mouse, get rid of the selection mode if we 135 + // don't have a selection. 136 + function onendselect(e) { 137 + if (!copy_mode) { 138 + return; 139 + } 140 + 141 + var ranges = get_selected_ranges(); 142 + if (!ranges.length) { 143 + clear_selection_mode(); 144 + } 145 + } 146 + 147 + function get_selected_ranges() { 148 + var ranges = []; 149 + 150 + if (!window.getSelection) { 151 + return ranges; 152 + } 153 + 154 + var selection = window.getSelection(); 155 + for (var ii = 0; ii < selection.rangeCount; ii++) { 156 + var range = selection.getRangeAt(ii); 157 + if (range.collapsed) { 158 + continue; 35 159 } 36 160 37 - if (text.indexOf(zws) == -1) { 38 - // If there's no marker in the text, just let it copy normally. 39 - return; 161 + ranges.push(range); 162 + } 163 + 164 + return ranges; 165 + } 166 + 167 + function clear_selection_mode() { 168 + if (!copy_root) { 169 + return; 170 + } 171 + 172 + JX.DOM.alterClass(copy_root, copy_mode, false); 173 + copy_root = null; 174 + copy_mode = null; 175 + } 176 + 177 + function oncopy(e) { 178 + // If we aren't in a special copy mode, just fall back to default 179 + // behavior. 180 + if (!copy_mode) { 181 + return; 182 + } 183 + 184 + var ranges = get_selected_ranges(); 185 + if (!ranges.length) { 186 + return; 187 + } 188 + 189 + var text_nodes = []; 190 + for (var ii = 0; ii < ranges.length; ii++) { 191 + var range = ranges[ii]; 192 + 193 + var fragment = range.cloneContents(); 194 + if (!fragment.children.length) { 195 + continue; 40 196 } 41 197 42 - var result = []; 198 + // In Chrome and Firefox, because we've already applied "user-select" 199 + // CSS to everything we don't intend to copy, the text in the selection 200 + // range is correct, and the range will include only the correct text 201 + // nodes. 43 202 44 - // Strip everything before the marker (and the marker itself) out of the 45 - // text. If a line doesn't have the marker, throw it away (the assumption 46 - // is that it's a line number or part of some other meta-text). 47 - var lines = text.split('\n'); 48 - var pos; 49 - for (var ii = 0; ii < lines.length; ii++) { 50 - pos = lines[ii].indexOf(zws); 51 - if (pos == -1 && ii !== 0) { 52 - continue; 203 + // However, in Safari, "user-select" does not apply to clipboard 204 + // operations, so we get everything in the document between the beginning 205 + // and end of the selection, even if it isn't visibly selected. 206 + 207 + // Even in Chrome and Firefox, we can get partial empty nodes: for 208 + // example, where a "<tr>" is selectable but no content in the node is 209 + // selectable. (We have to leave the "<tr>" itself selectable because 210 + // of how Firefox applies "user-select" rules.) 211 + 212 + // The nodes we get here can also start and end more or less anywhere. 213 + 214 + // One saving grace is that we use "content: attr(data-n);" to render 215 + // the line numbers and no browsers copy this content, so we don't have 216 + // to worry about figuring out when text is line numbers. 217 + 218 + for (var jj = 0; jj < fragment.childNodes.length; jj++) { 219 + var node = fragment.childNodes[jj]; 220 + if (JX.DOM.isType(node, 'tr')) { 221 + // This is an inline comment row, so we never want to copy any 222 + // content inside of it. 223 + if (JX.Stratcom.hasSigil(node, 'inline-row')) { 224 + continue; 225 + } 226 + 227 + // Assume anything else is a source code row. Keep only "<td>" cells 228 + // with the correct mode. 229 + for (var kk = 0; kk < node.childNodes.length; kk++) { 230 + var child = node.childNodes[kk]; 231 + 232 + var node_mode = child.getAttribute('data-copy-mode'); 233 + if (node_mode === copy_mode) { 234 + text_nodes.push(child); 235 + } 236 + } 237 + } else { 238 + // For anything else, assume this is a text fragment or part of 239 + // a table cell or something and should be included in the selection 240 + // range. 241 + text_nodes.push(node); 53 242 } 54 - result.push(lines[ii].substring(pos + 1)); 55 243 } 56 - result = result.join('\n'); 244 + 245 + var text = []; 246 + for (ii = 0; ii < text_nodes.length; ii++) { 247 + text.push(text_nodes[ii].textContent); 248 + } 249 + text = text.join(''); 57 250 58 251 var rawEvent = e.getRawEvent(); 59 - var clipboardData = 'clipboardData' in rawEvent ? 60 - rawEvent.clipboardData : 61 - window.clipboardData; 62 - clipboardData.setData('Text', result); 252 + var data; 253 + if ('clipboardData' in rawEvent) { 254 + data = rawEvent.clipboardData; 255 + } else { 256 + data = window.clipboardData; 257 + } 258 + data.setData('Text', text); 259 + 63 260 e.prevent(); 64 - }); 261 + } 262 + } 263 + 264 + JX.enableDispatch(document.body, 'copy'); 265 + JX.enableDispatch(window, 'selectionchange'); 266 + 267 + JX.Stratcom.listen('mousedown', null, onstartselect); 268 + JX.Stratcom.listen('selectionchange', null, onchangeselect); 269 + JX.Stratcom.listen('mouseup', null, onendselect); 270 + 271 + JX.Stratcom.listen('copy', null, oncopy); 65 272 });