4.9 Check Book CSS warning

[Phssthpok]

Since updating to 4.9, I get a warning ("You have 2 h1s defined") for CSS like this:

h1 { ... }
h1.chapter { ... }

Why should this be? What is wrong with doing this (defining rules for H1 in general, and additional rules for H1s with class="chapter")?

Edit: Also, it turns out, for rules "h1.a" and "h1.b".

[phossler]

Same here using 4.9.1 - didn't used to do that

I know it's only a warning, but I think it's a wrong warning since I think it's allowed by the CSS gods


From the 'detailed description' link

Headings should only be defined once

Object-Oriented CSS (OOCSS) works by defining reusable objects that can be put into place anywhere on a site and appear exactly the same. The heading elements (h1-h6) are considered to be built-in objects that should look the same regardless of where they appear. As such, each heading should have exactly one rule defining its appearance. Multiple rules defining the same heading appearance can lead to objects that are hard to use because the context defines the appearance rather than being completely atomic.

[JSWolf]

Yes it's allowed to have CSS like the example. But really, it's poor code.

[hobnail]

That "only defined once" rule seems bad to me. What I have in my standard stylesheet is like the following. All the common shared stuff in one rule and then the size stuff in separate rules. Following their rule would make maintenance a hassle since you'd have to duplicate stuff using copy and paste. As a programmer I learned that copy and paste was a bad "code smell".

h1, h2, h3, h4 {
    break-inside: avoid;
    break-after: avoid;
    font-weight: bold;
    hyphens: none;
    margin-bottom: 0em;
    margin-top: 1em;
    page-break-after: avoid;
    page-break-inside: avoid;
    text-align: center;
    text-indent: 0em;
}

h1 {
    font-size: 1.44em;
}

h2 {
    font-size: 1.3em;
}

h3 {
    font-size: 1.12em;
}

h4 {
    font-size: 1em;
}

[phossler]

@JSWolf -- Why is it 'poor code'?

@hobnail -- I agree. Easier to maintain, clearer, and less redundancy

[JSWolf]

@hobnail, when you have classes together like in your CSS, Calibre will not remove any of the unused classes.

h1 {
  font-size: 1.44em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h2 {
  font-size: 1.3em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h3 {
  font-size: 1.12em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h4 {
  font-size: 1em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}

That is much nicer code.

[Tex2002ans]

Originally Posted by phossler

Same here using 4.9.1 - didn't used to do that

Agreed. Perhaps it was some recent addition.

In the Calibre 4.9.1 release notes it mentions:

Edit book: The Check book function has a new, improved CSS checker with support for CSS 3

Perhaps this was an accidental regression that was introduced.

Originally Posted by phossler

I know it's only a warning, but I think it's a wrong warning since I think it's allowed by the CSS gods

From the 'detailed description' link

Here's the exact URL the "detailed description" links to:

https://github.com/CSSLint/csslint/w...e-defined-once

... very odd. I don't agree with that at all.

I can understand Check Book pointing out an (accidental) duplicate in your CSS:

h1, h2, h3, h4 { }
h2 { }
h2 { }

but pointing out ones that aren't the same?

h2
h2.title
h2.author

Originally Posted by JSWolf

That is much nicer code.

No. I'll have to agree with phossler and hobnail on this one.

Sometimes it's easier to have a centralized location for shared CSS, and then specific overrides in a different location.

Side Note: And I would strongly recommend against adding hyphens: none though. Think big words ("Acknowledgements"), with big font, on tiny devices (cell phone). Just leave that setting up to user devices/preferences.

[davidfor]

Originally Posted by JSWolf

@hobnail, when you have classes together like in your CSS, Calibre will not remove any of the unused classes.

h1 {
  font-size: 1.44em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h2 {
  font-size: 1.3em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h3 {
  font-size: 1.12em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}
h4 {
  font-size: 1em;
  font-weight: bold;
  hyphens: none;
  margin-bottom: 0;
  margin-top: 1em;
  page-break-after: avoid;
  page-break-inside: avoid;
  text-align: center;
  text-indent: 0;
}

That is much nicer code.

I so disagree with that. What you have means there means that if you want to change the overall style of the headers, say from centred to on the right, you have to do it in four places instead of one. Ugly, inefficient, a pain to maintain and prone to error.

[hobnail]

Originally Posted by JSWolf

@hobnail, when you have classes together like in your CSS, Calibre will not remove any of the unused classes.

I can live with a few unused classes in the CSS when I have cleaner and easier to maintain CSS.

[kovidgoyal]

You will have to argue with the developers of csslint as to why they make that recommendation. But I agree that check book in general should avoid stylistic checks, and I have already turned most of them off, must have missed that one.

[Phssthpok]

Originally Posted by JSWolf

Yes it's allowed to have CSS like the example. But really, it's poor code.

On the other hand, it also warns if you don't have a rule for h1, but just two rules for h1.a and h1.b. Changing them to .a and .b makes the warning go away. It seems wrong to be forced to reduce specificity like that.

[Phssthpok]

Originally Posted by kovidgoyal

You will have to argue with the developers of csslint as to why they make that recommendation. But I agree that check book in general should avoid stylistic checks, and I have already turned most of them off, must have missed that one.

So... is there a way to turn it off? Please?

[theducks]

I am not happy with the stylistic warnings...

BUT I am really unhappy that they (and others) pop a RED Error shield, when that, IMHO, should just be the older Yellow warning (fix me if you care )

[kovidgoyal]

Originally Posted by Phssthpok

So... is there a way to turn it off? Please?

Umm wait for the next release when I will disable it along with the rest of the stylistic checks that I have already disabled.

[kovidgoyal]

Originally Posted by theducks

I am not happy with the stylistic warnings...

BUT I am really unhappy that they (and others) pop a RED Error shield, when that, IMHO, should just be the older Yellow warning (fix me if you care )

Which stylistic warnings (apart from this one that I missed) give you errors rather than warnings?