1317982 - Reviewboard URLs lack trailing slash after navigating to filenames, causing broken "download diff" links

Status: RESOLVED FIXED

(MozReview Graveyard :: Review Board: DiffViewer, defect)

Description

[:Gijs (he/him)]

STR:

1. open e.g. https://reviewboard.mozilla.org/r/84386/diff/9/
2. click a filename
3. click 'download diff' at the top

ER:

you get the diff of version 9

AR:
you get the diff of the latest version


Possible fixes include fixing the filename navigation to not drop the /, and fixing the 'download diff' stuff to deal with URLs without the trailing / (which are, needless to say, also legal and correct etc.).

Comment 1

[:Gijs (he/him)]

(In reply to :Gijs Kruitbosch (PTO Nov. 17-20 inclusive) from comment #0)
> STR:
> 
> 1. open e.g. https://reviewboard.mozilla.org/r/84386/diff/9/
> 2. click a filename

Er, so missing bits: the links for the filename claim to point to the same URL as step 1 with the addition of "#n" where n is the index of the filename (so 1 for the first file, 2 for the second, etc.).

However, after clicking such a link, the URL actually ends up being ".../diff/9#n" instead of ".../diff/9/#n" - note that the trailing / has gone missing.

> 3. click 'download diff' at the top
> 
> ER:
> 
> you get the diff of version 9
> 
> AR:
> you get the diff of the latest version
> 
> 
> Possible fixes include fixing the filename navigation to not drop the /, and
> fixing the 'download diff' stuff to deal with URLs without the trailing /
> (which are, needless to say, also legal and correct etc.).

Comment 2

[:glob ✱]

Attachment: diffviewer: Don't treat in-page file navigation as page navigation (bug 1317982);

Clicking on a file to scroll to it breaks the "download diff" button when
viewing an interdiff.  This happens because the anchor click is treated as a
full navigation event which generates the URL from the anchor target.  As
filename clicks aren't full navigation clicks we end up in the wrong place.

Fix this by not triggering updating navigation urls for file-anchor clicks.

Review commit: https://reviewboard.mozilla.org/r/94386/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/94386/

Comment 3

[David Walsh :davidwalsh]

Comment on attachment 8812755 [details]
diffviewer: Don't treat in-page file navigation as page navigation (bug 1317982);

https://reviewboard.mozilla.org/r/94386/#review94964

Works for me!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/webtools/reviewboard/rev/a85107cd508b
diffviewer: Don't treat in-page file navigation as page navigation ; r=davidwalsh