Support copying text from columns in the diff viewer.

Review Request #6435 — Created Oct. 12, 2014 and submitted

Information

Review Board
release-2.0.x
b8e47e4...

Reviewers

This adds support for selecting text within a column of the diff viewer
or the text file review UI and copying it to the clipboard.

Browsers do not have any sort of support for confining a selection
within a column of text in a table. The few UIs out there that implement
this instead have a div per column, and do tricks like turning off line
wrapping or cutting off text in order to keep rows lined up. Not good
enough for us.

CSS does offer a way of turning off selection for an element, but all
browsers get it terribly wrong. Chrome will limit the visible selection,
but actually select all elements in-between (including other columns),
ignoring what's presented as the selection. Firefox does the inverse,
showing all as selected but only selecting the ones allowed to be
selected (with the added problem of selecting all whitespace that's in
the HTML, even if they're irrelevant to the output).

The good news is, with some cleverness, it turns out we can do
column-based selection sanely across platforms.

On MouseDown, we record the column, with the intent of limiting the
selection to that column. We then set a CSS class that contains the
column index. CSS rules unset selection for all other columns but the
active one, and turn off the selection background (for Firefox).

When the 'copy' event is fired (when the user triggers Copy), we have
the ability to manipulate the clipboard on modern browsers. We grab the
selection range, and extract the contents to a DocumentFragment. We can
then traverse the rows, extract the relevant columns, and build strings
ourselves, which are then set as the clipboard contents.

This gives us smart column-based copying on all modern versions of
Chrome, Firefox, and IE.

Tested on Chrome, Firefox, and IE9+ (IE8 and lower aren't supported).

Description From Last Updated

These could be combined.

daviddavid

You need to add a variable definition for $node.

daviddavid
reviewbot
  1. Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/diffviewer/templatetags/difftags.py
    
    Ignored Files:
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/css/diffviewer.less
        reviewboard/templates/reviews/ui/text.html
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
    
    Tool: Pyflakes
    Processed Files:
        reviewboard/diffviewer/templatetags/difftags.py
    
    Ignored Files:
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/css/diffviewer.less
        reviewboard/templates/reviews/ui/text.html
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
  2. 
      
david
  1. 
      
  2. Show all issues

    These could be combined.

  3. Show all issues

    You need to add a variable definition for $node.

  4. 
      
chipx86
reviewbot
  1. Tool: Pyflakes
    Processed Files:
        reviewboard/diffviewer/templatetags/difftags.py
    
    Ignored Files:
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/css/diffviewer.less
        reviewboard/templates/reviews/ui/text.html
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
    
    Tool: PEP8 Style Checker
    Processed Files:
        reviewboard/diffviewer/templatetags/difftags.py
    
    Ignored Files:
        reviewboard/static/rb/css/defs.less
        reviewboard/static/rb/css/diffviewer.less
        reviewboard/templates/reviews/ui/text.html
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
  2. 
      
david
  1. Ship It!

  2. 
      
chipx86
Review request changed
Status:
Completed
Change Summary:
Pushed to release-2.0.x (6721e10)