CSSLint / csslint

Automated linting of Cascading Stylesheets
http://csslint.net
Other
4.76k stars 483 forks source link

Duplicate H element rule might force duplication #80

Open trygve-lie opened 13 years ago

trygve-lie commented 13 years ago

The following style:

h1, h2 {font-face: Verdana;} h1 {font-size: 2.1em;} h2 {font-size: 1.8em;}

does prompt a warning due to the recommendation that one should only define H elements only once in a style. Tough; In this example one would have to replicate the font-face for all H tags to conform with the lint rule. That would increase the size of the css file.

Isn't it most common to set some styles which apply to all H elements and then override only the differences, like the font-size, for each H element?

nzakas commented 13 years ago

Interesting case. Perhaps we should track with properties are being set in order to determine if something is really a duplicate or not. Nicole?

stubbornella commented 13 years ago

Hmm, this makes a good point. I think the rule may need to be more clever...

e.g. check that each text and font property is declared only once per heading.

would that work?

nzakas commented 13 years ago

Should it be limited to just text and font properties? Or would it make sense to track them all?

trygve-lie commented 13 years ago

I think it would be good if one could check for duplicate properties all over. Not just text and font properties.

Tough I do might see another challenge (not sure if it's worth a new issue in the tracker): As we know headings are intended to describe the section it introduces. In HTML4 (and earlier) we normally use the H1 element to describe the whole page due to the fact that we have no good way to divide our pages into more fine grained sections. I assume this CSS lint rule to be bound to this and for me it makes much sense. Please correct me if I'm wrong.

But in HTML5 this changes: "The first element of heading content in an element of sectioning content represents the heading for that section." Ref: http://dev.w3.org/html5/spec/Overview.html#headings-and-sections

In other words; in HTML5 it will be perfectly OK to have multiple SECTION elements on a page where each start with a new set of headings. In such a case; wouldn't it be desirable to assign different styles for heading in each section?

nzakas commented 13 years ago

Even though HTML5 allows you to nest heading elements, I think it's still good advice to try to keep your headings unique and use H1 at the highest level, H2 underneath, and so on. In any event, if you disagree with the rule, you'll be able to turn it off. :)

stubbornella commented 13 years ago

There are also good reasons for duplicating properties. For example, all the headings are the same weight except one:

h1, h2, h3, h4, h5, h6 {font-face: Verdana;font-weight:bold;} h1 {font-size: 2.1em;} h2 {font-size: 1.8em;} h3 {font-size: 1.5em; font-weight:normal;} h4 {font-size: 1.2em;} h5{font-size:1.1em;}

I think we need to keep in mind that what we are trying to do is point out places where a human should check if there might be an error. It isn't to say that the linter won't sometimes catch things that look like an error but are actually a choice.

If we change it to check by property value pair we might miss a lot of duplicate heading declarations. In fact, they can write as many as they like as long as they use different properties each time.

The current rule gives warnings when sometimes it shouldn't, but I think simply increasing the number of allowed heading declarations to 2 would solve the problem better without the risk of missing dups.

Regarding HTML5, styling those headings will still be completely impractical unless you define a set of reusable heading classes. You don't want css that looks like this:

section h1{color:red;} section section h1{color: blue;} section section section h1{color: gray;} section section section section h1{color: green;}

hojberg commented 13 years ago

What also applies is if you had a css reset at the top of your css file, and then later setting h rules

stubbornella commented 13 years ago

@hojberg - if we use normalize.css instead of reset.css, we won't have that problem. The headings in normalize are meant to be adapted for every project.

mahonnaise commented 13 years ago

Nesting-level dependent headings do work really great for some kinds of documents though. It's very nice for technical documentation, ebooks, and thinks like that. Structured text documents, basically. In that case you do want CSS which looks like that.

But for regular websites it's indeed somewhat awkward.

nzakas commented 13 years ago

Regarding checking all properties, consider this code:

h1 { font-family: Arial } h1 { margin: 10px; } h1 { color: red; }

I would say the error here is not multiple heading definitions, but rules that can (should?) be combined.

mahonnaise commented 13 years ago

Let's see...

This should allow you to use a reset (or not), a baseline setup (or not), and individual declarations.

Wait a minute.

There is an easier way to enforce Nicole's intention. It's only necessary to check if some heading element is used as key selector in a more complex combined selector. ("Combined" because over-qualification is checked by a separate rule.)

Basically, the idea is to turn this "duplicate H" check into a "location dependent H" check.

[...] rules that can (should?) be combined.

This should be handled separately. Fine-grained one-trick ponies offer more control.

stubbornella commented 13 years ago

@mahonnaise - so you mean check for location dependent selectors like this?

.foo h1{} .bar h1{}

mahonnaise commented 13 years ago

Yes, it A) uses a H tag as key selector and B) it's a combined selector (descendant combinator). As such, it's deemed nag-worthy.

trygve-lie commented 13 years ago

Some of my thoughts on headings in HTML5 first:

I think it's still good advice to try to keep your headings unique and use H1 at the highest level, H2 underneath, and so on.

Markup wise I do not quite agree on that. There are a lot of cases where one would like to start a new H1 inside a section. I think the frontpage of http://www.bbc.co.uk/ are a good example of where H1 does not have a natural place as the highest level of the whole page. In BBC's case most people would place the H1 around the logo at the top of the page. I find that wrong.

Its tough interesting to see what BBC actually does now; they have a top level text saying BBC Homepage in a H1 with a css class set to hidden.

You don't want css that looks like this: section h1{color:red;} section section h1{color: blue;} section section section h1{color: gray;}

I do agree on that. But I might want to go like this:

.fooBox h1{color: red;} .barBox h1{color: blue;} .candyBox h1{color: grey;}

There are also good reasons for duplicating properties. For example, all the headings are the same weight except one:

That's a good point. I agree on that one.

In any event, if you disagree with the rule, you'll be able to turn it off.

I do agree its very nice to be able to turn rules of. What I like about such lint tools are that they can point out things which will cause errors and also give personal advices / flavors of the author(s). In the case of the linter warning about errors there are little doubt about the reason for the warning. When it comes to advices from the author(s) I find it good to be able to read up on the author(s) advice and upon that decide if I should turn the warning off or not. I think good documentation and reasoning for each warning in the lint tool are a key factor for the tool.

oli commented 13 years ago

Re: HTML5, the spec says:

“Sections may contain headings of any rank, but authors are strongly encouraged to either use only h1 elements, or to use elements of the appropriate rank for the section's nesting level.” — http://www.whatwg.org/specs/web-apps/current-work/complete/sections.html#headings-and-sections

but ‘h1 everywhere’ isn’t practical atm due to lack of AT support (their update cycle is s-l-o-w too), and lack of decent CSS selectors. There’s talk of :heading(), but it’s not specced yet, let alone implemented. So for now it’s best to stick with old-school style headers. If you use HTML5 sectioning elements and hgroup you can just define styles so highest to lowest rank is also most to least visually important. Your styles should then match using headings logically, with subheads covered by hgroup.

…fwiw :)

@nzakas rather than a simple case of combination

h1 { font-family: Arial }
h1 { margin: 10px; }
h1 { color: red; }

it’s more like

h1, h2, h3, h4, h5, h6 {font-family: Georgia, "Times New Roman", Times, serif; font-weight: normal;}
h1 {margin: 0 0 .75em; padding: .75em 0 0; font-size: 2em; line-height: 1.5;}
h2 {margin: 1.3333em 0 0.6667em; font-size: 1.5em; line-height: 1;}
…
h5, h6 {margin: 1.5em 0 0; font-size: 1em; line-height: 1.5;}
h5 {font-weight: bold;}

Some properties defined for all headings, some for only a few headings, and the occasional override.

shapeshifta78 commented 13 years ago

Hey Nicole,

if I should not qualify headings then what about these bits from oocss? table h1,table h2,table h3, table h4, table h5, table h6, table p, table ul, table ol, table dl { padding:0 }

Along with some other tiny things that don't validate against csslint.

No accusation, just asking :)

nzakas commented 13 years ago

Part of the missing documentation for CSS Lint is that most rules don't apply to your foundational CSS bits (reset stylesheets, grids stylesheets, etc.). The oocss library is also foundational.

shapeshifta78 commented 13 years ago

I see why you would regard it as foundational, but oocss contains the same selections oli mentioned in his comment beforehand (https://github.com/stubbornella/csslint/issues/80#issuecomment-1594331)

As well as the selections I already mentioned.

nzakas commented 13 years ago

I'm not sure what you're trying to say. I'm saying that oocss is a foundational library so many of the CSS Lint rules actually wouldn't apply. Are you trying to say it's not a foundational library? Or something else?

shapeshifta78 commented 13 years ago

The code mentioned by oli is similar to the headline declarations in oocss:

h1, h2, h3, h4, h5, h6, ul, ol,dl, p,blockquote {padding:10px;}
h1, h2, h3, h4, h5, h6,img{padding-bottom:0px;}
pre{margin: 10px;}
table h1,table h2,table h3, table h4, table h5, table h6, table p, table ul, table ol, table dl{padding:0;}

and

h1, .h1{font-size:196%;  font-weight:normal; font-style: normal; color:#AE0345;}
h2, .h2{font-size:167%; font-weight:normal; font-style: normal; color:#AE0345;}
h3, .h3{font-size:146.5%; font-weight:normal; font-style: normal; color:#DF2B72;}
h4, .h4{font-size:123.1%; font-weight:normal; font-style: normal; color: #333;}
h5, .h5{font-size:108%; font-weight:bold; font-style: normal; color:#AE0345;}
h6, .h6{font-size:108%; font-weight:normal;  font-style: italic; color:#333;}

and you say I should just skip it? I don't think that this is a reasonable solution.

nzakas commented 13 years ago

I'm sorry, I'm having a hard time understanding the point you're trying to make. Your original question asked why there were CSS Lint warnings in oocss, and I've answered that. If you like me to answer another question, please ask it directly.

shapeshifta78 commented 13 years ago

I thought I did that :D

You say: OOCSS is foundational and does not have to validate. I say: It should validate.

Why? It qualifies headings like table h1 as well as grouping headings for a base layout. I think CSSlint should produce no warnings when using bits from OOCSS. I am not allowed to do this in my stylesheets so why should OOCSS be allowed to do that?

So, once again: What should I do with those warnings which come from code used in OOCSS and borrowed into my own project? I think, it should be possible to reduce the errors and warnings in CSSLint to zero like I can use JSLint to reduce all errors mentioned by JSLint and if I don't agree with one of the rules, I am free to ignore it.

Sidenote: normalize.css - 3 easy to fix warnings, errors reset.css (Eric Meyer - reloaded) - no warnings, no errors

nzakas commented 13 years ago

Okay, you're arguing that the oocss library should pass CSS Lint 100%. As I said, foundational libraries have different characteristics than non-foundational, so not all CSS Lint rules will apply and therefore this won't happen. I'm sure other foundational libraries such as YUI CSS foundation also would see quite a few warnings. That's expected because foundational libraries abstract away ugliness so you don't have to worry about it.

In any event, this isn't the right forum for debating whether oocss should or should not pass CSS Lint. I'd suggest either using the CSS Lint mailing list or the oocss mailing list for that purpose.

shapeshifta78 commented 13 years ago

ok, maybe you should add a link to google groups in the docs then or am I too blind to see it? And maybe a small note, that csslint is not a validator and it is ok to have some errors.

mahonnaise commented 13 years ago

csslint is not a validator and it is ok to have some errors

Not every rule is compatible with every approach.

You should use and/or create rules which enforce the conventions you are using.

Decide for yourself where you want to draw the line when it comes to accuracy.

bronson commented 8 years ago

I don't quite understand the conclusion... It sounds like people are agreeing that the current rule is somewhat specialized, but it should be left on by default? That seems like a recipe for false positives.

My case: I'm styling Markdown-generated HTML so headings can't nest. Here's my markup:

h1,h2,h3,h4,h5,h6 {
  font-family: 'Lucida Grande', Helvetica, Verdana, sans-serif;
  font-weight: bold;
}
h1,h2 { color: rgb(52,101,164); }
h3,h4,h5,h6 { color: rgb(114,159,207); }
h1 { margin-top: 1.3em; font-size: 1.5em; }
h2 { font-size: 1.3em; }

Is the linter suggesting that there's a better way to write this?

Nateowami commented 7 years ago

Pardon me if I'm wrong, but it appears to me that this rule—and most of the discussion of it—even its documentation—misses the entire point of the rule:

The heading elements (h1-h6) are considered to be built-in objects that should look the same regardless of where they appear.

The documentation gives this example of "bad code":

/* Two rules for h3 */
h3 {
    font-weight: normal;
}

.box h3 {
    font-weight: bold;
}

But if the idea is to make heading elements appear the same anywhere in the page, then it takes only one rule to break this:

.box h3 {
    /* There. Now h3 inside of a .box and outside will differ. With only one rule 
    declared. The h3 is no longer atomic, but no warnings are shown. */
    font-weight: bold;
}

Even this code follows the spirit of the rule:

h1, h2 {
    color: black;
    font-weight: bold;
}
h1 {
  font-size: 48pt;
}
/* There. Now h1 and h2 will look the same no matter where on the page. 
   The h1 and h2 headings are still atomic, but the linter complains about multiple 
   rules. */

It makes no difference whatsoever whether there is one rule or a thousand rules. What matters is whether the headings are qualified, making them context-dependent, and therefore non-atomic.

In fact, I cannot think of a single instance where this rule would catch non-atomic headings that wouldn't be caught by a "don't qualify headers rule" (Of course things like h3:hover need to be allowed, but that shouldn't be a problem).

Am I missing something? Is there a legitimate reason to count the number of rules instead of whether rules are qualified?