Page MenuHomePhabricator

Behold! Copy text from either side of a diff!
ClosedPublic

Authored by epriestley on Sun, Feb 17, 3:38 PM.

Details

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".

Diff Detail

Repository
rP Phabricator
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Sun, Feb 17, 3:38 PM
epriestley retitled this revision from Behold! Select text from either side of a diff! to Behold! Copy text from either side of a diff!.Sun, Feb 17, 3:39 PM
epriestley requested review of this revision.Sun, Feb 17, 3:40 PM
amckinley accepted this revision.Tue, Feb 19, 10:29 PM
amckinley added inline comments.
webroot/rsrc/js/core/behavior-oncopy.js
58

"begin begin"

This revision is now accepted and ready to land.Tue, Feb 19, 10:29 PM
This revision was automatically updated to reflect the committed changes.