Page MenuHomePhabricator

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

Authored by epriestley on Feb 17 2019, 3:38 PM.
Tags
None
Referenced Files
F12842954: D20191.diff
Thu, Mar 28, 10:24 PM
Unknown Object (File)
Mon, Mar 25, 5:07 PM
Unknown Object (File)
Fri, Mar 1, 4:46 AM
Unknown Object (File)
Feb 21 2024, 12:52 PM
Unknown Object (File)
Feb 17 2024, 9:35 AM
Unknown Object (File)
Feb 16 2024, 5:03 AM
Unknown Object (File)
Feb 3 2024, 11:26 PM
Unknown Object (File)
Feb 3 2024, 9:26 PM
Subscribers
None
Tokens
"Mountain of Wealth" token, awarded by leoluk."Mountain of Wealth" token, awarded by amckinley.

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
Branch
sorcery4
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 22047
Build 30120: Run Core Tests
Build 30119: arc lint + arc unit

Event Timeline

epriestley retitled this revision from Behold! Select text from either side of a diff! to Behold! Copy text from either side of a diff!.Feb 17 2019, 3:39 PM
amckinley added inline comments.
webroot/rsrc/js/core/behavior-oncopy.js
58

"begin begin"

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