font shorthand should accept variation values
authormmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Mar 2017 23:14:02 +0000 (23:14 +0000)
committermmaxfield@apple.com <mmaxfield@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Mar 2017 23:14:02 +0000 (23:14 +0000)
https://bugs.webkit.org/show_bug.cgi?id=168998

Reviewed by Simon Fraser.

Source/WebCore:

The CSS Fonts 4 spec has stabilized as to which variation values are allowed in
the font shorthand property. Weights are allowed because a 0 weight is considered
as a parse error, so there is no conflict with a unitless font-size of 0.
font-style accepts angles, so there is no conflict there. However, font-stretch
accepts percentages, which are also accepted by font-size, which means the newly
extended grammar for font-stretch can't be accepted in the shorthand.

Tests: fast/text/font-style-parse.html
       fast/text/font-weight-parse.html

* css/parser/CSSPropertyParser.cpp:
(WebCore::consumeFontWeight):
(WebCore::consumeFontStyle):
(WebCore::CSSPropertyParser::consumeFont):
(WebCore::consumeFontWeightCSS21): Deleted.
* css/parser/CSSPropertyParserHelpers.cpp:
(WebCore::CSSPropertyParserHelpers::consumeFontWeightNumber):
* css/parser/CSSPropertyParserHelpers.h:

LayoutTests:

* fast/text/font-style-parse-expected.txt:
* fast/text/font-style-parse.html:
* fast/text/font-weight-parse-expected.txt:
* fast/text/font-weight-parse.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@214324 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/text/font-style-parse-expected.txt
LayoutTests/fast/text/font-style-parse.html
LayoutTests/fast/text/font-weight-parse-expected.txt
LayoutTests/fast/text/font-weight-parse.html
Source/WebCore/ChangeLog
Source/WebCore/css/parser/CSSPropertyParser.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.cpp
Source/WebCore/css/parser/CSSPropertyParserHelpers.h

index 6b191d5..934be51 100644 (file)
@@ -1,3 +1,15 @@
+2017-03-22  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        font shorthand should accept variation values
+        https://bugs.webkit.org/show_bug.cgi?id=168998
+
+        Reviewed by Simon Fraser.
+
+        * fast/text/font-style-parse-expected.txt:
+        * fast/text/font-style-parse.html:
+        * fast/text/font-weight-parse-expected.txt:
+        * fast/text/font-weight-parse.html:
+
 2017-03-23  Chris Dumez  <cdumez@apple.com>
 
         SVG animations are not paused when inserted into a hidden page
index 60b3d5c..7af10cf 100644 (file)
@@ -18,6 +18,7 @@ PASS window.getComputedStyle(document.getElementById('test17')).fontStyle is "no
 PASS window.getComputedStyle(document.getElementById('test18')).fontStyle is "italic"
 PASS window.getComputedStyle(document.getElementById('test19')).fontStyle is "italic"
 PASS window.getComputedStyle(document.getElementById('test20')).fontStyle is "normal"
+PASS window.getComputedStyle(document.getElementById('test21')).fontStyle is "14deg"
 PASS window.getComputedStyle(document.getElementById('test1')).font is "normal normal normal normal 16px/18px Times"
 PASS window.getComputedStyle(document.getElementById('test2')).font is "16px/18px Times"
 PASS window.getComputedStyle(document.getElementById('test3')).font is "16px/18px Times"
@@ -38,6 +39,7 @@ PASS window.getComputedStyle(document.getElementById('test17')).font is "normal
 PASS window.getComputedStyle(document.getElementById('test18')).font is "italic normal normal normal 48px/49px 'Helvetica Neue'"
 PASS window.getComputedStyle(document.getElementById('test19')).font is "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'"
 PASS window.getComputedStyle(document.getElementById('test20')).font is "normal normal normal normal 16px/18px Times"
+PASS window.getComputedStyle(document.getElementById('test21')).font is "48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test1').style.font is ""
 PASS document.getElementById('test15').style.font is "italic 100 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test16').style.font is "italic 100 48px/49px 'Helvetica Neue'"
@@ -45,6 +47,7 @@ PASS document.getElementById('test17').style.font is "100 48px/49px 'Helvetica N
 PASS document.getElementById('test18').style.font is "italic 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test19').style.font is "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test20').style.font is ""
+PASS document.getElementById('test21').style.font is "14deg small-caps 123 extra-expanded 48px/49px 'Helvetica Neue'"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 9b12cae..1b5ce01 100644 (file)
@@ -25,6 +25,7 @@
 <div style="font-stretch: normal;"><div id="test18" style="font: italic 48px/49px 'Helvetica Neue';"></div></div>
 <div id="test19" style="font: italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue';"></div>
 <div id="test20" style="font: 13deg small-caps 123 extra-expanded 48px/49px 'Helvetica Neue';"></div>
+<div id="test21" style="font: oblique 14deg small-caps 123 extra-expanded 48px/49px 'Helvetica Neue';"></div>
 </div>
 <script>
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test1')).fontStyle", "normal");
@@ -47,6 +48,7 @@ shouldBeEqualToString("window.getComputedStyle(document.getElementById('test17')
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test18')).fontStyle", "italic");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test19')).fontStyle", "italic");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test20')).fontStyle", "normal");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test21')).fontStyle", "14deg");
 
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test1')).font", "normal normal normal normal 16px/18px Times");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test2')).font", "16px/18px Times");
@@ -68,6 +70,7 @@ shouldBeEqualToString("window.getComputedStyle(document.getElementById('test17')
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test18')).font", "italic normal normal normal 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test19')).font", "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test20')).font", "normal normal normal normal 16px/18px Times");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test21')).font", "48px/49px 'Helvetica Neue'");
 
 shouldBeEqualToString("document.getElementById('test1').style.font", "");
 shouldBeEqualToString("document.getElementById('test15').style.font", "italic 100 48px/49px 'Helvetica Neue'");
@@ -76,6 +79,7 @@ shouldBeEqualToString("document.getElementById('test17').style.font", "100 48px/
 shouldBeEqualToString("document.getElementById('test18').style.font", "italic 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("document.getElementById('test19').style.font", "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("document.getElementById('test20').style.font", "");
+shouldBeEqualToString("document.getElementById('test21').style.font", "14deg small-caps 123 extra-expanded 48px/49px 'Helvetica Neue'");
 </script>
 <script src="../../resources/js-test-post.js"></script>
 </body>
index 1509517..91006c4 100644 (file)
@@ -21,7 +21,12 @@ PASS window.getComputedStyle(document.getElementById('test20')).fontWeight is "1
 PASS window.getComputedStyle(document.getElementById('test21')).fontWeight is "normal"
 PASS window.getComputedStyle(document.getElementById('test22')).fontWeight is "100"
 PASS window.getComputedStyle(document.getElementById('test23')).fontWeight is "100"
-PASS window.getComputedStyle(document.getElementById('test24')).fontWeight is "normal"
+PASS window.getComputedStyle(document.getElementById('test24')).fontWeight is "123"
+PASS window.getComputedStyle(document.getElementById('test25')).fontWeight is "normal"
+PASS window.getComputedStyle(document.getElementById('test26')).fontWeight is "normal"
+PASS window.getComputedStyle(document.getElementById('test27')).fontWeight is "normal"
+PASS window.getComputedStyle(document.getElementById('test28')).fontWeight is "0"
+PASS window.getComputedStyle(document.getElementById('test29')).fontWeight is "1000"
 PASS window.getComputedStyle(document.getElementById('test1')).font is "normal normal normal normal 16px/18px Times"
 PASS window.getComputedStyle(document.getElementById('test2')).font is "16px/18px Times"
 PASS window.getComputedStyle(document.getElementById('test3')).font is "normal normal 100 normal 16px/18px Times"
@@ -45,14 +50,19 @@ PASS window.getComputedStyle(document.getElementById('test20')).font is "normal
 PASS window.getComputedStyle(document.getElementById('test21')).font is "normal normal normal extra-condensed 48px/49px 'Helvetica Neue'"
 PASS window.getComputedStyle(document.getElementById('test22')).font is "normal normal 100 normal 48px/49px 'Helvetica Neue'"
 PASS window.getComputedStyle(document.getElementById('test23')).font is "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'"
-PASS window.getComputedStyle(document.getElementById('test24')).font is "italic small-caps normal extra-expanded 48px/49px 'Helvetica Neue'"
+PASS window.getComputedStyle(document.getElementById('test24')).font is "48px/49px 'Helvetica Neue'"
+PASS window.getComputedStyle(document.getElementById('test25')).font is "normal normal normal normal 16px/18px Times"
+PASS window.getComputedStyle(document.getElementById('test26')).font is "normal normal normal normal 16px/18px Times"
+PASS window.getComputedStyle(document.getElementById('test27')).font is "normal normal normal normal 16px/18px Times"
+PASS window.getComputedStyle(document.getElementById('test28')).font is "16px/18px Times"
+PASS window.getComputedStyle(document.getElementById('test29')).font is "16px/18px Times"
 PASS document.getElementById('test1').style.font is ""
 PASS document.getElementById('test19').style.font is "100 extra-condensed 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test20').style.font is "100 extra-condensed 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test21').style.font is "extra-condensed 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test22').style.font is "100 48px/49px 'Helvetica Neue'"
 PASS document.getElementById('test23').style.font is "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'"
-PASS document.getElementById('test24').style.font is "italic small-caps extra-expanded 48px/49px 'Helvetica Neue'"
+PASS document.getElementById('test24').style.font is "italic small-caps 123 extra-expanded 48px/49px 'Helvetica Neue'"
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 3ca0991..ebe850b 100644 (file)
 <div style="font-weight: 275;"><div id="test22" style="font: 100 48px/49px 'Helvetica Neue';"></div></div>
 <div id="test23" style="font: italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue';"></div>
 <div id="test24" style="font: italic small-caps 123 extra-expanded 48px/49px 'Helvetica Neue';"></div>
+<div id="test25" style="font-weight: 0;"></div>
+<div id="test26" style="font-weight: -7;"></div>
+<div id="test27" style="font-weight: 1623;"></div>
+<div id="test28" style="font-weight: calc(-4 - 5);"></div>
+<div id="test29" style="font-weight: calc(600 + 700);"></div>
 </div>
 <script>
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test1')).fontWeight", "normal");
@@ -54,7 +59,12 @@ shouldBeEqualToString("window.getComputedStyle(document.getElementById('test20')
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test21')).fontWeight", "normal");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test22')).fontWeight", "100");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test23')).fontWeight", "100");
-shouldBeEqualToString("window.getComputedStyle(document.getElementById('test24')).fontWeight", "normal");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test24')).fontWeight", "123");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test25')).fontWeight", "normal");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test26')).fontWeight", "normal");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test27')).fontWeight", "normal");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test28')).fontWeight", "0");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test29')).fontWeight", "1000");
 
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test1')).font", "normal normal normal normal 16px/18px Times");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test2')).font", "16px/18px Times");
@@ -79,7 +89,12 @@ shouldBeEqualToString("window.getComputedStyle(document.getElementById('test20')
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test21')).font", "normal normal normal extra-condensed 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test22')).font", "normal normal 100 normal 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("window.getComputedStyle(document.getElementById('test23')).font", "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'");
-shouldBeEqualToString("window.getComputedStyle(document.getElementById('test24')).font", "italic small-caps normal extra-expanded 48px/49px 'Helvetica Neue'");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test24')).font", "48px/49px 'Helvetica Neue'");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test25')).font", "normal normal normal normal 16px/18px Times");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test26')).font", "normal normal normal normal 16px/18px Times");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test27')).font", "normal normal normal normal 16px/18px Times");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test28')).font", "16px/18px Times");
+shouldBeEqualToString("window.getComputedStyle(document.getElementById('test29')).font", "16px/18px Times");
 
 shouldBeEqualToString("document.getElementById('test1').style.font", "");
 shouldBeEqualToString("document.getElementById('test19').style.font", "100 extra-condensed 48px/49px 'Helvetica Neue'");
@@ -87,7 +102,7 @@ shouldBeEqualToString("document.getElementById('test20').style.font", "100 extra
 shouldBeEqualToString("document.getElementById('test21').style.font", "extra-condensed 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("document.getElementById('test22').style.font", "100 48px/49px 'Helvetica Neue'");
 shouldBeEqualToString("document.getElementById('test23').style.font", "italic small-caps 100 extra-expanded 48px/49px 'Helvetica Neue'");
-shouldBeEqualToString("document.getElementById('test24').style.font", "italic small-caps extra-expanded 48px/49px 'Helvetica Neue'");
+shouldBeEqualToString("document.getElementById('test24').style.font", "italic small-caps 123 extra-expanded 48px/49px 'Helvetica Neue'");
 </script>
 <script src="../../resources/js-test-post.js"></script>
 </body>
index ea326f1..cee1a3e 100644 (file)
@@ -1,3 +1,29 @@
+2017-03-22  Myles C. Maxfield  <mmaxfield@apple.com>
+
+        font shorthand should accept variation values
+        https://bugs.webkit.org/show_bug.cgi?id=168998
+
+        Reviewed by Simon Fraser.
+
+        The CSS Fonts 4 spec has stabilized as to which variation values are allowed in
+        the font shorthand property. Weights are allowed because a 0 weight is considered
+        as a parse error, so there is no conflict with a unitless font-size of 0.
+        font-style accepts angles, so there is no conflict there. However, font-stretch
+        accepts percentages, which are also accepted by font-size, which means the newly
+        extended grammar for font-stretch can't be accepted in the shorthand.
+
+        Tests: fast/text/font-style-parse.html
+               fast/text/font-weight-parse.html
+
+        * css/parser/CSSPropertyParser.cpp:
+        (WebCore::consumeFontWeight):
+        (WebCore::consumeFontStyle):
+        (WebCore::CSSPropertyParser::consumeFont):
+        (WebCore::consumeFontWeightCSS21): Deleted.
+        * css/parser/CSSPropertyParserHelpers.cpp:
+        (WebCore::CSSPropertyParserHelpers::consumeFontWeightNumber):
+        * css/parser/CSSPropertyParserHelpers.h:
+
 2017-03-23  Chris Dumez  <cdumez@apple.com>
 
         SVG animations are not paused when inserted into a hidden page
index 8a66b74..acc6d69 100644 (file)
@@ -869,23 +869,11 @@ static RefPtr<CSSPrimitiveValue> consumeFontWeightKeywordValue(CSSParserTokenRan
     return consumeIdent<CSSValueNormal, CSSValueBold, CSSValueBolder, CSSValueLighter>(range);
 }
 
-static RefPtr<CSSPrimitiveValue> consumeFontWeightCSS21(CSSParserTokenRange& range)
-{
-    if (auto result = consumeFontWeightKeywordValue(range))
-        return result;
-    int weight;
-    if (!consumePositiveIntegerRaw(range, weight))
-        return nullptr;
-    if (!isCSS21Weight(weight))
-        return nullptr;
-    return CSSValuePool::singleton().createValue(weight, CSSPrimitiveValue::CSS_NUMBER);
-}
-
 static RefPtr<CSSPrimitiveValue> consumeFontWeight(CSSParserTokenRange& range)
 {
     if (auto result = consumeFontWeightKeywordValue(range))
         return result;
-    return consumeNumber(range, ValueRangeAll);
+    return consumeFontWeightNumber(range);
 }
 
 #if ENABLE(VARIATION_FONTS)
@@ -982,7 +970,6 @@ static RefPtr<CSSPrimitiveValue> consumeFontStyle(CSSParserTokenRange& range, CS
                 return angle;
             if (auto number = consumeNumber(range, ValueRangeAll))
                 return number;
-            return nullptr;
         }
         return result;
     }
@@ -4430,7 +4417,7 @@ bool CSSPropertyParser::consumeFont(bool important)
     while (!m_range.atEnd()) {
         CSSValueID id = m_range.peek().id();
         if (!fontStyle) {
-            fontStyle = consumeFontStyleKeywordValue(m_range);
+            fontStyle = consumeFontStyle(m_range, m_context.mode);
             if (fontStyle)
                 continue;
         }
@@ -4442,7 +4429,7 @@ bool CSSPropertyParser::consumeFont(bool important)
                 continue;
         }
         if (!fontWeight) {
-            fontWeight = consumeFontWeightCSS21(m_range);
+            fontWeight = consumeFontWeight(m_range);
             if (fontWeight)
                 continue;
         }
index a917ed2..cde7141 100644 (file)
@@ -197,6 +197,24 @@ RefPtr<CSSPrimitiveValue> consumeNumber(CSSParserTokenRange& range, ValueRange v
     return nullptr;
 }
 
+RefPtr<CSSPrimitiveValue> consumeFontWeightNumber(CSSParserTokenRange& range)
+{
+    // Values less than or equal to 0 or greater than or equal to 1000 are parse errors.
+    auto& token = range.peek();
+    if (token.type() == NumberToken && token.numericValue() > 0 && token.numericValue() < 1000)
+        return consumeNumber(range, ValueRangeAll);
+
+    // "[For calc()], the used value resulting from an expression must be clamped to the range allowed in the target context."
+    CalcParser calcParser(range, ValueRangeAll);
+    double result;
+    if (calcParser.consumeNumberRaw(result)) {
+        result = std::min(std::max(result, std::nextafter(0., 1.)), std::nextafter(1000., 0.));
+        return CSSValuePool::singleton().createValue(result, CSSPrimitiveValue::UnitType::CSS_NUMBER);
+    }
+
+    return nullptr;
+}
+
 inline bool shouldAcceptUnitlessValue(double value, CSSParserMode cssParserMode, UnitlessQuirk unitless)
 {
     // FIXME: Presentational HTML attributes shouldn't use the CSS parser for lengths
index 754e3e5..d957823 100644 (file)
@@ -62,6 +62,7 @@ bool consumePositiveIntegerRaw(CSSParserTokenRange&, int& result);
 RefPtr<CSSPrimitiveValue> consumePositiveInteger(CSSParserTokenRange&);
 bool consumeNumberRaw(CSSParserTokenRange&, double& result);
 RefPtr<CSSPrimitiveValue> consumeNumber(CSSParserTokenRange&, ValueRange);
+RefPtr<CSSPrimitiveValue> consumeFontWeightNumber(CSSParserTokenRange&);
 RefPtr<CSSPrimitiveValue> consumeLength(CSSParserTokenRange&, CSSParserMode, ValueRange, UnitlessQuirk = UnitlessQuirk::Forbid);
 RefPtr<CSSPrimitiveValue> consumePercent(CSSParserTokenRange&, ValueRange);
 RefPtr<CSSPrimitiveValue> consumeLengthOrPercent(CSSParserTokenRange&, CSSParserMode, ValueRange, UnitlessQuirk = UnitlessQuirk::Forbid);