We continue to improve bugzilla html/css code (see bug 240486, bug 245924, bug 248379, bug 251068). This is corrected header code. These changes are tested with: Mozilla 0.9.4 (Windows NT 5.2) Mozilla 1.7.0 (Windows NT 5.2) Firefox 0.9.3 (Windows NT 5.2) Opera 6.06 (Windows NT 5.2) Opera 7.51 (Windows NT 5.2) MSIE 5.0 (Windows NT 5.2) MSIE 5.5 (Windows NT 5.2) MSIE 6.0 (Windows NT 5.2) Netscape 4.78 (Windows NT 5.2) Links 0.98 (Windows NT 5.2) Lynx 2.8.5rel.1 (Windows NT 5.2)

Comment on attachment 171058 [details] [diff] [review] Corrected header CSS code. duplicate for patch 171059

Comment on attachment 171057 [details] [diff] [review] Corrected header HTML code. >+ [% IF h1 %] >+ <h1><div>[% h1 %]</div></h1> >+ [% END %] How come there's a div inside the h1? The h1 is itself a block level element that can be styled the same as the div can, so enclosing the content in this additional block doesn't seem to add anything but complexity.

Oh, sorry, it's my fault. According to HTML 4.01 DTD <Hn> elements can not contain block-level elements. So I replaced <div> to <span> in previous patch. <span> elements may be used for applying addition styling.

Why do we need al this bloat in our markup? CSS Zen Garden is fun, but Bugzilla should not try to emulate such markup.

(In reply to comment #7) > Why do we need al this bloat in our markup? CSS Zen Garden is fun, but > Bugzilla should not try to emulate such markup. The goal of Bugzilla CSSification (bug 253449) is to separate style from structure and make Bugzilla style customizable and skinnable. In general, our markup should be as clean and simple as possible, and these patches have been achieving that, replacing archaic markup with modern equivalents and removing most non-essential markup but including some optional additional markup that adds useful customizability/skinnability.

> Oh, sorry, it's my fault. According to HTML 4.01 DTD <Hn> elements can not > contain block-level elements. So I replaced <div> to <span> in previous patch. It wasn't that I was concerned about so much, it's that the span seems unnecessary. After all, any style that can be applied to that span could be applied to the enclosing header tag instead, and the result would be exactly the same, so the span isn't necessary for styling of the content within the headers.

Anne brought up an additional point on IRC. Regarding the intro and outro elements, a simpler approach, and the more standard one, is to have stylers use the :before and :after pseudo-elements to provide additional style before and after the message. This approach obviates the need for additional markup without sacrificing functionality (f.e. it's still possible to add styled content before and after the message). We should adopt it.

Comment on attachment 172904 [details] [diff] [review] div -> span This looks good except for the issues raised in comment 9 and comment 10. Fix those, and I'll review and approve!

(In reply to comment #9) > > Oh, sorry, it's my fault. According to HTML 4.01 DTD <Hn> elements can not > > contain block-level elements. So I replaced <div> to <span> in previous patch. > > It wasn't that I was concerned about so much, it's that the span seems > unnecessary. After all, any style that can be applied to that span could be > applied to the enclosing header tag instead, and the result would be exactly the > same, so the span isn't necessary for styling of the content within the headers. You may apply more styles to two elements whan one, e.g. you may underline header with two different borders. We may show examples of such styling.

(In reply to comment #10) > Anne brought up an additional point on IRC. Regarding the intro and outro > elements, a simpler approach, and the more standard one, is to have stylers use > the :before and :after pseudo-elements to provide additional style before and > after the message. This approach obviates the need for additional markup > without sacrificing functionality (f.e. it's still possible to add styled > content before and after the message). We should adopt it. This is not implemented in MSIE and partially implemented in Gecko-based browsers. See bug 280459.

(In reply to comment #12) > You may apply more styles to two elements whan one, e.g. you may underline > header with two different borders. We may show examples of such styling. And you might want five. That is no reason to Add 4 nested SPAN elements inside a DIV element by default, just to make sure everyone can at least apply styles 5 layers deep. I really do not like the patches provided so for. (Both here and in related bugs.) Bugzilla should use semantic markup, not nested DIV tag-soup.

(In reply to comment #14) > (In reply to comment #12) > > You may apply more styles to two elements whan one, e.g. you may underline > > header with two different borders. We may show examples of such styling. > > And you might want five. That is no reason to Add 4 nested SPAN elements > inside a DIV element by default, just to make sure everyone can at least > apply styles 5 layers deep. > > I really do not like the patches provided so for. (Both here and in related > bugs.) Bugzilla should use semantic markup, not nested DIV tag-soup. As you wish. We may remove additional markup and provide new patch-set with minimal clear markup.

We removed unnecessary tags. Shall we provide new patch for removing unnecessary tags in our patches (bug 240486, bug 245924, bug 248379 and bug 251068) that was before?

Comment on attachment 174399 [details] [diff] [review] Markup is minimized and cleared up. >> ... a simpler approach, and the more standard one, is to have stylers use >> the :before and :after pseudo-elements to provide additional style before >> and after the message. > >This is not implemented in MSIE and partially implemented in Gecko-based >browsers. See bug 280459. That's certainly a problem of that approach, but in this case I think we're better off pushing the standard than working around it. And demonstrating this capability in Bugzilla is a great way to get the Mozilla developers to fix those lingering Gecko bugs, since they use Bugzilla day in and day out. >Shall we provide new patch for removing unnecessary tags in our patches >(bug 240486, bug 245924, bug 248379 and bug 251068) that was before? Yes, that would be a good idea, since it makes those fixes consistent with this one. >+<div id="message">[% message %]</div> Nit: indent the div two spaces as the table was indented before. I had trouble applying this patch, but I was able to apply it by hand, and it looks and works great along with its corresponding CSS fixes. r=myk

Vitaly and Svetlana, do you have CVS write access these days, or do you need me to check this in for you?

(In reply to comment #18) > Vitaly and Svetlana, do you have CVS write access these days, or do you > need me to check this in for you? No, we have no check-in rights (can we get it?). Please, commit patch by yourself.

> No, we have no check-in rights (can we get it?). Please, commit patch by > yourself. Ok, checked in: Checking in skins/standard/global.css; /cvsroot/mozilla/webtools/bugzilla/skins/standard/global.css,v <-- global.css new revision: 1.9; previous revision: 1.8 done Checking in template/en/default/global/header.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/global/header.html.tmpl,v <-- header.html.tmpl new revision: 1.36; previous revision: 1.35 done You can certainly get CVS write access. Just follow the instructions on the following page: http://www.mozilla.org/hacking/getting-cvs-write-access.html I'll vouch for you, and you don't need super-reviewer approval since you're only checking into Bugzilla code.