• 
      

    Fix numerous problems with handling diff viewer URLs and navigation.

    Review Request #7023 — Created March 8, 2015 and submitted

    Information

    Review Board
    release-2.0.x
    22955d8...

    Reviewers

    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 into diff/<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.

    reviewbot
    1. Tool: Pyflakes
      Processed Files:
          reviewboard/reviews/views.py
      
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
      
      Tool: PEP8 Style Checker
      Processed Files:
          reviewboard/reviews/views.py
      
      Ignored Files:
          reviewboard/templates/reviews/review_detail.html
          reviewboard/static/rb/js/pages/views/diffViewerPageView.js
      
      
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-2.0.x (42aee3b)