1440: Comment flag not visible in View Diff

lon***@gmai***** (Google Code) (Is this you? Claim this profile.)
Jan. 4, 2010
1165 
What version are you running?

1.0.5.1


What's the URL of the page containing the problem?

http://reviewboard.xxxxxxx.com/r/4174/diff/1/#index_header

What steps will reproduce the problem?

Unknown.  This problem was only noticed for a few comments.

What is the expected output? What do you see instead?

A comment flag is missing in view diffs, though the comment is visible in
the reviews page (and the comment is also visible in the html source for
the view diffs page).

What operating system are you using? What browser?

Windows, Firefox

Please provide any additional information below.

Bug if row is undefined for the first iteration.  i is not updated, so row
stays undefined, and the loop continues until high reaches low.  The
function then exits and failed to find the correct row.

In diffviewer.js

function findLineNumRow(table, linenum, startRow, endRow) {


885 	/* Binary search for this cell. */
886 	for (var i = Math.round((low + high) / 2); low < high - 1;) {
887 		row = table.rows[row_offset + i];
888
889 		if (!row) {
890 			high--;
891 			continue;
892 		}
#1 lon***@gmai***** (Google Code) (Is this you? Claim this profile.) 
There is also an issue with this assignment:

 877     } else if (row != null) {
 878         /*
 879          * We collapsed the rows (unless someone mucked with the DB),
 880          * so the desired row is less than the row number retrieved.
 881          */
 882         high = parseInt(row.getAttribute('line'))
 883     }

high should not be set to a linenum, but to a row number.


And the following statement

 896         if (!value) {
 897             i++;
 898             continue;
 899         }

assumes that value will not be NULL eventually as i keeps increasing.  This is not
very robust.
david
#2 david
#3 lon***@gmai***** (Google Code) (Is this you? Claim this profile.) 
diffs sent to reviewboard for review.

http://reviews.reviewboard.org/r/1308/