Page MenuHomePhabricator
Files F14775830

D20191.id48219.diff
No OneTemporary
Actions

D20191.id48219.diff
View Options

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 "<tr>" is selectable but no content in the node is
+ // selectable. (We have to leave the "<tr>" 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 "<td>" 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);
});

File Metadata

Mime Type
text/plain
Expires
Sat, Jan 25, 3:23 AM (18 h, 37 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7046262
Default Alt Text
D20191.id48219.diff (16 KB)

Event Timeline

Phabricator · Built by Phacility