Created attachment 463960 [details] [diff] [review] Patch to remove hardcoded italics I believe there are 2 things wrong in get_bug_link in Bugzilla/Template.pm: #1, it hardcodes italics instead of a CSS class that can be skinned #2, it does a string match on UNCONFIRMED to write out that styling I am attaching a patch that adds css class and styles them so behavior stays the same.
Also note the html/styling should really be in a template, but this 1/2 step gets it closer and is good enough for now.
Christian: Generally, you want to ask for review from somebody specific, or your review will probably never get done. I'm not sure if you've read over the Developers process, but if you haven't, it's here: http://wiki.mozilla.org/Bugzilla:Developers
Yep, I was reading http://www.bugzilla.org/docs/reviewer-list.html right now to figure out who...you guys are quick ha.
Comment on attachment 463960 [details] [diff] [review] Patch to remove hardcoded italics However, since I'm here, I'll do the review. :-) >=== modified file 'Bugzilla/Template.pm' >+ my $css_classes = ["bz_bug_link"]; That should be @css_classes instead of an arrayref, since you're always using it like an array and then dereferencing it. Other than that, this looks like a great cleanup, so just attach a new patch with that fixed and it will be r+.
Attachment #463960 - Flags: review? → review-
Created attachment 463961 [details] [diff] [review] Same patch without using an arrayref Ok, thanks for the speedy review!
Comment on attachment 463961 [details] [diff] [review] Same patch without using an arrayref Looks good. Would prefer a p0 patch next time.
Attachment #463961 - Flags: review?(mkanat) → review+
Ok, I now see https://wiki.mozilla.org/Bugzilla:Patches says p0..will do next time.
All righty. Do you want to work to get commit access for this patch, or should I just commit it myself?
We don't use checkin-needed, we just have approval flags.
You can just commit, if you guys will possibly grant me commit access I'll work towards that tomorrow, though I understand if I need more patches under my belt.
I'll commit this one, and then perhaps for future patches we'll work toward getting you commit access.
Thanks for the patch, Christian! :-) Committing to: bzr+ssh://bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/Template.pm modified skins/standard/global.css Committed revision 7436.
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.