Bug 46192

Summary: Add link to bug to review page
Product: WebKit Reporter: Darin Adler <darin>
Component: WebKit WebsiteAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, ap, cfleizach, commit-queue, darin, eric
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
dev extension
none
Patch eric: review+, eric: commit-queue+

Description Darin Adler 2010-09-21 09:39:56 PDT
I’d really like to have a link to the bug on the review page!
Comment 1 Adam Barth 2010-09-21 11:46:36 PDT
There are three (small) problems that need to be solved to make that happen:

1) Adding the link at a time when we know the bug_id.  We know the bug_id in the context of this function:
http://trac.webkit.org/browser/trunk/BugsSite/code-review.js#L225

2) Deciding where to display the link.  I think putting it on the toolbar makes sense, but the toolbar is getting a bit crowded.  Maybe we should move the help text elsewhere?

3) The CSS for hyperlinks in PrettyPatch is overbroad, which means it would apply to the bug link too.  We probably need to tighten down the CSS for hyperlinks so the bug link isn't the wrong font/size.
Comment 2 Adam Barth 2010-09-21 11:48:38 PDT
Created attachment 68267 [details]
dev extension

Here's the extension I use to develop the tool.  Basically, it substitutes the existing code-review.js on the live site with the one in the extension.  That way you can hack on the script and see the effects on the live site at the same time.
Comment 3 Darin Adler 2010-09-21 12:32:58 PDT
(In reply to comment #1)
> 1) Adding the link at a time when we know the bug_id.  We know the bug_id in the context of this function:
> http://trac.webkit.org/browser/trunk/BugsSite/code-review.js#L225

Now that I know about this it won’t be too hard.

> 2) Deciding where to display the link.  I think putting it on the toolbar makes sense, but the toolbar is getting a bit crowded.  Maybe we should move the help text elsewhere?
> 
> 3) The CSS for hyperlinks in PrettyPatch is overbroad, which means it would apply to the bug link too.  We probably need to tighten down the CSS for hyperlinks so the bug link isn't the wrong font/size.

When doing the work myself, I used the web inspector to insert the link and it looked fine. For both (2) and (3) above, it was OK the way I had it. I figured others might later want to refine further, but I think just putting the bug before the instructions is fine. And the style for links looked OK to me.

Assuming it’s a Chromium extension, I’ll have to make it work in Safari, but thanks very much for showing me that extension.
Comment 4 Adam Barth 2010-09-21 17:20:41 PDT
> Assuming it’s a Chromium extension, I’ll have to make it work in Safari, but thanks very much for showing me that extension.

Yeah, it's a Chrome extension, but should give you the general idea.
Comment 5 Adam Barth 2010-09-22 13:49:12 PDT
*** Bug 46282 has been marked as a duplicate of this bug. ***
Comment 6 Adam Barth 2010-09-22 20:39:51 PDT
Created attachment 68495 [details]
Patch
Comment 7 Eric Seidel (no email) 2010-09-22 20:41:06 PDT
Comment on attachment 68495 [details]
Patch

OK.
Comment 8 Adam Barth 2010-09-22 20:42:21 PDT
Committed r68118: <http://trac.webkit.org/changeset/68118>
Comment 9 Alexey Proskuryakov 2010-09-25 15:52:41 PDT
*** Bug 46498 has been marked as a duplicate of this bug. ***