What steps will reproduce the problem? 1. Upload a diff, publish. Upload another diff, publish. 2. Look at the interdiff between 1 and 2 (URL will be blah/r/81/diff/1-2/) and make a comment. 3. Reply to the comment you just made on the review request page. What is the expected output? What do you see instead? The reply is linked to the wrong interdiff. The link in the email says diff/1/ instead of diff/1-2/ and the view tagged is in diff/1/ as well. What operating system are you using? What browser? I'm using reviewboard rev 1497. Please provide any additional information below.
This is a bit confusing, but the reason for this is that when you have an interdiff where there's a file in one diff but not the other, we treat the file as a stand-alone file and not part of an interdiff. There's nothing to compare it to, as we have no revision to look up the other side of that. So the URL generated isn't an interdiff URL. The comment is still in the right place, though. Interdiffs are in some ways a little wonky. We might have to get to that sometime, but for now, this isn't really incorrect in the current design. I've verified that this happens when adding a comment to a diff in that situation, but when adding to a file that has actually changed across revisions, it's correct.
+ Interdiffs + Component-DiffViewer
I don't think you reproduced it correctly. The 2 patch files are identical, except for maybe 1 character so it actually renders content on the interdiff page. The part of your above comment "when you have an interdiff where there's a file in one diff but not the other" does not apply. When I make a comment on an interdiff page like the following URL: http://blah.com/r/179/diff/1-2/#index_header I get a RB email that has a link to the comment <http://blah.com/r/179/#comment813> I look at the review page (not the interdiff, on the review request page) and reply to the comment I just made. Another email is sent out that creates a link that looks like the following: <http://blah.com/r/179/diff/1/?file=1908#file1908line39> This is the wrong link! The correct one links to the interdiff: <http://blah.com/r/179/diff/1-2/?file=1908#file1908line39> I wish we had the power to reopen issues that aren't resolved. =)
Oh, I'm sorry. I did misunderstand slightly, and thought it was this other issue.
- NotABug + Confirmed
Okay, pretty sure this is figured out. Fixed in r288dd43. This will be available for 1.0.4.
- Confirmed + Fixed