Fix and standardize building of rendered diff URLs.

Review Request #12292 — Created May 20, 2022 and submitted — Latest diff uploaded


Review Board


DiffReviewable has a private function, _buildRenderedDiffURL(), for
generating the URL for a rendered diff, which constructs the URL based
on some instance state. There's then a couple of functions that use
that and try to modify it to suit their needs.

This falls apart when a query string is returned. Some code assumes it
can append a path to that, which fails in the case where someone's
trying to expand code in the diff viewer while looking at a commit with
a base commit ID.

Now, _buildRenderedDiffURL() takes in all the parameters needed to
construct the URL, and does so correctly. Callers only need to pass in
what they need, and never need to modify the result.

The URL also always contains the TEMPLATE_SERIAL now in the query
string, to ensure proper cache busting (and to further discourage
appending paths to the URL).

This should ensure we always have reliable, tested URLs going forward.

Tested rendering and expanding chunks in:

  • Normal diffs
  • Interdiffs
  • Commits/ranges
  • Commits with interdiffs