Bug 169610

Summary: [CSS 2.1 Conformance] font shorthand and inherit incorrectly parsed
Product: konqueror Reporter: Gérard Talbot (no longer involved) <browserbugs2>
Component: khtml rendererAssignee: Konqueror Developers <konq-bugs>
Status: VERIFIED FIXED    
Severity: normal CC: maksim
Priority: NOR Keywords: reproducible, testcase
Version: 4.7.3   
Target Milestone: ---   
Platform: Ubuntu Packages   
OS: All   
Latest Commit: Version Fixed In: 4.13.2

Description Gérard Talbot (no longer involved) 2008-08-22 20:53:44 UTC
Version:            (using KDE 4.1.0)
Compiler:          cc 
OS:                Linux
Installed from:    Debian stable Packages


Bug entry:
http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/#bug159

  h1 {font-size: 32px;}

  h1 {font: 96px inherit;}
  </style>

  <h1>This text should be using a  font-size of 32px and not a font-size of 96px.</h1>

CSS validator reports that the 2nd rule has a Parse error, should be rejected; therefore only 
h1 {font-size: 32px;}
should be accepted and rendered.

Testcase:
http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/font-shorthand-inherit.html

Regards, Gérard
Comment 1 Gérard Talbot (no longer involved) 2008-08-23 03:26:10 UTC
Also reported :
https://bugs.webkit.org/show_bug.cgi?id=20181
Comment 2 Maksim Orlovich 2008-08-23 17:25:48 UTC
Hi... I am a bit confused here (but I am not an expert in CSS parsing): 
isn't 'inherit' a valid value for fant-family?
Comment 3 Gérard Talbot (no longer involved) 2008-08-23 22:06:22 UTC
> isn't 'inherit' a valid value for fant-family?

Hello Maksin,

Yes, inherit is a correct and valid value for font-family:

h1 {font-family: inherit;}

is clear, without ambiguity or source of interpretation. But when the CSS parser meets

h1 {font: 96px inherit;}

the CSS parser has no way to know for sure which other font subproperty is actually explicitly being inherited. On the other hand, 

h1 {font: inherit;}

is clear because it applies to all font subproperties (*) and it meets, honors the font shorthand syntax.

The font shorthand can take several font subproperties. The problem for the CSS parser is that it wouldn't be able to figure out which font subproperty. Imagine this situation:

h1 {font: inherit inherit;}

Which pair of font sub-properties have now inherited from its parent? font-size and font-family? font-weight and font-family? font-size and font-style? font-variant and font-style?

The CSS 2.1 spec. is a bit implicit, equivocal on this issue. All the 15.8 section had to say explicitly is that inherit keyword has to apply to all font subproperties when it is involved, used for the font shorthand. Just like it is said for system font reserved keywords:

"System fonts may only be set as a whole; that is, the font family, size, weight, style, etc. are all set at the same time."

It does however states in its syntax that inherit must occur only once. And that is what the CSS validator message is about:

"Value Error : font  Too many values or values are not recognized :  96px inherit "

One last thing. The last paragraph of section 15.8 says:
"the 'font' shorthand property resets any [sub]property not explicitly given a value to its initial value".
Comment 4 Gérard Talbot (no longer involved) 2008-08-23 22:13:44 UTC
> inherit must occur only once.

[ [ <'font-style'> || <'font-variant'> || <'font-weight'> ]? <'font-size'> [ /
<'line-height'> ]? <'font-family'> ] | caption | icon | menu | message-box |
small-caption | status-bar | inherit

"A bar (|) separates two or more alternatives: exactly one of them must occur."
http://www.w3.org/TR/CSS21/about.html#value-defs
Comment 5 Gérard Talbot (no longer involved) 2008-08-23 22:32:09 UTC
> Hello Maksin,

Sorry! Maksim :)
Comment 6 Gérard Talbot (no longer involved) 2008-11-15 19:04:17 UTC
"
consider the declaration
font: inherit 100% Arial;
Does 'inherit' set font-style, or font-variant, or font-weight? They are
allowed in any order, and they are all optional, so which one is it?
"

coming from
Newsgroup: comp.infosystems.www.authoring.stylesheets
Date: Mon, Nov 24 2003
Subject: font-family: inherit -- does not work with shorthand?
http://groups.google.com/group/comp.infosystems.www.authoring.stylesheets/msg/d933e2c079cdb3e4?hl=en
Comment 7 Gérard Talbot (no longer involved) 2009-05-02 20:02:38 UTC
From CSS 2.1, Candidate Recommendation 23 April 2009, Appendix C. Changes

{
C.3.1 Shorthand properties

Shorthand properties take a list of subproperty values or the value 'inherit'. One cannot mix 'inherit' with other subproperty values as it would not be possible to specify the subproperty to which 'inherit' applied. The definitions of a number of shorthand properties did not enforce this rule: 'border-top', 'border-right', 'border-bottom', 'border-left', 'border', 'background', 'font', 'list-style', 'cue', and 'outline'. 
}
http://www.w3.org/TR/CSS21/changes.html#q142

regards, Gérard
Comment 8 Gérard Talbot (no longer involved) 2009-11-16 23:55:47 UTC
I submitted a precise test regarding this issue to be included in the upcoming CSS 2.1 test suite and it was reviewed and approved by Microsoft (Arron Eicholz):

http://test.csswg.org/source/contributors/gtalbot/submitted/font-002.html

also at

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/font-002.html

regards, Gérard
Comment 9 Gérard Talbot (no longer involved) 2010-01-04 22:04:03 UTC
Due to filename numbering conflicts, the URL for the test cases had to be changed. So now, correct URLs:

http://test.csswg.org/source/contributors/gtalbot/submitted/font-045.html

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/font-045.html

regards, Gérard
Comment 10 Gérard Talbot (no longer involved) 2010-02-19 20:53:05 UTC
Correct URLs for the testcases:

http://test.csswg.org/source/contributors/gtalbot/submitted/font-045.xhtml

http://www.gtalbot.org/BrowserBugsSection/css21testsuite/font-045.html

Testcase in current CSS 2.1 test suite snapshot:
http://www.w3.org/Style/CSS/Test/CSS2.1/20100127/html4/font-045.htm

For each and all of those testcases, the tester needs to install the Ahem font, downloadable here:

http://www.w3.org/Style/CSS/Test/Ahem/

regards, Gérard
Comment 11 Gérard Talbot (no longer involved) 2010-04-05 04:24:21 UTC
Correct URLs (served as text/html only):

http://test.csswg.org/source/contributors/gtalbot/submitted/font-045.htm

Current snapshot of CSS 2.1 test suite:
http://www.w3.org/Style/CSS/Test/CSS2.1/20100316/html4/font-045.htm

regards, Gérard
Comment 12 Andrea Iacovitti 2012-01-05 07:55:29 UTC
Confirmed on linux systems too.
Comment 13 Gérard Talbot (no longer involved) 2012-01-20 04:56:19 UTC
Latest snapshot (served as text/html):
http://test.csswg.org/suites/css2.1/latest/html4/font-045.htm
Comment 14 Andrea Iacovitti 2014-05-10 23:57:59 UTC
Git commit f563cf1608ab820d7357fbdbae7ea4886352bf70 by Andrea Iacovitti.
Committed on 10/05/2014 at 23:56.
Pushed by aiacovitti into branch 'KDE/4.13'.

Reject font shorthand property when inherit value is present in font-family subproperty.
FIXED-IN: 4.13.2

M  +16   -8    khtml/css/cssparser.cpp

http://commits.kde.org/kdelibs/f563cf1608ab820d7357fbdbae7ea4886352bf70
Comment 15 Gérard Talbot (no longer involved) 2014-05-11 00:59:55 UTC
Andrea,

You may want to check your patch with/against this meta-test:

http://test.csswg.org/source/contributors/gtalbot/submitted/font-family-rule-004a.htm

because it involves 9 sub-tests on inherit (as identifier or not).

You need to have installed the fonts "CSSTest Fallback", "CSSTest FamilyName" and "CSSTest Verify" available at/downloadable from
http://www.w3.org/Style/CSS/Test/Fonts/

Gérard
Comment 16 Gérard Talbot (no longer involved) 2014-05-11 01:02:11 UTC
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/font-family-rule-004a.htm
Comment 17 Gérard Talbot (no longer involved) 2014-06-13 17:29:12 UTC
Tests
http://www.gtalbot.org/BrowserBugsSection/MSIE7Bugs/font-shorthand-inherit.html
and
http://test.csswg.org/suites/css2.1/latest/html4/font-045.htm
now pass in Konqueror 4.13.2 

I may create another bug report for sub-test cases in
http://test.csswg.org/suites/css2.1/nightly-unstable/html4/font-family-rule-004a.htm
that Konqueror currently fails.

Status -> VERIFIED

Thank you Andrea !! :)