WIP use querySelector instead of hand-rolled code (bug 1115707); r!smacleod draft
authorbyron jones <glob@mozilla.com>
Mon, 22 Aug 2016 14:42:44 +0800
changeset 9466 2fbbe6da231cc4110ced884f7890d3ff3765862a
parent 9465 2cb79cc98da839870a9171a8fd0fc63583ab85db
child 9467 f63bc4097a9ff6e528236007d1f823460dca4706
push id1188
push userbjones@mozilla.com
push dateThu, 01 Sep 2016 13:14:49 +0000
bugs1115707
WIP use querySelector instead of hand-rolled code (bug 1115707); r!smacleod Modern CSS engines are fast enough to allow for removal of the hand-rolled binary searching of table rows. This has the advantage of always working regardless of how the table is laid out (eg. with inline comments). MozReview-Commit-ID: FaJTvjYPPtN
reviewboard/reviewboard/static/rb/js/views/textCommentRowSelector.js
--- a/reviewboard/reviewboard/static/rb/js/views/textCommentRowSelector.js
+++ b/reviewboard/reviewboard/static/rb/js/views/textCommentRowSelector.js
@@ -127,154 +127,19 @@ RB.TextCommentRowSelector = Backbone.Vie
             return [beginRowEl, endRowEl];
         } else {
             return null;
         }
     },
 
     /*
      * Finds the row in a table matching the specified line number.
-     *
-     * This will perform a binary search of the lines trying to find
-     * the matching line number. It will then return the row element,
-     * if found.
      */
-    findLineNumRow: function(lineNum, startRow, endRow) {
-        var row = null,
-            table = this.el,
-            rowOffset = 1, // Get past the headers.
-            guessRowNum,
-            guessRow,
-            oldHigh,
-            oldLow,
-            high,
-            low,
-            value,
-            found,
-            i,
-            j;
-
-        if (table.rows.length - rowOffset > lineNum) {
-            row = table.rows[rowOffset + lineNum];
-
-            // Account for the "x lines hidden" row.
-            if (row && this.getLineNum(row) === lineNum) {
-                return row;
-            }
-        }
-
-        if (startRow) {
-            // startRow already includes the offset, so we need to remove it.
-            startRow -= rowOffset;
-        }
-
-        low = startRow || 0;
-        high = Math.min(endRow || table.rows.length, table.rows.length);
-
-        if (endRow !== undefined && endRow < table.rows.length) {
-            // See if we got lucky and found it in the last row.
-            if (this.getLineNum(table.rows[endRow]) === lineNum) {
-                return table.rows[endRow];
-            }
-        } else if (row) {
-            /*
-             * We collapsed the rows (unless someone mucked with the DB),
-             * so the desired row is less than the row number retrieved.
-             */
-            high = Math.min(high, rowOffset + lineNum);
-        }
-
-        // Binary search for this cell.
-        for (i = Math.round((low + high) / 2); low < high - 1;) {
-            row = table.rows[rowOffset + i];
-
-            if (!row) {
-                // This should not happen, unless we miscomputed high.
-                high--;
-
-                /*
-                 * This won't do much if low + high is odd, but we'll catch
-                 * up on the next iteration.
-                 */
-                i = Math.round((low + high) / 2);
-                continue;
-            }
-
-            value = this.getLineNum(row);
-
-            if (!value) {
-                /*
-                 * Bad luck, let's look around.
-                 *
-                 * We'd expect to find a value on the first try, but the
-                 * following makes sure we explore all rows.
-                 */
-                found = false;
-
-                for (j = 1; j <= (high - low) / 2; j++) {
-                    row = table.rows[rowOffset + i + j];
-
-                    if (row && this.getLineNum(row)) {
-                        i = i + j;
-                        found = true;
-                        break;
-                    } else {
-                        row = table.rows[rowOffset + i - j];
-
-                        if (row && this.getLineNum(row)) {
-                            i = i - j;
-                            found = true;
-                            break;
-                        }
-                    }
-                }
-
-                if (found) {
-                    value = this.getLineNum(row);
-                } else {
-                    return null;
-                }
-            }
-
-            // See if we can use simple math to find the row quickly.
-            guessRowNum = lineNum - value + rowOffset + i;
-
-            if (guessRowNum >= 0 && guessRowNum < table.rows.length) {
-                guessRow = table.rows[guessRowNum];
-
-                if (guessRow && this.getLineNum(guessRow) === lineNum) {
-                    // We found it using maths!
-                    return guessRow;
-                }
-            }
-
-            oldHigh = high;
-            oldLow = low;
-
-            if (value > lineNum) {
-                high = i;
-            } else if (value < lineNum) {
-                low = i;
-            } else {
-                return row;
-            }
-
-            /*
-             * Make sure we don't get stuck in an infinite loop. This can happen
-             * when a comment is placed in a line that isn't being shown.
-             */
-            if (oldHigh === high && oldLow === low) {
-                break;
-            }
-
-            i = Math.round((low + high) / 2);
-        }
-
-        // Well.. damn. Ignore this then.
-        return null;
+    findLineNumRow: function(lineNum) {
+        return this.el.querySelector('tr[line="' + lineNum + '"]');
     },
 
     /*
      * Begins the selection of line numbers.
      */
     _begin: function($row) {
         var lineNum = this.getLineNum($row[0]);