diff --git a/resources/celerity/map.php b/resources/celerity/map.php --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -10,8 +10,8 @@ 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', 'core.pkg.css' => '261ee8cf', - 'core.pkg.js' => '5ace8a1e', - 'differential.pkg.css' => 'fcc82bc0', + 'core.pkg.js' => '5ba0b6d7', + 'differential.pkg.css' => 'd1b29c9c', 'differential.pkg.js' => '0e2b0e2c', 'diffusion.pkg.css' => '42c75c37', 'diffusion.pkg.js' => '91192d85', @@ -61,7 +61,7 @@ 'rsrc/css/application/dashboard/dashboard.css' => '4267d6c6', 'rsrc/css/application/diff/inline-comment-summary.css' => '81eb368d', 'rsrc/css/application/differential/add-comment.css' => '7e5900d9', - 'rsrc/css/application/differential/changeset-view.css' => '58236820', + 'rsrc/css/application/differential/changeset-view.css' => 'e2b81e85', 'rsrc/css/application/differential/core.css' => 'bdb93065', 'rsrc/css/application/differential/phui-inline-comment.css' => '48acce5b', 'rsrc/css/application/differential/revision-comment.css' => '7dbc8d1d', @@ -473,7 +473,7 @@ 'rsrc/js/core/behavior-linked-container.js' => '74446546', 'rsrc/js/core/behavior-more.js' => '506aa3f4', 'rsrc/js/core/behavior-object-selector.js' => 'a4af0b4a', - 'rsrc/js/core/behavior-oncopy.js' => '418f6684', + 'rsrc/js/core/behavior-oncopy.js' => 'f20d66c1', 'rsrc/js/core/behavior-phabricator-nav.js' => 'f166c949', 'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '2f80333f', 'rsrc/js/core/behavior-read-only-warning.js' => 'b9109f8f', @@ -541,7 +541,7 @@ 'conpherence-thread-manager' => 'aec8e38c', 'conpherence-transaction-css' => '3a3f5e7e', 'd3' => 'd67475f5', - 'differential-changeset-view-css' => '58236820', + 'differential-changeset-view-css' => 'e2b81e85', 'differential-core-view-css' => 'bdb93065', 'differential-revision-add-comment-css' => '7e5900d9', 'differential-revision-comment-css' => '7dbc8d1d', @@ -636,7 +636,7 @@ 'javelin-behavior-phabricator-nav' => 'f166c949', 'javelin-behavior-phabricator-notification-example' => '29819b75', 'javelin-behavior-phabricator-object-selector' => 'a4af0b4a', - 'javelin-behavior-phabricator-oncopy' => '418f6684', + 'javelin-behavior-phabricator-oncopy' => 'f20d66c1', 'javelin-behavior-phabricator-remarkup-assist' => '2f80333f', 'javelin-behavior-phabricator-reveal-content' => 'b105a3a6', 'javelin-behavior-phabricator-search-typeahead' => '1cb7d027', @@ -1222,10 +1222,6 @@ 'javelin-behavior', 'javelin-uri', ), - '418f6684' => array( - 'javelin-behavior', - 'javelin-dom', - ), '42c7a5a7' => array( 'javelin-install', 'javelin-dom', @@ -1380,9 +1376,6 @@ 'javelin-vector', 'javelin-typeahead-static-source', ), - 58236820 => array( - 'phui-inline-comment-view-css', - ), '5902260c' => array( 'javelin-util', 'javelin-magical-init', @@ -2039,6 +2032,9 @@ 'javelin-dom', 'javelin-stratcom', ), + 'e2b81e85' => array( + 'phui-inline-comment-view-css', + ), 'e562708c' => array( 'javelin-install', ), @@ -2090,6 +2086,10 @@ 'javelin-request', 'javelin-util', ), + 'f20d66c1' => array( + 'javelin-behavior', + 'javelin-dom', + ), 'f340a484' => array( 'javelin-install', 'javelin-dom', diff --git a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php --- a/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetHTMLRenderer.php @@ -436,7 +436,7 @@ 'table', array( 'class' => implode(' ', $classes), - 'sigil' => 'differential-diff', + 'sigil' => 'differential-diff intercept-copy', ), array( $this->renderColgroup(), diff --git a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php --- a/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php +++ b/src/applications/differential/render/DifferentialChangesetTwoUpRenderer.php @@ -319,12 +319,22 @@ $html[] = phutil_tag('tr', array(), array( $old_number, - phutil_tag('td', array('class' => $o_classes), $o_text), + phutil_tag( + 'td', + array( + 'class' => $o_classes, + 'data-copy-mode' => 'copy-l', + ), + $o_text), $new_number, $n_copy, phutil_tag( 'td', - array('class' => $n_classes, 'colspan' => $n_colspan), + array( + 'class' => $n_classes, + 'colspan' => $n_colspan, + 'data-copy-mode' => 'copy-r', + ), $n_text), $n_cov, )); diff --git a/webroot/rsrc/css/application/differential/changeset-view.css b/webroot/rsrc/css/application/differential/changeset-view.css --- a/webroot/rsrc/css/application/differential/changeset-view.css +++ b/webroot/rsrc/css/application/differential/changeset-view.css @@ -176,12 +176,6 @@ cursor: pointer; border-right: 1px solid {$thinblueborder}; overflow: hidden; - - -moz-user-select: -moz-none; - -khtml-user-select: none; - -webkit-user-select: none; - -ms-user-select: none; - user-select: none; } .differential-diff td.n::before { @@ -430,3 +424,33 @@ .diff-banner-buttons { float: right; } + +/* In Firefox, making the table unselectable and then making cells selectable +does not work: the cells remain unselectable. Narrowly mark the cells as +unselectable. */ + +.differential-diff.copy-l > tbody > tr > td, +.differential-diff.copy-r > tbody > tr > td { + -moz-user-select: -moz-none; + -khtml-user-select: none; + -ms-user-select: none; + -webkit-user-select: none; + user-select: none; +} + +.differential-diff.copy-l > tbody > tr > td, +.differential-diff.copy-r > tbody > tr > td { + opacity: 0.5; +} + +.differential-diff.copy-l > tbody > tr > td:nth-child(2) { + -webkit-user-select: auto; + user-select: auto; + opacity: 1; +} + +.differential-diff.copy-r > tbody > tr > td:nth-child(5) { + -webkit-user-select: auto; + user-select: auto; + opacity: 1; +} diff --git a/webroot/rsrc/js/core/behavior-oncopy.js b/webroot/rsrc/js/core/behavior-oncopy.js --- a/webroot/rsrc/js/core/behavior-oncopy.js +++ b/webroot/rsrc/js/core/behavior-oncopy.js @@ -4,62 +4,269 @@ * javelin-dom */ -/** - * Tools like Paste and Differential don't normally respond to the clipboard - * 'copy' operation well, because when a user copies text they'll get line - * numbers and other metadata. - * - * To improve this behavior, applications can embed markers that delimit - * metadata (left of the marker) from content (right of the marker). When - * we get a copy event, we strip out all the metadata and just copy the - * actual text. - */ JX.behavior('phabricator-oncopy', function() { + var copy_root; + var copy_mode; - var zws = '\u200B'; // Unicode Zero-Width Space + function onstartselect(e) { + var target = e.getTarget(); - JX.enableDispatch(document.body, 'copy'); - JX.Stratcom.listen( - ['copy'], - null, - function(e) { - - var selection; - var text; - if (window.getSelection) { - selection = window.getSelection(); - text = selection.toString(); + var container; + try { + // NOTE: For now, all elements with custom oncopy behavior are tables, + // so this tag selection will hit everything we need it to. + container = JX.DOM.findAbove(target, 'table', 'intercept-copy'); + } catch (ex) { + container = null; + } + + var old_mode = copy_mode; + clear_selection_mode(); + + if (!container) { + return; + } + + // If the potential selection is starting inside an inline comment, + // don't do anything special. + try { + if (JX.DOM.findAbove(target, 'div', 'differential-inline-comment')) { + return; + } + } catch (ex) { + // Continue. + } + + // Find the row and cell we're copying from. If we don't find anything, + // don't do anything special. + var row; + var cell; + try { + // The target may be the cell we're after, particularly if you click + // in the white area to the right of the text, towards the end of a line. + if (JX.DOM.isType(target, 'td')) { + cell = target; } else { - selection = document.selection; - text = selection.createRange().text; + cell = JX.DOM.findAbove(target, 'td'); } + row = JX.DOM.findAbove(target, 'tr'); + } catch (ex) { + return; + } - if (text.indexOf(zws) == -1) { - // If there's no marker in the text, just let it copy normally. - return; + // If the row doesn't have enough nodes, bail out. Note that it's okay + // to begin a selection in the whitespace on the opposite side of an inline + // comment. For example, if there's an inline comment on the right side of + // a diff, it's okay to start selecting the left side of the diff by + // clicking the corresponding empty space on the left side. + if (row.childNodes.length < 4) { + return; + } + + // If the selection's cell is in the "old" diff or the "new" diff, we'll + // activate an appropriate copy mode. + var mode; + if (cell === row.childNodes[1]) { + mode = 'copy-l'; + } else if ((row.childNodes.length >= 4) && (cell === row.childNodes[4])) { + mode = 'copy-r'; + } else { + return; + } + + // We found a copy mode, so set it as the current active mode. + copy_root = container; + copy_mode = mode; + + // If the user makes a selection, then clicks again inside the same + // selection, browsers retain the selection. This is because the user may + // want to drag-and-drop the text to another window. + + // Handle special cases when the click is inside an existing selection. + + var ranges = get_selected_ranges(); + if (ranges.length) { + // We'll have an existing selection if the user selects text on the right + // side of a diff, then clicks the selection on the left side of the + // diff, even if the second click is clicking part of the selection + // range where the selection highlight is currently invisible because + // of CSS rules. + + // This behavior looks and feels glitchy: an invisible selection range + // suddenly pops into existence and there's a bunch of flicker. If we're + // switching selection modes, clear the old selection to avoid this: + // assume the user is not trying to drag-and-drop text which is not + // visually selected. + + if (old_mode !== copy_mode) { + window.getSelection().removeAllRanges(); } - var result = []; - - // Strip everything before the marker (and the marker itself) out of the - // text. If a line doesn't have the marker, throw it away (the assumption - // is that it's a line number or part of some other meta-text). - var lines = text.split('\n'); - var pos; - for (var ii = 0; ii < lines.length; ii++) { - pos = lines[ii].indexOf(zws); - if (pos == -1 && ii !== 0) { - continue; + // In the more mundane case, if the user selects some text on one side + // of a diff and then clicks that same selection in a normal way (in + // the visible part of the highlighted text), we may either be altering + // the selection range or may be initiating a text drag depending on how + // long they hold the button for. Regardless of what we're doing, we're + // still in a selection mode, so keep the visual hints active. + + JX.DOM.alterClass(copy_root, copy_mode, true); + } + + // We've chosen a mode and saved it now, but we don't actually update to + // apply any visual changes until the user actually starts making some + // kind of selection. + } + + // When the selection range changes, apply CSS classes if the selection is + // nonempty. We don't want to make visual changes to the document immediately + // when the user press the mouse button, since we aren't yet sure that + // they are starting a selection: instead, wait for them to actually select + // something. + function onchangeselect() { + if (!copy_mode) { + return; + } + + var ranges = get_selected_ranges(); + JX.DOM.alterClass(copy_root, copy_mode, !!ranges.length); + } + + // When the user releases the mouse, get rid of the selection mode if we + // don't have a selection. + function onendselect(e) { + if (!copy_mode) { + return; + } + + var ranges = get_selected_ranges(); + if (!ranges.length) { + clear_selection_mode(); + } + } + + function get_selected_ranges() { + var ranges = []; + + if (!window.getSelection) { + return ranges; + } + + var selection = window.getSelection(); + for (var ii = 0; ii < selection.rangeCount; ii++) { + var range = selection.getRangeAt(ii); + if (range.collapsed) { + continue; + } + + ranges.push(range); + } + + return ranges; + } + + function clear_selection_mode() { + if (!copy_root) { + return; + } + + JX.DOM.alterClass(copy_root, copy_mode, false); + copy_root = null; + copy_mode = null; + } + + function oncopy(e) { + // If we aren't in a special copy mode, just fall back to default + // behavior. + if (!copy_mode) { + return; + } + + var ranges = get_selected_ranges(); + if (!ranges.length) { + return; + } + + var text_nodes = []; + for (var ii = 0; ii < ranges.length; ii++) { + var range = ranges[ii]; + + var fragment = range.cloneContents(); + if (!fragment.children.length) { + continue; + } + + // In Chrome and Firefox, because we've already applied "user-select" + // CSS to everything we don't intend to copy, the text in the selection + // range is correct, and the range will include only the correct text + // nodes. + + // However, in Safari, "user-select" does not apply to clipboard + // operations, so we get everything in the document between the beginning + // and end of the selection, even if it isn't visibly selected. + + // Even in Chrome and Firefox, we can get partial empty nodes: for + // example, where a "" is selectable but no content in the node is + // selectable. (We have to leave the "" itself selectable because + // of how Firefox applies "user-select" rules.) + + // The nodes we get here can also start and end more or less anywhere. + + // One saving grace is that we use "content: attr(data-n);" to render + // the line numbers and no browsers copy this content, so we don't have + // to worry about figuring out when text is line numbers. + + for (var jj = 0; jj < fragment.childNodes.length; jj++) { + var node = fragment.childNodes[jj]; + if (JX.DOM.isType(node, 'tr')) { + // This is an inline comment row, so we never want to copy any + // content inside of it. + if (JX.Stratcom.hasSigil(node, 'inline-row')) { + continue; + } + + // Assume anything else is a source code row. Keep only "" cells + // with the correct mode. + for (var kk = 0; kk < node.childNodes.length; kk++) { + var child = node.childNodes[kk]; + + var node_mode = child.getAttribute('data-copy-mode'); + if (node_mode === copy_mode) { + text_nodes.push(child); + } + } + } else { + // For anything else, assume this is a text fragment or part of + // a table cell or something and should be included in the selection + // range. + text_nodes.push(node); } - result.push(lines[ii].substring(pos + 1)); } - result = result.join('\n'); + + var text = []; + for (ii = 0; ii < text_nodes.length; ii++) { + text.push(text_nodes[ii].textContent); + } + text = text.join(''); var rawEvent = e.getRawEvent(); - var clipboardData = 'clipboardData' in rawEvent ? - rawEvent.clipboardData : - window.clipboardData; - clipboardData.setData('Text', result); + var data; + if ('clipboardData' in rawEvent) { + data = rawEvent.clipboardData; + } else { + data = window.clipboardData; + } + data.setData('Text', text); + e.prevent(); - }); + } + } + + JX.enableDispatch(document.body, 'copy'); + JX.enableDispatch(window, 'selectionchange'); + + JX.Stratcom.listen('mousedown', null, onstartselect); + JX.Stratcom.listen('selectionchange', null, onchangeselect); + JX.Stratcom.listen('mouseup', null, onendselect); + + JX.Stratcom.listen('copy', null, oncopy); });