User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b) Gecko/20030825 Build Identifier: Well, if you check Print-Ready version of any bug, i think you'll agree, that it is not quite readable. I'll attach another version of it in a second. I think, that there is also no special reason, why the print-ready version should use another cgi file than edit bug does. Reproducible: Always Steps to Reproduce:

Ignore headers - focus on the content.

I like your display. :) Are you interested in proposing a patch?

i would love to make a patch, still some questions need to be solved. for initial i would propose to fix print ready template, and leave other stuff (mentioned in bug 135595) the problem is default values - there is no sense of displaying "Version: unspecified" or "Priority: ---". Still, hardcoded dealing with this is rather lame. If we do not target this question now, i could try to make rearranged version of print-ready.

(In reply to comment #3) > for initial i would propose to fix print ready template, and leave other stuff > (mentioned in bug 135595) Sure. No problem. > the problem is default values - there is no sense of displaying "Version: > unspecified" or "Priority: ---". Why not? This makes perfect sense to me. These are informations meaning "the version where this bug appeared is unknown or has not been given". I have many bugs assigned to me; I want these informations in all cases. I added a dependency to a bug which asks to display references to dependent bugs too. You have them already in your beautiful "screenshot", so I guess it's not a problem for you to include these two fields in the patch as well. ;)

first version. i am not clear about how far i can go with styling, so i din't apply much style. feel free to take it apart. i am acknowledged, that i'm not wrapping all lines at 80 column - that i will fix later, together with other stuff. also - styles currently are applied directly to table, td and th tags - that will be changed to application to classes

I just applied your patch locally... The display looks good. Feel free to style it a bit more, especially the background color of the "header" which I really like in your "snapshot". CC'ing myk who is the UI module owner.

I haven't had a chance to look at the patch, but note that we're in the process of converting all pages to structural HTML and stylistic CSS, and we should make sure that this work is compatible with that effort and general direction.

now it runs smooth through tests. added gray background. somebody should really review it, because i'm not quite sure that i understood fully what myk said.

I think myk said that we should aim towards a direction where we would use only HTML Strict + CSS style formatting. As far as I can see your patch conforms to myk's comment.

See also bug 110152.

(In reply to comment #8) I really like the display. A few (minor) points however: - could you please add the URL field? Even if this page is printed, having a URL may be useful, for reference. - The creation date of the bug should be the last item in your gray box, instead of being displayed in the middle of the page below the box. - I think the assignee, reporter and QA contact should be displayed in the right half of the gray box, at the same level than the product, component and status: Product: Bugzilla Assignee: Toms Baugis <toms@myrealbox.com> Component: User Interface Reporter: Toms Baugis <toms@myrealbox.com> Status: ASSIGNED QA Contact: MattyT <mattyt-bugzilla@tpg.com.au> Severity: enhancement Priority: -- .... Toms, what do you think about my last suggestion?

Oh... forget my previous comment. The URL wasn't displayed because I didn't specify one. Now I can see it. :) About having the assignee, QA and reporter on the right, I finally wonder if we have enough place. Maybe doing some tests could answer this question. This left us with the creation date, which is a nit, really.

Comment on attachment 189410 [details] [diff] [review] vertical layout, version 0.2 >Index: template/en/default/bug/show-multiple.html.tmpl >+ <h1> >+ [% terms.Bug %] >+ <a href="show_bug.cgi?id=[% bug.bug_id %]">[% bug.bug_id %]</a> >+ [% " [$bug.alias]" FILTER html IF Param("usebugaliases") AND bug.alias %]: >+ >+ [% bug.short_desc FILTER html %] >+ </h1> Nit: I wonder if having the bug summary as the first item in the gray box wouldn't be better, instead of this big title, especially for printing. Moreover, could you please leave the IF block as: [% IF Param("usebugaliases") AND bug.alias %] ([% bug.alias FILTER html %]) [% END %] >+ <th>[% field_descs.bug_status FILTER html %]:</th> >+ <td> >+ [% bug.bug_status FILTER html %] >+ [% " - $bug.resolution" FILTER html IF bug.bug_status == "RESOLVED" >+ OR bug.bug_status == "VERIFIED" >+ OR bug.bug_status == "CLOSED" %] >+ </td> Instead of the IF test, you could directly write this as: >+ <th>[% field_descs.bug_status FILTER html %]:</th> >+ <td> >+ [% bug.bug_status FILTER html %] >+ [%+ bug.resolution FILTER html %] >+ </td> This way, you don't care whether the resolution is blank or not. >+ [% IF Param('useqacontact') %] >+ <th>[% field_descs.qa_contact FILTER html %]:</th> > <td> >+ [% bug.qa_contact.identity FILTER html %]</td> >+ [% END %] Please leave <td> and [% bug.qa_contact.identity FILTER html %]</td> on the same line. Moreover, you have to add <tr></tr> tags in this IF block. >- <tr> >- <td colspan="4"> >- <b>Opened:</b>&nbsp; >- [% bug.creation_ts FILTER time %] >- </td> >- </tr> As I said in a previous comment, the creation date should be the last item. Please don't remove it. These comments are easy to addressed, so the next patch should be the final one. I want to say that the display is really beautiful and looks very professional. I love it! :)

The trunk is now frozen to prepare Bugzilla 2.22. Enhancement bugs are retargetted to 2.24.

Toms, are you still interested in this bug? There is not a lot of work left to get it ready. And this would be a great UI improvement.

oh i am so sorry - currently i'm pretty overloaded at work. it would be great if somebody else could finish it! i hope to get back to bugzilla next year.

Toms: Do you have time to fix it? If so, I'll reassign to you. If not, I want to fix this. (Reassigning to me first so I won't forget about this bug.)

bkor, he said he was busy so go on! His last patch was pretty good, see my review in comment 13. There shouldn't be a lot of work left, except maybe unbitrotting his patch. ;)

Changes: - unbitrotted (resolution_descs / status_descs) - fixes nits - only shows creation time if there are no comments (otherwise bug/comments.html.tmpl will show the creation time for the first comment) - shows both dependson/blocked if there is at least a bug in one of them (most bugs to not have these fields, but I want both shown if at least one field is filled in) - add Toms Baugis to contributor of show-multiple.html.tmpl (he wrote most of the patch, changing most of the file) - get rid of h1

Changes: - 2 columns - flags - attachments

Comment on attachment 209384 [details] [diff] [review] Patch v4: Bugzilla CVS 2006-01-23 This patch looks good. There are still some points to fix as discussed on IRC. I will review the updated patch.

Changes: - passes runtests.pl - only show keywords if the bug has them - attachment flags joined with ", " - space between left and right columns using css - show rightcell next to opening date (not always shown) - add my name to contributors

Here is how patch v5 looks like. I love it! :)

Comment on attachment 209430 [details] [diff] [review] Patch v5 >Index: template/en/default/bug/show-multiple.html.tmpl >+ [% IF bug.use_keywords && bug.keywords %] >+ [% rightcells.push('keywords') %] >+ [% END %] Nit: You don't really care whether the installation has some defined keywords. What interests us here is whether the bug itself has some keywords. So [% bug.keywords %] is enough. >+ [% IF bug.bug_file_loc %] > <tr> >+ <th>[% field_descs.bug_file_loc FILTER html %]:</th> >+ <td colspan="3"> >+ [% IF bug.bug_file_loc >+ AND NOT bug.bug_file_loc.match("^(javascript|data)") %] Nit: the second IF test doesn't need to verify 'bug.bug_file_loc' as you already tested it in the first IF test. You could write: [% UNLESS bug.bug_file_loc.match("^(javascript|data)") %] >+ [% IF !bug.longdescs.size %] > <tr> >+ <th>[% field_descs.creation_ts FILTER html %]:</th> >+ <td>[% bug.creation_ts FILTER time %]</td> >+ >+ [% PROCESS rightcell %] > </tr> > [% END %] Nit: AFAIK, there is *always* at least one comment, even when 'commentonecreate' is turned off. So this block is probably useless and should be removed. >+ [% IF flag.setter %] >+ [% flag.setter.nick FILTER html %]: >+ [% END %] Nit: a flag *always* has a setter, so there is no need to check whether one is defined; you already know there is one. This test is useless. This concerns both bug and attachment flags. >+ <br /> Nit: no need to try to be XHTML compliant. >+[% BLOCK row %] >+ <tr> >+ <th>[% field_descs.${cell} FILTER html %]:</th> >+ <td[% " colspan=3" IF fullrow %]>[% bug.${cell} FILTER html %]</td> >+ [% PROCESS rightcell IF !fullrow %] >+ </tr> >+[% fullrow = 0 %] >+[% END %] Nit: fix the indentation of [% fullrow = 0 %] (add two whitespaces). >+ [% IF flag.setter %] >+ [% flag.setter.nick FILTER html %]: >+ [% END %] Nit: same comment as above. >+ [% ELSIF name != "" %] >+ <th class="rightcell">[% field_descs.${name} FILTER html %]:</th> >+ <td>[% bug.${name} FILTER html %]</td> >+ [% END %] Nit: this is not enough. If name == "", you still have to add <td>&nbsp;</td> so that each row of the table has 4 columns. Some web browsers do not like when the number of columns is not consistent (you may have some UI bugs). Despite I marked all my comments as nits, I would like to see an updated patch with all these nits fixed anyway. Please fix them and carry forward my r+ (I trust you). r=LpSolit

Carrying forward r+ as agreed. > Nit: You don't really care whether the installation has some defined keywords. > What interests us here is whether the bug itself has some keywords. So [% > bug.keywords %] is enough. fixed. > >+ [% IF bug.bug_file_loc > >+ AND NOT bug.bug_file_loc.match("^(javascript|data)") %] > > Nit: the second IF test doesn't need to verify 'bug.bug_file_loc' as you > already tested it in the first IF test. You could write: > > [% UNLESS bug.bug_file_loc.match("^(javascript|data)") %] Fixed. Used IF and changed the order (for clarity; do not like UNLESS + ELSE). > Nit: AFAIK, there is *always* at least one comment, even when > 'commentonecreate' is turned off. So this block is probably useless and should > be removed. Checked back until Bugzilla 2.10 sources and it is indeed the case. Thought it wasn't because the changed importxml.pl used on b.g.o had the reporter under a different id than the description. Removed it. > >+ [% IF flag.setter %] > >+ [% flag.setter.nick FILTER html %]: > >+ [% END %] > > Nit: a flag *always* has a setter, so there is no need to check whether one is > defined; you already know there is one. This test is useless. This concerns > both bug and attachment flags. Took this from existing Bugzilla sources. Checked current Bugzilla and it indeed always stores a setter plus assumed one is always stored. This code was introduced in bug 98801 between patch 30 and patch 31, but not because of a review comment. This template code exists in two more files, I'll file a new bug for that. > Nit: no need to try to be XHTML compliant. Fixed. > Nit: fix the indentation of [% fullrow = 0 %] (add two whitespaces). Fixed. > >+ [% IF flag.setter %] > >+ [% flag.setter.nick FILTER html %]: > >+ [% END %] > > Nit: same comment as above. Fixed. > >+ [% ELSIF name != "" %] > >+ <th class="rightcell">[% field_descs.${name} FILTER html %]:</th> > >+ <td>[% bug.${name} FILTER html %]</td> > >+ [% END %] > > Nit: this is not enough. If name == "", you still have to add <td>&nbsp;</td> > so that each row of the table has 4 columns. Some web browsers do not like when > the number of columns is not consistent (you may have some UI bugs). Fixed.

Said in previous comment that I changed UNLESS / ELSE order to IF / ELSE. I did do that, but didn't rediff the patch. Both versions work, but IF / ELSE is much better (clarity).

Checking in skins/standard/show_multiple.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/show_multiple.css,v <-- show_multiple.css new revision: 1.3; previous revision: 1.2 done Checking in template/en/default/filterexceptions.pl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/filterexceptions.pl,v <-- filterexceptions.pl new revision: 1.62; previous revision: 1.61 done Checking in template/en/default/bug/show-multiple.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/show-multiple.html.tmpl,v <-- show-multiple.html.tmpl new revision: 1.27; previous revision: 1.26 done

Added to the relnotes currently attached to bug 349423.