• 
      

    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:
    Completed
    Change Summary:
    Pushed to release-2.0.x (cd9c4a6)