From 57fd4bc68bcbb632705a0780c49cc3a43de236d0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 15 Mar 2012 19:04:59 -0700 Subject: [PATCH] Kind-of-terrible (?) oncopy handler Summary: Works in Safari, Firefox, Chrome. Test Plan: Copied some text, threw up a little in my mouth. Reviewers: aran, tuomaspelkonen, tomo, rstout, btrahan Reviewed By: aran CC: aran, epriestley, ddfisher Maniphest Tasks: T145, T995 Differential Revision: https://secure.phabricator.com/D244 --- src/__celerity_resource_map__.php | 11 +++ .../changeset/DifferentialChangesetParser.php | 5 +- .../DifferentialChangesetDetailView.php | 2 + .../view/PhabricatorPasteViewController.php | 6 +- .../paste/controller/view/__init__.php | 1 + .../js/application/core/behavior-oncopy.js | 80 +++++++++++++++++++ 6 files changed, 103 insertions(+), 2 deletions(-) create mode 100644 webroot/rsrc/js/application/core/behavior-oncopy.js diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index 06febdb093..93cf2d638f 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -839,6 +839,17 @@ celerity_register_resource_map(array( ), 'disk' => '/rsrc/js/application/core/behavior-object-selector.js', ), + 'javelin-behavior-phabricator-oncopy' => + array( + 'uri' => '/res/70b9b75e/rsrc/js/application/core/behavior-oncopy.js', + 'type' => 'js', + 'requires' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + ), + 'disk' => '/rsrc/js/application/core/behavior-oncopy.js', + ), 'javelin-behavior-phabricator-tooltips' => array( 'uri' => '/res/49f92a92/rsrc/js/application/core/behavior-tooltip.js', diff --git a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php index 78b3980a59..0440d28494 100644 --- a/src/applications/differential/parser/changeset/DifferentialChangesetParser.php +++ b/src/applications/differential/parser/changeset/DifferentialChangesetParser.php @@ -1417,7 +1417,10 @@ final class DifferentialChangesetParser { ''.$o_num.''. ''.$o_text.''. ''.$n_num.''. - ''.$n_text.''. + // NOTE: This is a unicode zero-width space, which we use as a hint + // when intercepting 'copy' events to make sure sensible text ends + // up on the clipboard. See the 'phabricator-oncopy' behavior. + ''."\xE2\x80\x8B".$n_text.''. $n_cov. ''; diff --git a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php index c6134b09ee..f3d9b6629a 100644 --- a/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php +++ b/src/applications/differential/view/changesetdetailview/DifferentialChangesetDetailView.php @@ -65,6 +65,8 @@ final class DifferentialChangesetDetailView extends AphrontView { require_celerity_resource('differential-changeset-view-css'); require_celerity_resource('syntax-highlighting-css'); + Javelin::initBehavior('phabricator-oncopy', array()); + if ($this->revisionID) { $edit = true; } else { diff --git a/src/applications/paste/controller/view/PhabricatorPasteViewController.php b/src/applications/paste/controller/view/PhabricatorPasteViewController.php index 48ecbea65f..26ccb1d7b9 100644 --- a/src/applications/paste/controller/view/PhabricatorPasteViewController.php +++ b/src/applications/paste/controller/view/PhabricatorPasteViewController.php @@ -142,6 +142,7 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { $text_list = explode("\n", $source); + Javelin::initBehavior('phabricator-oncopy', array()); $rows = $this->buildDisplayRows($text_list); $corpus_table = phutil_render_tag( @@ -179,7 +180,10 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController { ), $n); $rows[] = ''.$link.''. - ''.$line.''; + ''. + // NOTE: See the 'phabricator-oncopy' behavior. + "\xE2\x80\x8B". + $line.''; ++$n; } diff --git a/src/applications/paste/controller/view/__init__.php b/src/applications/paste/controller/view/__init__.php index 892964bc8c..57ad86603b 100644 --- a/src/applications/paste/controller/view/__init__.php +++ b/src/applications/paste/controller/view/__init__.php @@ -13,6 +13,7 @@ phutil_require_module('phabricator', 'applications/markup/syntax'); phutil_require_module('phabricator', 'applications/paste/controller/base'); phutil_require_module('phabricator', 'applications/paste/storage/paste'); phutil_require_module('phabricator', 'infrastructure/celerity/api'); +phutil_require_module('phabricator', 'infrastructure/javelin/api'); phutil_require_module('phabricator', 'view/control/table'); phutil_require_module('phabricator', 'view/layout/panel'); diff --git a/webroot/rsrc/js/application/core/behavior-oncopy.js b/webroot/rsrc/js/application/core/behavior-oncopy.js new file mode 100644 index 0000000000..fb3b6bfd06 --- /dev/null +++ b/webroot/rsrc/js/application/core/behavior-oncopy.js @@ -0,0 +1,80 @@ +/** + * @provides javelin-behavior-phabricator-oncopy + * @requires javelin-behavior + * 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 zws = "\u200B"; // Unicode Zero-Width Space + + document.body.oncopy = function(e) { + + var selection = window.getSelection(); + var text = selection.toString(); + + if (text.indexOf(zws) == -1) { + // If there's no marker in the text, just let it copy normally. + return; + } + + 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; + } + result.push(lines[ii].substring(pos + 1)); + } + result = result.join("\n"); + + if (e.clipboardData) { + // Safari and Chrome support this easy, straightforward mechanism. + e.clipboardData.setData('Text', result); + e.preventDefault(); + } else { + + // In Firefox, we have to create a 
 and select the text in it, then
+      // let the copy event fire. It has to be a 
 because Firefox won't
+      // copy returns properly out of a div, even if it has 'whitespace: pre'.
+      // There's been a bug open for 10 (!) years:
+      //
+      //   https://bugzilla.mozilla.org/show_bug.cgi?id=116083
+
+      var style = {
+        position: 'absolute',
+        left:     '-10000px'
+      };
+      var pre = JX.$N('pre', style, result);
+      document.body.appendChild(pre);
+
+      // Select the text in the 
.
+      var range = document.createRange();
+      range.selectNodeContents(pre);
+      selection.removeAllRanges();
+      selection.addRange(range);
+
+      setTimeout(function() { JX.DOM.remove(pre) }, 0);
+
+      // TODO: I tried to restore the old selection range but it doesn't seem
+      // to work or give me any errors. So you lose your selection when you
+      // copy. Oh well?
+    }
+  }
+});