RESOLVED FIXED52747
[reviewtool] Add a link for annotated trac page on review page
https://bugs.webkit.org/show_bug.cgi?id=52747
Summary [reviewtool] Add a link for annotated trac page on review page
Ryosuke Niwa
Reported 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.
Attachments
Patch (7.35 KB, patch)
2011-01-19 16:51 PST, Ojan Vafai
no flags
Patch (7.51 KB, patch)
2011-01-20 11:04 PST, Ojan Vafai
abarth: review+
screenshot (105.79 KB, image/png)
2011-01-20 11:06 PST, Ojan Vafai
no flags
Ojan Vafai
Comment 1 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.
Adam Barth
Comment 2 2011-01-19 14:46:36 PST
Yeah, I bet there's a bunch of cool stuff we could do along these lines.
Ojan Vafai
Comment 3 2011-01-19 16:51:12 PST
Adam Barth
Comment 4 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.
Ojan Vafai
Comment 5 2011-01-20 11:04:13 PST
Ojan Vafai
Comment 6 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.
Ojan Vafai
Comment 7 2011-01-20 11:06:30 PST
Created attachment 79620 [details] screenshot
Adam Barth
Comment 8 2011-01-20 12:25:57 PST
Comment on attachment 79619 [details] Patch Thanks! Very cool.
Ryosuke Niwa
Comment 9 2011-01-20 12:26:31 PST
Thanks for the feature!
Ojan Vafai
Comment 10 2011-01-20 12:31:02 PST
Note You need to log in before you can comment on or make changes to this bug.