1224: yellow diff highlighting is off by one line
- Fixed
- Review Board
| eri***@gmai***** (Google Code) (Is this you? Claim this profile.) | |
| Jan. 4, 2010 | |
| 1225 |
What version are you running? Don't know. It's VMware's internal reviewboard installation as of 7/15/2009. What's the URL of the page containing the problem? https://reviewboard.eng.vmware.com/r/88100/diff/2-3/#index_header What steps will reproduce the problem? 1. No idea, but you can see the problem by looking at line 1013 (left) comparing against line 1048 (right) of that review's interdiff between revisions 2 & 3. What is the expected output? What do you see instead? The expected output would have been these 2 lines not being highlighted (since they are identical). Instead this is what I see: 1. The lines are identical, but they are highlighted as if they are different. 2. The next lines after these are the ones that differ. 3. Also, the "bold highlighted" (darker yellow) areas directly correspond to the actual changed areas of the next lines, so if the highlighting would have been 1 line later it would have been perfect. What operating system are you using? What browser? Windows XP SP3. Firefox 3.5. Please provide any additional information below.
Been looking into this. It's not just those lines. Most of the lines are off. Any replace line, in fact. This seems to be due to our markup and text lines being off. We compare the plain text lines and then generate the interline diff highlighting, and then apply the highlighting to the HTML lines. However, we're applying them to a line too early, implying that the HTML line is off by one compared to the plain text lines. This all appears to start with a block that looks somewhat like: /* * NOTE: Blah blah blah... * Blah blah blah... * total of 6 lines of this... */ The highlighting here is wrong. It starts at the line before the comment, actually. While both diffs show a blank line there, the diff viewer shows a replace line. This implies that the lines are not equal. I suspect there's a line termination issue here. Why it starts on that line, I do not know. Perhaps there's something screwy in this particular change with the newlines. We would need a good, self-contained test case for this.
-
+ Confirmed -
+ Interdiffs + Component-DiffViewer
I think this is actually something that had been fixed and just wasn't caught up because our version at VMware was old. New review requests shouldn't have this problem.
-
- Confirmed + Fixed