The following consecutive CSS rules h1 {font-size: 32px;} h1 {font: 96px inherit;} should be parsed and only the h1 {font-size: 32px;} rule should apply. A parsing error should be found for h1 {font: 96px inherit;} rule. Steps to reproduce: 1- Load provided URL Actual results in Safari 3.1.2 build 525.21 The rendered text will be using a font-size of 96px Expected results The rendered text should be using a font-size of 32px Notes: - Section 15.8 of CSS 2.1 http://www.w3.org/TR/CSS21/fonts.html#font-shorthand only allows font: inherit as correct - CSS validator will report h1 Value Error : font Too many values or values are not recognized : 96px inherit - I searched for a duplicate and did not find any
This bug, it seems, also occurs in Safari 4.0 / Mac OS X 10.5 (Leopard) AppleWebKit build 526.11. Therefore, setting version to 526+
[ [ <'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 Worth noting is that "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." The same should have been said about inherit to avoid possible confusion, to be explicit.
Created attachment 22535 [details] minimal test case
Bug 169610: font shorthand and inherit incorrectly parsed https://bugs.kde.org/show_bug.cgi?id=169610 has more explanations on this bug. Also filed for Opera: http://www.gtalbot.org/BrowserBugsSection/Opera9Bugs/#bug35
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
Robert, your minimal testcase, attachment 22535 [details] , raises a problem, at least a question. Font shorthand requires font-size and font-family if one of the 2 properties are defined. In other words: div {font: 1px; } should be parsed as an error and be rejected and ignored. Was this intentional/deliberate on your part? Just asking.. regards, Gérard
Created attachment 31279 [details] minimal test case (fixed)
(In reply to comment #6) > your minimal testcase, attachment 22535 [details] [review] , raises a problem, at least a > question. Font shorthand requires font-size and font-family if one of the 2 > properties are defined. I fixed the test case to avoid confusion.
Created attachment 126809 [details] Patch
(In reply to comment #9) > Created an attachment (id=126809) [details] > Patch Though in a following patch I think the parseFont() could be improved.
Comment on attachment 126809 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=126809&action=review > Source/WebCore/css/CSSParser.cpp:4303 > + if (value->id == CSSValueInitial || value->id == CSSValueInherit) > + return list.release(); Is this correct? It looks like it's possible for list to have values appended to it that are returned. Can you add some tests that just set font family directly? > LayoutTests/fast/css/font-shorthand-mix-inherit.html:29 > + test.style.font = "12pt/14pt inherit"; > + shouldBe("test.style.getPropertyValue('font')", "''"); Can you also add a test for "inherit sans-serif"? > LayoutTests/fast/css/font-shorthand-mix-inherit.html:36 > + test.style.font = "x-large/110% \"new century schoolbook\", serif, inherit"; Nit: Use '' so you don't have to escape the "s.
Created attachment 127012 [details] Patch
Comment on attachment 127012 [details] Patch Clearing flags on attachment: 127012 Committed r107728: <http://trac.webkit.org/changeset/107728>
All reviewed patches have been landed. Closing bug.
Hello, As far as I can see, it appears that this bug has NOT been fixed. Under Linux, when using Chrome 17.0.963.79, the tested sentence uses a font-size of 96px and not 32px. Under XP Pro SP3, when using Safari 5.1.4 (build 7534.54.16), the tested sentence uses a font-size of 96px and not 32px. Is this bug supposed to be fixed with next release of Safari and Chrome? Just asking... regards, Gérard
(In reply to comment #15) > Hello, Hi, > > As far as I can see, it appears that this bug has NOT been fixed. > It has been fixed and is covered by a test. > Under Linux, when using Chrome 17.0.963.79, the tested sentence uses a font-size of 96px and not 32px. > > Under XP Pro SP3, when using Safari 5.1.4 (build 7534.54.16), the tested sentence uses a font-size of 96px and not 32px. > > Is this bug supposed to be fixed with next release of Safari and Chrome? Just asking... > There is delay, you may not see yet the fix. It's there you just need to wait that Google or Apple rebase the webkit shipping with their browsers. > regards, Gérard
With Chrome 23.0.1271.91, I get expected results. Thank you Alexis! Marking as VERIFIED
(In reply to comment #17) > With Chrome 23.0.1271.91, I get expected results. Thank you Alexis! > > Marking as VERIFIED You're welcome!