use CSS to style informational message at bottom of banner

RESOLVED FIXED in Bugzilla 2.20

Status

()

enhancement
RESOLVED FIXED
19 years ago
7 years ago

People

(Reporter: jacob, Assigned: gphat)

Tracking

2.13
Bugzilla 2.20
Bug Flags:
blocking2.20 -
blocking2.18 -

Details

(Whiteboard: see comment 75 and onward for the current direction of this bug)

Attachments

(4 attachments, 11 obsolete attachments)

148.46 KB, patch
Details | Diff | Splinter Review
9.19 KB, text/css
Details
134.15 KB, patch
Details | Diff | Splinter Review
3.40 KB, patch
justdave
: review-
Details | Diff | Splinter Review
As time passes I've been noticing more and more bugs where people are requestion
that one thing or another should have color of some sort associated with it.  It
seems to be that the best/easiest way to do this would be to hack in some
support for CSS.  We could then either have a "bugzilla stylesheet" or (probably
better) allow users to specify their own style sheet.
Blocks: 28884, 36013, 57845
for the resolved => strikeout
if we used css, we should provide a default CSS sheet that uses strikeout

and an alternate that uses something else perhaps lightgray background

> how does the user pick one?
mozilla has a view menuitem that allows you to pick
[although it's usually broken :) ] <-- by offering this feature in bugzilla, we 
encourage people to notice and bug about the feature in mozilla. :)
Keywords: helpwanted
Hardware: PC → All
I've been thinking about this especially.

In the BZ3 world we don't want users to have to be able to hack code to get
their preferred style, and they do currently (eg bugzilla.gnome.org).

We can't really move to templates because they're fairly incompatible with the
fact we want to allow what fields that appear to be customisable.

CSS would seem to be the solution to that.

Of course the browser support issue is there, but hopefully we can at least get
the default look to be fine in all browsers even if they have to gracefully
degrade and our support for older browsers regresses partially.  I know Hixie
expressed some concerns we might not be able to do that.

Any admin changes beyond the default look are on their own head.

I'll move this to BZ3 as I don't consider this is appropriate for the BZ2 line.
Assignee: tara → ian
Component: Bugzilla → Bugzilla 3
QA Contact: matty → ian
Target Milestone: --- → Bugzilla 3.0
This is right in line with the customisable output thingy of bz3.
Taking QA.
QA Contact: ian → matty
I think this is something we should attempt to fit into the 2.x framwork.  I
really shouldn't be terribly difficult (although I really don't know enough
about CSS at the moment to be sure).  There are a few requests for color in
buglists esp. and CSS is the quickest/easiest way to impliment it.  It seems to
me that the best way to impliment this would be to first do it w/just one
default sitewide stylesheet (which could be customized by each installation as
seen fit relatively easily) and then at a later date build in the options for
users to be able to customize their style sheets (or maybe skip that and say if
you want to customize your colors you should use a browser that allows you to
apply your own style sheet [Like the aforementioned View menyitem in Mozilla]).

Assignee: ian → justdave
No longer blocks: 28884, 36013, 57845
Component: Bugzilla 3 → Bugzilla
Target Milestone: Bugzilla 3.0 → ---
Depends on: 28884, 36013, 57845, 66007, 92565
No longer depends on: 36013
Depends on: 36013
I'll attach a patch in a bit which will add a css link into all the bugzilla cgi's.

This is as part of my investigation into colouring bugs in buglists (bug#92565 +
bug#36013), but as this is the css-support bug, I'll start with a little patch here.

Of course, this has no effect whatsoever without any class="" changes to the
code, but it is a start, for discussion:

1) Should there be support for multiple stylesheets?
2) Should it be a global param?
3) Should this script check if the css exists before adding the link to it?
4) Should the css href be url_quoted()?
5) Should there be the normal, standard, as distributed bugzilla css which is
never changed, and then another, 'local' css, which can be used to redefine
stuff for other installations? (which would make merging changes easier) 
I think it'd be better to set a default value for Param('headerhtml') of:
<LINK REL="stylesheet" HREF="bugzilla.css" TYPE="text/css">

Doing it that was allows the administrator to have multiple style sheets (or 
none, if desired). But that's the easy part.  Making bugzilla.css (picking good 
default colors, etc) and hooking the bugzilla pages (making the approiate item 
the correct class) into it is the difficult part.
I've added a lot of css support to our Bugzilla install. I'll do a patch against
2.14, when it is out.

Basically, as I see it, there 2 sorts of css classes to add:

a) classes for things affecting the general layout of the forms and pages, to
enable sites to configure the overall look and feel of stuff. e.g. labels on
forms, headings, links etc.

b) classes for bugs indicating the status of a bug, e.g. that a bug is
'critical', or a bug has the 'patch' keyword associated with it.

Anyway, following are the notes on all the changes I have made, any comments?

------


Notes on my Bugzilla CSS Implementation...


1) Where the CGI output is changed, or the html source updated, the
bugzilla.css has been updated to produce the same results.

2) The Param('bodyhtml') is changed so that instead of setting BGCOLOR,
VLINK etc, a class="bugzilla" is added for all CGI scripts.

3) All normal html files (e.g. index.html) also have this change made to
the BODY tag, so that everything Bugzilla produces has a class of
"bugzilla"

4) The pages: buglist,bugform,newbug,activity and query also have a class each.
On these pages, a <DIV> is used to specify classes "bug-activity", "bug-list",
"bug-form", "bug-enter" and "bug-query" for the whole page. This is so that
styles can be applied on a per-page basis, if desired.

5) Each label/fieldname on the forms (enterbug + showbug) has a class of
"label", and a 2nd class of "<fieldname>-label". 
e.g. class="label component-label", class="label product-label".

6) Each actual field on these forms also has a class according to the
fieldname, e.g. class="component", class="product"

7) On the buglist page, each row is tagged with the following classes: the
severity, the priority and all keywords related to the bug. 
e.g. class = "kw-mostfreq kw-smoketest sev-critical pri-P2"

8) Each cell on the buglist page has the same classes as the row, and also a
class related to the column.
e.g. class = "kw-mostfreq kw-smoketest sev-critical pri-P2 status"
e.g. class = "kw-mostfreq kw-smoketest sev-critical pri-P2 summary"

9) Previously, on the forms, some labels used <B>, and some used <TH> to
achieve the same look. These are all now: class="label"

10) All class names go through a new routine: css_class_name_quote(). This
is used to ensure as much as possible that the class names are
valid for css. (e.g. remove spaces)

11) Most of these changes really only become useful though, when users can
specify their own style sheet. How should this be done? How can this be
done? Should this be another bug? 

Possibilties include: allow a user parameter to specify an url to a
stylesheet, which would be included after the standard one(s), or allow a user
parameter to specify the contents of a <STYLE> tag which could be included in
each page...

12) Of course, this is the sort of thing that people could get used to
depending on for spotting bugs in their lists which they consider
important, so it is *very* important that it is correct.

13) Some bugzilla css examples of what can now be done:

/**
 ** EXAMPLES
 **/

.bug-form .label { /* all labels on the bug-form */ }

.bug-list .severity { /* style for the severity column on buglists */ }

.bug-list .sev-critical { /* style for a whole row containing 
a critical bug */ }

.kw-smoketest.sev-critical, .kw-smoketest.sev-blocker
{ 
  /* critical or blockers with smoketest KW:  */
  background-color: red;
  font-weight: bold;
} 

14) What other styles/classes would be useful?

15) There is a new cgi: styles.cgi, which shows what all the styles look
    like. This can be used as a key to decipher what all the lovely colors
    mean, and also as a way of testing out new styles and changes to
    styles.

16) There is currently just a class for 'user', and 'user-label'. Should
    there be different classes for assigned-to/qa/reporter etc?

17) As well as including a link to 'css/bugzilla.css' in each CGI and
    static html file, I have also included a link to a 'css/local.css'
    stylesheet, and any site-local definitions should go in there.


Target Milestone: --- → Future
-> New Bugzilla Product
Assignee: justdave → myk
Component: Bugzilla → User Interface
Product: Webtools → Bugzilla
Version: other → 2.13
Gavin, 2.14 is out; do you want to resync with the tree and start the merge? 

I'd like to point out I am wondering about the implications of this + bug 86168
which calls for templatizing bugzilla. Do we feel it's time to do the move to
templates and fix these all at once?
Yes, I'm also interested how this would interact with the Template stuff, which
I haven't looked at at all (yet). I'd like to know if there is anything I can do
in the CSS changes to help/ease any future changes to Template usage.

I'm also concerned that this CSS change affects so much of the code that there
could be lots of patches that would then be unusable, and need to be re-done,
which could cause pain. (especially things like the 'make BZ output valid
html/xhtml code' bug.)

But anyway, back to the actual bug, and implementing CSS support...

I've done most of the merge, I'll try and tidy it up and attach something for
comments in the next few days...
Bug#98123 will be needed to provide some, many or none of the following 
parameters:

1) enable css for a user
2) specify a user css
3) specify some inline css styles for a user
Depends on: 98123
Attachment #44110 - Attachment is obsolete: true
OK, I've attached a big patch, which contains a whole load of changes to add CSS
support to Bugzilla. ( A lot of which is probably to be superceded by Template
support, or is out-dated by recent patches and checkins to Bugzilla :(

Anyway, hopefully it is all there, and it works pretty well for me. As I doubt
anyone will ever apply the patch as is, perhaps the most useful bits are the new
routines in globals.pl used to generate all the CSS code on all the other pages.

Also included in this patch, as they are sort of CSS related are pretty complete
fixes for bug#36013, bug#57845, bug#94303, bug#96405, bug#92565, and bug#93220
Don't know if we want to get this in before or after the big templatisation
effort.  If after, this would still be useful as a base to see what has to be
done.
Keywords: helpwantedpatch, review
Target Milestone: Future → Bugzilla 2.16
I suggest we finish templatising first. This should make adding the CSS a whole
lot easier.

Gerv
Gerv> I suggest we finish templatising first. This should make adding the CSS a
Gerv> whole lot easier.

As Templatising and CSS are both GUI features, and both affect just about every
bit of display code, maybe they could both be done at the same time?



Pushing out - if templatisation is going to the wire for 2.16, the CSS feature
will be beyond 2.16. 

We don't want to do them both at once because the amount of changes
templatisation is causing are already scary - and this is just trying to
reproduce the _same_ html but with templates.

Gerv
Target Milestone: Bugzilla 2.16 → Bugzilla 2.18
Depends on: 104247
No longer depends on: 66007, 92911
*** Bug 130225 has been marked as a duplicate of this bug. ***
> 3) Should this script check if the css exists before 
>    adding the link to it?

IMO that shouldn't be necessary if a default stylesheet 
is included in the distribution.  Then again, it wouldn't
be a difficult thing to do, with a -e filetest.  Does 
that have a significant cost in execution time?  

> 11) Most of these changes really only become useful 
> though, when users can specify their own style sheet. 
> How should this be done? How can this be done? Should
> this be another bug? 

I say it should be a separate bug.  (One well worth doing,
but distinct from merely allowing bugzilla sites to style
their own output.)  It should depend on this bug, because
until stuff has classes, the stylesheets have no more power
than existing user customisations in many browsers (e.g.,
user stylesheets in Mozilla).  Once the classes are in,
then there are more interesting possibilities.  

One way to do stylesheets on a per-user basis would be
to allow users to specify a URL for the stylesheet to
use; then if the user is logged in the link could use
that URL instead of the default one.  The URL could be
a file URL if the user so desires and the user's browser
supports getting a stylesheet that way.  Or the user
could put a stylesheet up on a website someplace and 
use that URL.  Bugzilla site admins could also offer 
several stylesheets on their servers that users could 
select from, and make one of them the default for users
that haven't chosen.  
Bugzilla 2.16/2.17 contains support for stylesheets, but needs a further patch...

When templates invoke header.html.tmpl, they should be able to specify a list of
stylesheets rather than being constrained to a single stylesheet.  If anyone
wants to implement this one, please post a comment on this bug asap,  otherwise,
I'll put it on my TODO list.


This patch causes header.html.tmpl to treat the style_url paramtere as a
space-seperated list of stylesheet irls to link instead of as a single url. 
This should be backward compatible.
CSS support doesn't just mean the ability to specify stylesheets, it also means
using structural HTML rather than presentational HTML, in order to improve the
ability to style the HTML.
I agree with Matty. I think we should consider this as a meta bug for the CSS
issues - we have so much to do with structuralizing/classing/id'ing the html stuff. 

I still think Joel's patch is good, but IMO that should be posted as a different
bug (possibly blocking this). Otherwise we'll end up with several patches on
this bug being reviewed and maybe even gradually checked in, but without the
relevant bug never reaching resolved status.
Yep, different bug. And the interface should be a Perl list, not a space-
separated one, and the patch should be small and simple so we can get it into 
2.16 and change the interface before release.

Gerv
Comment on attachment 85927 [details] [diff] [review]
Patch to 2.17 (tip) permitting multiple stylesheets to be included

Obsoileting in favor of bug 148679
Attachment #85927 - Attachment is obsolete: true
Depends on: 148679
This has essentially become the CSS meta bug, so marking it as such. There are
no valid patches to review here either, so updating those keywords too.
Alias: bz-css
Keywords: patch, reviewmeta
I also would really like to see bugzilla shed the "tables as formatting" html it
uses in favor of CSS (at least in the 2.16 that I have now, and reading this, it
sounds like this isn't yet in any new version).
Replacing table formatting with CSS isn't IMO a realistic target at this point.
CSS positioning isn't well enough supported to replace table-based formatting on
complex form-based pages.

While it's true that Bugzilla's approach conceptually ignores the semantic
meaning of a table, there are few or none real problems involved. In particular,
I'm referring to the fact that Bugzilla is decently usable even with a text-only
browser without table support. Sure, especially the bug form could linearize a
lot better, but this enhancement can also be done without including the
additional uncertainty involved in CSS layout of something as complex as this.
Or then we could just wait a few years. :-)
Well, IMHO CSS formatting is increasingly supported, particularly by Mozilla :).
Would it be so bad if Mozilla renderd bugzilla better than other browsers? It's
not like this is a product that makes a profit, and graceful degredation is of
course a key thing to check nomatter how you design the page. A first year
college student I know did the following page for our lab and it seems to work
fairly well all the way back to netscape 4 browsers, maintaining legibility and
logical sense. 

http://www.cognition.olin.edu

Not as complicated as the forms in bugzilla, but reason for hope I think.
Don't take me wrong; I agree that CSS layout rules. However, even though the
page you mentioned is cool by itself, it's far from the complexity of Bugzilla
forms. 

>Would it be so bad if Mozilla renderd bugzilla better than other browsers?

No, but the success of Bugzilla demands that it works well on at least Mozilla,
IE and Opera. The fact that Bugzilla already now works somewhat better using
Gecko-based browsers is just a bonus. Anyway, I don't think it's a trivial task
to make BZ use CSS layout without a) a significant amount of work and b) adverse
effects on rendering in other browsers. If we lose (some measure of) usability
on IE, we lose a lot of reputation.

I look forward to be proven wrong, though. ;-)
There are improvements which can be done without dropping tables for complex
form layouts.

Things like removing the bgcolor attribute, all <font> tags (as the one in
"<font size="+1"><b>Bugzilla Bug 104382</b></font>").

There should be a better use of structural markup  (like <h2> <h3> <h4>), e.g.
in the bug comment headers.

Anyway, the HTML seems to be pretty clean already. =)
Attachment #48877 - Attachment mime type: text/plain → text/css
The User Interface component now belongs to Gerv.  Reassigning all UNCONFIRMED
and NEW (but not ASSIGNED) bugs currently owned by Myk (the previous component
owner) to Gerv.
Assignee: myk → gerv
Reassigning back to Myk.  That stuff about Gerv taking over the User Interface
component turned out to be short-lived.  Please pardon our confusion, and I'm
very sorry about the spam.
Assignee: gerv → myk
In 2.16.3 error messages do not have a HTML class. This makes it visually hard to
see that an operation has failed. For example the text line
"Sorry, you must use an existing Bugzilla account as initial owner"
just comes out directly after an </table> with no <span> or similar around.
I'd suggest to add at least two more style classes: success and error
Errors could be italics or red or both, maybe bold, too. Success messages could be
green maybe.
This is a patch with CSS support for bugzilla 2.17.6..
Since it was done for my needs it has certain requirements on other patches
submitted to bugzilla.. The list is:

Bug 91037: 217H_customfields_v2.patch
Bug 220794: 217H_prodgroup_v2.patch
Bug 220995: 217H_browse_v2.patch
Bug 220998: 217H_depinentrytemplates.patch
Bug 221017: 217H_types.patch
Bug 102852: 217H_picker.patch
Bug 91037: 217H_customfieldspicker.patch
Bug 221481: 217H_quicksearch_v2.patch
Bug 223066: 217H_notifyrules.patch

However since the above patches merely add features it should be possible to
appy the patch and then make the corrections on a standard bugzilla version..

This patch allows to create skins configurable from the preferences interface.
Skins are in the skins/ directory and consist of images, a style sheet and a
client javascript.. skin can be declared in the parameters and are use
configurable in the preference..

A css and javascript is provided with buttons in the header and footer for
actions and drop down menus for administration and saved queries..

It should be CSS standard should display without too many quirks in Internet
Explorer 5.0+.. It could be looking the same in IE6 than in mozilla if there
wasn't Bug 225938

A demo should be shortly available at http://bzdev.xpertnet.biz/bzdemo/
This patch creates two new stylesheets (css/layout.css css/content.css) which
are imported by header.html.tmpl.  The import is done before any added styles,
so that user specified stylesheets can override our default settings.

The banner page is also modified to use a div tag for the header and version
information.

This patch is intentionally small to cause as little trouble as possible.
Comment on attachment 136829 [details] [diff] [review]
Small patch that creates and imports two new stylesheets.

> Dave's RedHat patches used this, and I thought it was a good idea, 
> because IIRC, older browsers ignore @import, so they would skip them.

Older browsers also ignore code inside comment tags <wink>

I'd suggest:

    - Using <link> elements, which to me are generally better than @import
statements (take a look at devedge.netscape.com). We can eventually use @import
to avoid markup that breaks NS4, but I don't think this is the case with the
content in this patch. Is it?

    - Not changing the default font to a sans-serif one, at least not for your
first Bugzilla patch :-)

Thanks for the patch, I'll be happy to look at an updated version.
Attachment #136829 - Flags: review-
Oh, the margins should probably be kept if you want this to be an expedited
checkin :-)
Doesnt the use of @import give the ability to cache the CSS in the browser or
does <link> tags give you this too? 
Commenting on Ludo's path:

> This patch allows to create skins configurable from the preferences interface.

I'd be against the idea of skins for Bugzilla, on the basis of lack of utility
and high requirement for maintenance. I'm all for making it easy for admins to
tweak look and feel via CSS, but some sort of user-selected skin mechanism would
be a pointless maintenance nightmare IMO. :-)  

> A css and javascript is provided with buttons in the header and footer for
> actions and drop down menus for administration and saved queries..

As it happens, we decided against a dropdown for saved queries when we
simplified the saved query interface. The same probably applies to the admin
links. Anyway, we already have a dropdown for both - it's called the Links
Toolbar ;-)

> It should be CSS standard should display without too many quirks in Internet
> Explorer 5.0+..

Any CSS we use has to work perfectly in IE 5.0+ - after all, about 20% of the
web still uses IE 5.0.

Gerv
Posted patch Small patch for CSS, Take 2 (obsolete) — Splinter Review
Same patch, but with <link> instead of @import and no margin or font-family
changes.
Attachment #136829 - Attachment is obsolete: true
> Doesnt the use of @import give the ability to cache the CSS in the browser or
> does <link> tags give you this too? 

Well, the CSS in question is static content, and I'm quite sure CSS files we
<link> to are cached just like any other static file.

Posted patch Header Cleanup (obsolete) — Splinter Review
This patch (which depends on the Take 2 CSS patch above) removes unecessary
tables from header.html.tmpl, and adds some new styles to css/content.css.

I attemped to reduce this to no tables, but the CSS required to make this
seamless would have caused less capable browsers (IE 5 Mac in my tests) to
stack the currently horizontal h1, h2, and h3 args passed to the page.

I assume that is not an acceptable UI change, so I'll stick with this for now.
Posted patch Header Cleanup Correction (obsolete) — Splinter Review
Same as above, but a small fix to the table's style.
Attachment #136837 - Attachment is obsolete: true
Adds new styles to css/content.css (from above patches) for the product listing
of choose-product.html.tmpl.  Switches product name from a th to a td
(nitpicky, aren't th's for column headers?).

Minor change, merely to leverage CSS for centralized changes.
Attachment #136835 - Flags: review?(kiko)
Comment on attachment 136835 [details] [diff] [review]
Small patch for CSS, Take 2

I have some comments and questions:

a) The heading used to be centered in lynx -- you removed the center elements
which means it is left-aligned. Is leaving the center elements in something
you're opposed to?

b) You may find you'd be better-off using 

#version {
    text-align: center;
    font-size: 0.9em;
    width: 100%;
}

h1 {
    font-size: 2.00em;
    padding: 6pt;
    margin: 0;
}

since it keeps things closer to what they were. However, I'm not sure we want
to specify that for *all* h1s, so you might have to do:

#banner h1 {
  padding: 6pt;
  margin: 0;
}

c) The header looks kinda bad in netscape, but I guess we can live with that.
It would be fun if there was a way to get NS to read the padding and width
information..

Let me know what you think and we'll finish off the review.
Comment on attachment 136839 [details] [diff] [review]
Header Cleanup Correction

Question: would it not be wiser to use structural layout here (<h2> and perhaps
an <h3>) instead of boldifying and enlarging the contents?
Attachment #136839 - Flags: review?(kiko)
Comment on attachment 136840 [details] [diff] [review]
Add styles to choose-product.html.tmpl

It could be that these styles lived better in something like admin.css, since
we have separate css files for site-specific areas.

I'm also curious as to why you've separated content.css from layout.css -- I
can't seem to identify a pattern there, but maybe my glasses need mending
<wink>
At least on our site layout.css is for page wide settings such as tabs, headers, footer, etc. The 
content.css is for everything that goes inside of a dynamic content area in the middle of the page. 
Any settings in content.css such as H1, P, etc. should override whatever is in layout.css and fall 
back if not declared.

ex: https://bugzilla.redhat.com/bugzilla
Posted patch Add new stylesheets. (obsolete) — Splinter Review
This basically the same as the earlier Take 2, but adds Christian's suggestions
for the banner and Dave's comments on which styles go where.  This patch does
_not_ change any of the templates, it merely adds the two stylesheets.

Styles used atop every page are in layout, whereas content styles are in
content.css.  This allows users to customize the header & footer information
without meddling in the styles used in the content.

It also adds some styles used in forthcoming patches which rework the other
patches I obsoleted.
Attachment #136835 - Attachment is obsolete: true
Attachment #136839 - Attachment is obsolete: true
Attachment #136840 - Attachment is obsolete: true
Uses the latest stylesheet to modify the banner.html.tmpl.
Those two patches hopefully address all your concerns.  I separated them a bit
to make it less painful.

I have two more pending, which are reworks of my Header Cleanup and
choose-product patches.  I'll hold them until these others are (hopefully)
accepted. :)
Er... not wanting to complain, but I'd expect a stylesheet called "layout.css"
to contain only CSS positioning commands. If it's a global stylesheet, call it
"global.css" or "site-wide.css" or even "bugzilla.css".

Gerv
WRT comment #56:

fair enough. agreed
Needed to get the CSS into other pages.  Also cleans up the markup.
I attached that last one because it actually <link>'s the CSS. ;)

I'm fine with the name change.  I'll leave it to the applier to choose a name,
but I'd be happy to produce a final patch with an agreed on name that combines
the three patches I've attached.

I'll wait for an agreement, rather than continuing to attach patches.
I'd be happy to review a single patch that did the changes. However, you should
address Gerv's comment 56, and I have questions pending in comment  49 and
comment 50.

Regarding comment 56, I think having a global.css file is the correct approach,
and given Gerv and dkl's agreement, I'd say it's a consensus. 
Posted patch Unified (obsolete) — Splinter Review
New patch: new stylesheets (content and global), addition of CSS <links> to
header, remove unneeded tables from header and banner.

comment 49(a): I'm not vehemently opposed to the <center>, but I'd prefer to
leave it out (for purity reasons, since the user would have to change it in two
places to get the proper result if they wanted it, say, right-aligned).

comment 50: Switched banner to use an <h1> and <h2> instead of <div>'s.  Makes
more semantic sense.  <h3> is much bigger than 0.9em, so we would have to
resize it anyway.  I made the version's weight normal to keep the current look.


comment 56: Changed to global (per the consensus).
Attachment #136870 - Attachment is obsolete: true
Attachment #136871 - Attachment is obsolete: true
Attachment #136873 - Attachment is obsolete: true
Err, actually, comment 50 was in regard to attachment 136839 [details] [diff] [review] (you seem to have
missed that part :-) which is where you changed the structure of the page
header. As I see it, the page header is what we should be marking up as <h1> or
at most <h2> since it indicates the intent of that specific page. I don't think
the version is important enough to warrant it to be a heading.

I'm not sure if the "This is Bugzilla" text is <h1>-material, but it may be just
for the fact that it's the only place which identifies the whole tool/site. If
you agree, then we should <h2> the page header, and perhaps <h3> one of the
secondary headers.
Attachment #136835 - Flags: review?(kiko)
Attachment #136839 - Flags: review?(kiko)
Comment on attachment 136870 [details] [diff] [review]
Add new stylesheets.

Umh... The amount of patches and sudden discussion on all fronts is baffling. I
still (see comment 26) strongly think this should only be a meta bug for CSS
issues, and we should clearly identify the issues and progressively work
through this stuff. 

My day job is managing a huge site with several megabytes of HTML, and I've had
experience of getting swamped with CSS rules with at least half a dozen other
sites as well. Our chance of getting this thing done properly just in this one
bug and a handful of patches is... Well, not optimal. Issues like proper class
naming and so on are tough, and once we release with some class names we're
going to have hard time changing them. So please guys, let's plan ahead very
carefully, or we'll be doing a disservice to Bugzilla customizability.

I see we have several efforts going on here. I still think we should split
these issues into their own bugs.

One major effort seems to be organizing the css directory into a coherent set
of rules. That's good, but only if the names are reasonably understandable.
"content.css" is interesting; the explanations given make sense, but "content"
and "css" are theoretically speaking mutually exclusive, so the selection of
terms is... Hard. Also, it's bound to create a very heavy style sheet - there
can be a huge set of rules, and having something called as generic as "content"
may well prove hard to maintain and customize.

Another run of patches concerns de-tabling the header. I'm not certain of the
merits, but it's a good place to start, so I'm all for it. 

And thirdly, we have a huge amount of other pages/templates, of which
change-product seems to be represented. Well... A single bug won't go very far
here. 


A significant issue here is, that before we come up with a set of CSS rules, we
should also come up with a generic set of rules on how to write CSS. Simple
stuff is like indentation and capitalization, harder is stuff like picking
between ids and classes, use of em/%/px/pt, how to manage NS4 issues and so on.
If we do something this drastic to our templates, we should also look forward
for maintainability - how are the future patch writers going to follow the plan
laid out here?


And finally, some points on the CSS (these are examples of the issues involved
instead of real review comments):

>+td.productName {

What is this rule doing semantically? To me, it seems like it's formatting a
single appearance of product name. What product name? Do we want all product
names all around Bugzilla to have right/top alignment? Why do we set this rule
to apply to table cells only?

Are these rules aimed for a single page or Bugzilla in whole? 

>+#pageHead {
>+#pageHeader {

We clearly need to discuss id and class names pretty heavily :-)

>+#pageSubHeader {

... the use of Hx elements, as already brought up by others...

>+#pageRightAlignedSubHeader {

Id's shouldn't, of course, contain information about the style rules themselves
- "right aligned" sort of sucks if you want to left align it :-)

>+	font-size: 2.00em;
>+	padding: 6pt;

Mixing of absolute and relative units, and their effects on browsers...


And so on. Sorry, I don't mean to sound evil or pessimistic, it's just that
this is a major undertaking. It takes more than code and review to successfully
do this sort of things. Like somebody (patch author?) said on developers@,
we've only had chit-chat so far.
Attachment #136870 - Attachment is obsolete: false
Comment on attachment 136870 [details] [diff] [review]
Add new stylesheets.

Didn't mean to change the obsolete flag, but got midaired.
Attachment #136870 - Attachment is obsolete: true
"This is Bugzilla" is designed to be changed or removed - it's a temporary title
only. Given that, it makes semantic sense to not make "This is Bugzilla" a
<h-anything>, but style it using font-size etc., and have the first heading
which is part of the page proper as a <H1>.

Gerv
I disagree on the 'This is BZ' header.  If I am customizing BZ's look, I'll
first put my company logo (or whatever) into the <h1>, then a description into
<h2>.  I end up with:

XYZ Corporation
Bugzilla Bug Tracker 2.17.6

So I think that <h1> and <h2> are appropriate.  While it's a temporary string,
whatever replaces it is _the_ page header.

When you say 'page header' are you referring to the piece that yields 'Bugzilla
Bug #' atop this page?  If so, we would be talking about placing a <table>
within an <h2>, which for some reason feels dirty to me.  This would also change
it's size and look.  I would like to put it into an <h3> and make each of the
headers in <div>s or <spans> (no line break), but that causes problems in older
browsers.

I agree with Jouni that we need to standardize alot of things, including all the
things he specifies.  (use of id or class, not having 'right' in a style name,
sorry!)

Shall I move more of my conceptual ideas to developers@ and hash them out?
This patch is the same as the others, but uses a table for the header's width,
an <h3> for the first element of the header, and simple id names for the 'sub'
and 'subsub' headers.

The poorly names classes pointed out by Jouni have been removed, as well as the
mixture of absolute and relative units.
Attachment #136877 - Attachment is obsolete: true
Urk, what a schizophrenic bug.  This bug appears to have been hijacked for
specific improvements to Bugzilla's CSS support, and it's no longer that useful
as a meta-bug, so instead of making y'all move this activity to a new bug, I'm
making this be the bug it has become.

Cory: please request review of your latest patch from one of the interested
reviewer-commenters in this bug.
Assignee: myk → gphat
No longer depends on: 28884, 36013, 57845, 92565, 98123, 104247, 148679
Keywords: meta
Summary: Bugzilla should have better support for CSS → use CSS to style banner, header, and message
i did a patch for bug 232397 that replaces the deprecated <strike> with <span>
and css.  (waiting for review)

perhaps that patch could be integrated into this one?
Comment on attachment 136943 [details] [diff] [review]
Add new stylesheets + clean up header & banner

>+    <link href="css/global.css" rel="stylesheet" type="text/css" />
>+    <link href="css/content.css" rel="stylesheet" type="text/css" />

Self-closing tags are illegal in HTML 4.01 Transitional.

See http://www.hixie.ch/advocacy/xhtml

with that change, and someone reviewing a new patch, this could forseeably make
2.18.
Attachment #136943 - Flags: review-
queueing for a later 2.18 decision after a chance is given for a patch refresh
Flags: blocking2.18?
(In reply to comment #70)
> (From update of attachment 136943 [details] [diff] [review])
> >+    <link href="css/global.css" rel="stylesheet" type="text/css" />
> >+    <link href="css/content.css" rel="stylesheet" type="text/css" />
> 
> Self-closing tags are illegal in HTML 4.01 Transitional.

Also, this patch doesn't contain a content.css file :-)
Pseudo-random heads-up: I've added a global.css file and a link to the header
template as part of the fix for bug 237757, so future patches can take that into
account.
I'm not going to hold 2.18 for this.  If someone gets a patch up before we
branch I'll probably take it, though.  If that happens, feel free to retarget it
back to 2.18.
Flags: blocking2.18? → blocking2.18-
Target Milestone: Bugzilla 2.18 → Bugzilla 2.20
The header and banner were converted in bug 240486.  Retargeting this bug to the
message and removing bz-css alias, which makes more sense as an alias for the
CSSification meta-bug 253449.
Alias: bz-css
Summary: use CSS to style banner, header, and message → use CSS to style message
This would be very good to have.  Several of my users are actually complaining 
about the look and feel of Bugzilla making it difficult to use.  I'm afraid 
that if someone finds something like Scarab ( http://scarab.tigris.org/ ) I 
will need to switch just for stupid UI issues.
Paul: Bugzilla is already pretty customisable using CSS and template changes.
See http://bugs.kde.org, http://bugzilla.redhat.com and http://bugs.gnome.org
for three examples.

Gerv
Yes, technically Bugzilla is 100% customizable because it's open source.
Yet I think they put a lot of time into customizing their Bugzillas.
I was trying to avoid making changes that I would have to start to maintain 
with newer releases of Bugzilla.  
Correct me if I'm wrong but I don't think the templates isolate me from this 
problem, but I think the CSS option does.

Let's say I like the KDE Bugzilla (and I do)  Is there an *easy* way to make my 
Bugzilla look like theirs?

Bugzilla 2.20 feature set is now frozen as of 15 Sept 2004.  Anything flagged
enhancement that hasn't already landed is being pushed out.  If this bug is
otherwise ready to land, we'll handle it on a case-by-case basis, please set the
blocking2.20 flag to '?' if you think it qualifies.
Target Milestone: Bugzilla 2.20 → Bugzilla 2.22
Flags: blocking2.20?
clarifying summary to make it more obvious what Myk meant in comment 75. 
Denying request to block the 2.20 release.  This will be REALLY cool to have, of
course, but it's not going to block the release.
Flags: blocking2.20? → blocking2.20-
Summary: use CSS to style message → use CSS to style informational message at bottom of banner
Whiteboard: see comment 75 and onward for the current direction of this bug
I think this bug should be marked invalid or duplicate of another validation
bug. It is useless now.
This has been covered by bug 278125.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → WORKSFORME
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Status: REOPENED → RESOLVED
Closed: 15 years ago15 years ago
Resolution: --- → FIXED
Target Milestone: Bugzilla 2.22 → Bugzilla 2.20
QA Contact: matty_is_a_geek → default-qa
You need to log in before you can comment on or make changes to this bug.