585490 - Italics for UNCONFIRMED bug links are hardcoded in Bugzilla/Template.pm

Status: RESOLVED FIXED

(Bugzilla :: User Interface, defect)

Description

[LegNeato]

Attachment: 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.

Comment 1

[LegNeato]

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.

Comment 2

[Max Kanat-Alexander]

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

Comment 3

[LegNeato]

Yep, I was reading http://www.bugzilla.org/docs/reviewer-list.html right now to figure out who...you guys are quick ha.

Comment 4

[Max Kanat-Alexander]

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+.

Comment 5

[LegNeato]

Attachment: Same patch without using an arrayref

Ok, thanks for the speedy review!

Comment 6

[Max Kanat-Alexander]

Comment on attachment 463961 [details] [diff] [review]
Same patch without using an arrayref

Looks good.

Would prefer a p0 patch next time.

Comment 7

[LegNeato]

Ok, I now see https://wiki.mozilla.org/Bugzilla:Patches says p0..will do next time.

Comment 8

[Max Kanat-Alexander]

All righty. Do you want to work to get commit access for this patch, or should I just commit it myself?

Comment 9

[Max Kanat-Alexander]

We don't use checkin-needed, we just have approval flags.

Comment 10

[LegNeato]

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.

Comment 11

[Max Kanat-Alexander]

I'll commit this one, and then perhaps for future patches we'll work toward getting you commit access.

Comment 12

[Max Kanat-Alexander]

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.