Fix numerous problems with handling diff viewer URLs and navigation.
Review Request #7023 — Created March 8, 2015 and submitted
We've had some breakages in the diff viewer URL handling, due to both recent and older changes. Recently, a change landed that included the diff revision in the URL when clicking View Diff, but this code wasn't Local Site-aware, breaking URLs when Local Sites were involved. Also, two different revisions of that change were landed, resulting in some duplicate code appearing in different spots. There was also a usability issue with encoding it in the URL for View Diff. If a user was looking at a review request, reading through reviews, and clicked View Diff, they were not guaranteed to see the latest diff. The author may have since uploaded a new diff, which the reviewer would end up easily missing. Going back further, we also had problems with URL histories and pagination, due to some attempts at fixes for Backbone converting the hash to a URL. This change addresses all these problems. The server-side code now links to the proper URL, without the revision, but with a Local Site-safe ID. The revision is included client-side in the JavaScript code during the initialization of the page. That URL includes the hash and query string, and doesn't make use of the other tricks we were using that broke the back button. The result is that the latest diff is always shown when clicking View Diff, and that the URL will end up containing the diff revision, without breaking the back button, pagination, or anchors.
Tested clicking View Diff. Saw that it initially goes to
diff/
, which
then turns intodiff/<latest_revision>/
.Tested clicking back, and saw it go back to the diff viewer.
Saw that the anchor was always preserved in the new URL, and that adding it
didn't affect the back button negatively.Tested a diff with pagination. Navigated through a few pages, and then clicked
Back until I got back to the review request page. It worked completely as
expected.Unit tests pass.