1564: javascript error: Error: 'rows[...]' is null or not an object in diffviewer, prevents file diffs to be shown (spinning wheel)

lon***@gmai***** (Google Code) (Is this you? Claim this profile.)
chipx86
chipx86
Feb. 20, 2011
What version are you running?

1.5 Beta 1

---------------------------
Error
---------------------------
A Runtime Error has occurred.
Do you wish to Debug?

Line: 798
Error: 'rows[...]' is null or not an object
---------------------------
Yes   No   
---------------------------


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

http://demo.reviewboard.org/r/2351/diff/?collapse=1

What steps will reproduce the problem?
0. Keep note of a line number at the end of a diff chunk, just before a 
collapsed section 
1. Expand the diffs.
2. Select lines that span the 'visible' chunk and the now expanded chunk.
3. Enter a comment
4. Save/publish the comment
5. Switch to the diff view

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

Would expect to see diffs for all files

With IE an error pop-up is displayed.  (See above)

With Firefox, Google Chrome of IE, only the diffs up to the comments are 
displayed.  The diffs for the remaining files are not shown, and a spinning 
wheel keeps spinning forever.

What operating system are you using? What browser?

Windows XP, Firefox, Chrome, IE

Please provide any additional information below.

The problem is on line 798.  endRow is out of range, with a value of 32 
while high is 31 and table.rows.length is 31.  As a result, 
table.rows[endRow] is undefined.

I'll propose a small patch testing for this condition, but I wonder if this 
piece of code is really needed

796 if (endRow != undefined) {
797 /* See if we got lucky and found it in the last row. */
798    if (parseInt(table.rows[endRow].getAttribute('line')) == linenum) {
799 return table.rows[endRow];
800 }
#1 lon***@gmai***** (Google Code) (Is this you? Claim this profile.)
This issue is still present with 1.5 RC1

http://demo.reviewboard.org/r/2351/diff/?collapse=1

Thanks!
chipx86
#2 chipx86
  • +Confirmed
  • -Priority-Medium
    +Priority-Critical
    +Milestone-Release1.5
    +Component-DiffViewer
chipx86
#3 chipx86
Pushing out to 1.5.x.
  • -Milestone-Release1.5
    +Milestone-Release1.5.x
chipx86
#4 chipx86
Fixed on release-1.5.x and master. This fix will be in 1.5.4.
  • -Confirmed
    +Fixed
  • +chipx86