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

RESOLVED FIXED in Bugzilla 4.2

Status

()

RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: christian, Assigned: christian)

Tracking

unspecified
Bugzilla 4.2
Bug Flags:
approval +

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

8 years ago
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.
(Assignee)

Updated

8 years ago
Assignee: ui → clegnitto
(Assignee)

Updated

8 years ago
Attachment #463960 - Flags: review?
(Assignee)

Comment 1

8 years ago
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.
(Assignee)

Updated

8 years ago
Status: NEW → ASSIGNED

Updated

8 years ago
Attachment #463960 - Attachment is patch: true
Attachment #463960 - Attachment mime type: application/octet-stream → text/plain

Comment 2

8 years ago
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
(Assignee)

Comment 3

8 years ago
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

8 years ago
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-

Updated

8 years ago
Target Milestone: --- → Bugzilla 4.2
(Assignee)

Comment 5

8 years ago
Created attachment 463961 [details] [diff] [review]
Same patch without using an arrayref

Ok, thanks for the speedy review!
Attachment #463960 - Attachment is obsolete: true
Attachment #463961 - Flags: review?(mkanat)

Comment 6

8 years ago
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+

Updated

8 years ago
Flags: approval+
(Assignee)

Comment 7

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

Comment 8

8 years ago
All righty. Do you want to work to get commit access for this patch, or should I just commit it myself?
(Assignee)

Updated

8 years ago
Keywords: checkin-needed

Comment 9

8 years ago
We don't use checkin-needed, we just have approval flags.
Keywords: checkin-needed
(Assignee)

Comment 10

8 years ago
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

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

Comment 12

8 years ago
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.