Fix problems with text selection in the diff viewer.

Review Request #8752 — Created Feb. 15, 2017 and submitted

Information

Review Board
release-2.5.x
9e99b8a...

Reviewers

The old logic for copying text in the diff viewer made some bad
assumptions that weren't valid in modern versions of Firefox, and could
end up with truncated text. It also didn't take into account
insert/delete lines, which needed to be ignored based on the side of the
diff being selected.

Part of the problem with the diff selection logic had to do with an
assumption that querying for <pre> tags would actually return the tags,
but that wasn't always the case depending on where the selection ended.

The new logic doesn't rely on the query we had before, but rather walks
the tree itself, filtering out insert/delete lines where appropriate.
The end result is that we can now populate the clipboard with the
entirety of the selected area, excluding the newlines from the
insert/delete lines.

This should be more future-proof and cross-browser than what we had
before.

Tested all sorts of selections on Chrome and Firefox:

  • 1 entire line
  • Portions of a single line
  • Multi-line (entire lines)
  • Multi-line (starting/ending part-way through a line)
  • Selections starting/ending/spanning different marked up sections.
reviewbot
  1. Tool: Pyflakes
    Ignored Files:
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
    
    Tool: PEP8 Style Checker
    Ignored Files:
        reviewboard/static/rb/js/views/textCommentRowSelector.js
    
    
  2. 
      
david
  1. Ship It!
  2. 
      
chipx86
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-2.0.x (cd9c4a6)
Loading...