Bug 52747

Summary: [reviewtool] Add a link for annotated trac page on review page
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: WebKit WebsiteAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Enhancement CC: abarth, aroben, eric, mihai, ojan, tony
Priority: P3    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch
abarth: review+
screenshot none

Description Ryosuke Niwa 2011-01-19 14:39:01 PST
This is a feature request.

I often have to go look up trac logs to see when and why certain code is added.  It'll be really useful if review page had a link to a trac page with annotation with the right line number.
Comment 1 Ojan Vafai 2011-01-19 14:42:29 PST
I wonder if we could also include links for the annotation and revision log. It's a pain to load trac just to click on the link you really want to get to. To avoid clutter, we could only display the links when you mouseover the filename H1 element.
Comment 2 Adam Barth 2011-01-19 14:46:36 PST
Yeah, I bet there's a bunch of cool stuff we could do along these lines.
Comment 3 Ojan Vafai 2011-01-19 16:51:12 PST
Created attachment 79524 [details]
Patch
Comment 4 Adam Barth 2011-01-19 18:41:42 PST
Comment on attachment 79524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79524&action=review

Screenshot?  :)

> Websites/bugs.webkit.org/code-review.js:346
> +  function tracLinksHtml(file_name, url_hash) {
> +    return '<a href="http://trac.webkit.org/browser/trunk/' + file_name + '?annotate=blame' + url_hash + '" target="_blank">annotate</a>' +
> +        '<a href="http://trac.webkit.org/log/trunk/' + file_name + '" target="_blank">revision log</a>';
> +  }

You haz the XSS.  file_name isn't trusted!  Please use the DOM to construct these links.
Comment 5 Ojan Vafai 2011-01-20 11:04:13 PST
Created attachment 79619 [details]
Patch
Comment 6 Ojan Vafai 2011-01-20 11:06:17 PST
Comment on attachment 79524 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=79524&action=review

>> Websites/bugs.webkit.org/code-review.js:346
>> +  }
> 
> You haz the XSS.  file_name isn't trusted!  Please use the DOM to construct these links.

Yikes. Sorry for the n00b.
Comment 7 Ojan Vafai 2011-01-20 11:06:30 PST
Created attachment 79620 [details]
screenshot
Comment 8 Adam Barth 2011-01-20 12:25:57 PST
Comment on attachment 79619 [details]
Patch

Thanks!  Very cool.
Comment 9 Ryosuke Niwa 2011-01-20 12:26:31 PST
Thanks for the feature!
Comment 10 Ojan Vafai 2011-01-20 12:31:02 PST
Committed r76270: <http://trac.webkit.org/changeset/76270>