Fix diff selection on Firefox, and make the code more bullet-proof.

Review Request #6895 — Created Feb. 3, 2015 and submitted

Information

Review Board
release-2.0.x
bce1e34...

Reviewers

Firefox recently broke our text selection in the diff viewer, due to
some changes in how it broke selections into Range objects, and
differences in how it computed the state for those Range objects.

This change bullet-proofs and future-proofs the logic quite a bit in a
few ways:

  • It's no longer assumed that we're working with a single Range.
    We now loop through all Ranges.

  • It's no longer assumed that we have any <tr> tags in the list of
    nodes. Instead, we assume <td> and <pre>, which should both be valid.
    The <tr> was previously not appearing for the last line on Firefox
    if ending the selection within the row's text.

  • We look for the specific <td> cells we want, instead of assuming
    that any <tr> that does come back will have the complete list of
    cells, which we were dependent on before.

  • We're smarter about when we decide to add a newline character.

This should also be a bit faster, as we've eliminated some queries and
some function calls.

Tested the following cases on Firefox, Chrome, and IE:

  • Selecting full line ranges spanning multiple <tbody>s.
  • Selecting ranges spanning multiple <tbody>s, with the beginning being in the middle of a line.
  • Selecting ranges spanning multiple <tbody>s, with the end being in the middle of a line.
  • Selecting a range of text within a single line, not spanning multiple lines.

These were all tested for standard diffs on both the left-hand and right-hand sides, and for new files.

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 (4e81066)
Loading...