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
--- 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]);