556692 - MXR's link to Hg blame drops the anchor

Status: RESOLVED FIXED

(Webtools Graveyard :: MXR, defect)

Description

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Maybe a fix?

STR:

1) MXR for some code in a file; end up at line foo (e.g. nsChildView.mm#5160)
2) Scroll back to the top of the page and click "Hg Blame"
3) When http://hg.mozilla.org/releases/mozilla-1.9.2/annotate/32e8189050ad/widget/src/cocoa/nsChildView.mm finally loads, wonder where in the @$@#$#@ you line is.
4) Go back to the previous page and find the line
5) Forget that Hg uses #l5160 and append #5160 to the URL
6) Remember your mistake and replace #5160 with #l5160
7) Scream
8) Repeat steps 1 and 2 with the "seamonkey" root (s/Hg/CVS/)
9) End up right exactly at http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/widget/src/cocoa/nsChildView.mm&rev=1.371&mark=5160#5160
10) Scream again!
11) view-source: on the MXR pages for Hg and CVS, notice incredibly awesome JS is missing
12) Scream yet again!

(Sadly, there's no severity for "Highly Annoying and Scream-Inducing")

For whatever insane reason, the "Hg template" for MXR is missing the awesome little block of JS (bug 356659) that the "CVS template" has.

Can we please get that added back (at least the var anchor part; I think Hg is lame and doesn't do "mark"), using #l instead of just #?

[...]hack, hack, hack[...]

I think this patch should do it, but JS is not a language I even pretend to know, nor do I have an MXR instance to actually test this fix.

Comment 1

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Comment on attachment 436645 [details] [diff] [review]
Maybe a fix?

Er, bleh, I left off the "l", so this definitely won't work.

document.location.hash seems to return the entire #5610 thing, so this would need to actually parse it apart and insert the "l" between the "#" and the linenumber, and that's more JS than I can probably do.

Comment 2

[timeless]

Comment on attachment 436645 [details] [diff] [review]
Maybe a fix?

don't you need that 'l' thing? and if you aren't using event, i'd probably skip including it

Comment 3

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Attachment: Fix that works

(In reply to comment #2)
> (From update of attachment 436645 [details] [diff] [review])
> don't you need that 'l' thing? 

Yeah, I forgot I hadn't fixed it until after I'd already attached the patch. :(

> and if you aren't using event, i'd probably skip including it

I believe it is necessary for a patch that actually works (which the previous patch wasn't).

This patch should work, though; I pasted the JS into a local saved copy of an MXR file view, added the onFoo events to the Blame URL, and tested both clicking a linenumber before clicking Blame and also arriving at the page with a linenumber specified before clicking Blame; in both cases I ended up at the appropriate line on hgweb.

(As for mark, https://developer.mozilla.org/en/Mercurial/Desired_Features#Allow_marking_lines_of_.22file.22_and_.22annotate.22_view indicates we're still waiting for hgweb to support that, so someone else can add back passing the mark to hgweb once hgweb knows what to do with it :P )

Comment 4

[timeless]

Comment on attachment 436825 [details] [diff] [review]
Fix that works

yeah, this works.

Comment 5

[timeless]

http://hg.mozilla.org/webtools/mxr/rev/be6b007d0c29