font-style: oblique with calc() should allow out-of-range angles, and…

… clamp them for computed style

https://bugs.webkit.org/show_bug.cgi?id=246909
rdar://problem/101484863

Reviewed by Tim Nguyen.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/at-font-face-descriptors-expected.txt:
Updated results. These are still failures, and the reason for this seems to be that either the
specification or the test is incorrect; specification says "specified values are not clamped".

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/font-parse-numeric-stretch-style-weight-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/font-style-parsing-expected.txt:
Updated for new pass results.

* Source/WebCore/css/CSSFontFace.cpp:
(WebCore::calculateItalicRange): Added calls to Style::BuilderConverter::convertFontStyleAngle,
which does clamping. Also refactored to tighten and simplify the function.

* Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp:
(WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontStyleAngle): Added. Needed because we
can't use consumeFontStyleRaw, since that discards calculations and turns them into numbers.
This function now only rejects out-of-range angles if the angle's value is *not* calculated.
(WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontStyleKeywordValue): Deleted.
(WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontStyleRange): Merged in the
consumeFontStyleKeywordValue function. Refactored this to use the consumeFontStyleAngle
function, and make some other simplifications.
(WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontStyle): Added. Callers can't use
consumeFontStyleRaw for the reason given above.

* Source/WebCore/style/StyleBuilderConverter.h: Removed redundant assertions that replicate the
assertion already built into the downcast<> function teplate.
(WebCore::Style::BuilderConverter::convertFontStyleAngle): Added. Does the necessary clamping.
(WebCore::Style::BuilderConverter::convertFontStyleFromValue): Call the new function above.

Canonical link: https://commits.webkit.org/255925@main

LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/at-font-face-descriptors-expected.txt

@@ -71,8 +71,8 @@ PASS font-style(valid): 'oblique' followed by zero degrees: oblique 0deg
 FAIL font-style(valid): 'oblique' followed by default 20deg angle: oblique 20deg assert_equals: Unexpected resulting value. expected "oblique" but got "oblique 20deg"
 PASS font-style(valid): 'oblique' followed by maxumum 90 degree angle: oblique 90deg
 PASS font-style(valid): 'oblique' followed by minimum -90 degree angle: oblique -90deg
-FAIL font-style(valid): 'oblique' followed by calc with out of range value (should be clamped): oblique calc(91deg) assert_not_equals: Valid value should be accepted. got disallowed value ""
-FAIL font-style(valid): 'oblique' followed by calc with out of range value (should be clamped): oblique calc(-91deg) assert_not_equals: Valid value should be accepted. got disallowed value ""
+FAIL font-style(valid): 'oblique' followed by calc with out of range value (should be clamped): oblique calc(91deg) assert_equals: Unexpected resulting value. expected "oblique 90deg" but got "oblique calc(91deg)"
+FAIL font-style(valid): 'oblique' followed by calc with out of range value (should be clamped): oblique calc(-91deg) assert_equals: Unexpected resulting value. expected "oblique -90deg" but got "oblique calc(-91deg)"
 FAIL font-style(valid): 'oblique' followed by  angle in radians: oblique 0rad assert_equals: Unexpected resulting value. expected "oblique 0deg" but got "oblique 0rad"
 PASS font-style(invalid): 'oblique' followed by unit-less number: oblique 20
 PASS font-style(invalid): 'oblique' followed by non-angle: oblique 20px

LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/font-parse-numeric-stretch-style-weight-expected.txt

@@ -18,7 +18,7 @@ PASS Valid value oblique for font property style used for styling.
 PASS Valid value oblique 50deg for font property style used for styling.
 PASS Valid value oblique -90deg for font property style used for styling.
 PASS Valid value oblique 90deg for font property style used for styling.
-FAIL Valid value oblique calc(90deg + 20deg) for font property style used for styling. assert_true: expected true got false
+PASS Valid value oblique calc(90deg + 20deg) for font property style used for styling.
 PASS Valid value oblique calc(30deg + 20deg) for font property style used for styling.
 PASS Invalid value 100 400 for font property weight used for styling.
 PASS Invalid value 100% 110% for font property stretch used for styling.
@@ -74,9 +74,7 @@ PASS Value 50% 0 must not be accepted as stretch in @font-face.
 PASS Value oblique 100deg must not be accepted as style in @font-face.
 PASS Value oblique italic must not be accepted as style in @font-face.
 PASS Value oblique -91deg must not be accepted as style in @font-face.
-FAIL Value oblique 0 must not be accepted as style in @font-face. assert_throws_dom: Value must not be accepted as weight value. function "() => {
-              fontFace[theProperty] = faceTest;
-            }" did not throw
+PASS Value oblique 0 must not be accepted as style in @font-face.
 PASS Value oblique 10 must not be accepted as style in @font-face.
 PASS Value iiitalic must not be accepted as style in @font-face.
 PASS Value 90deg must not be accepted as style in @font-face.

LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/variations/font-style-parsing-expected.txt

@@ -23,8 +23,8 @@ PASS Font-style (supports): 'oblique' followed by isolated minus is invalid
 PASS Font-style (supports): 'oblique' followed by minus and angle separated by space is invalid
 PASS Font-style (supports): 'oblique' followed by minus and non-number is invalid
 PASS Font-style (supports): 'oblique' followed by calc is valid
-FAIL Font-style (supports): 'oblique' followed by calc is valid even if it must be clamped (no computation) assert_equals: Font-style supports: 'oblique' followed by calc is valid even if it must be clamped (no computation) expected true but got false
-FAIL Font-style (supports): 'oblique' followed by calc is valid even if it must be clamped (with computation) assert_equals: Font-style supports: 'oblique' followed by calc is valid even if it must be clamped (with computation) expected true but got false
+PASS Font-style (supports): 'oblique' followed by calc is valid even if it must be clamped (no computation)
+PASS Font-style (supports): 'oblique' followed by calc is valid even if it must be clamped (with computation)
 PASS Font-style (supports): 'oblique' followed by calc is valid even if it mixes units (with computation)
 PASS Font-style (computed): 'italic' is valid
 PASS Font-style (computed): 'oblique' is valid
@@ -39,7 +39,7 @@ PASS Font-style (computed): 'oblique' followed by maxumum 90 degree angle is val
 PASS Font-style (computed): 'oblique' followed by minimum -90 degree angle is valid
 PASS Font-style (computed): 'oblique' followed by positive angle is valid
 PASS Font-style (computed): 'oblique' followed by calc is valid
-FAIL Font-style (computed): 'oblique' followed by calc is valid even if it must be clamped (no computation) assert_equals: Font-style computed style: 'oblique' followed by calc is valid even if it must be clamped (no computation) expected "oblique -90deg" but got "normal"
-FAIL Font-style (computed): 'oblique' followed by calc is valid even if it must be clamped (with computation) assert_equals: Font-style computed style: 'oblique' followed by calc is valid even if it must be clamped (with computation) expected "oblique 90deg" but got "normal"
+PASS Font-style (computed): 'oblique' followed by calc is valid even if it must be clamped (no computation)
+PASS Font-style (computed): 'oblique' followed by calc is valid even if it must be clamped (with computation)
 PASS Font-style (computed): 'oblique' followed by calc is valid even if it mixes units (with computation)
 

Source/WebCore/css/CSSFontFace.cpp

@@ -246,34 +246,26 @@ void CSSFontFace::setStretch(CSSValue& style)
 
 static FontSelectionRange calculateItalicRange(CSSValue& value)
 {
-    if (value.isFontStyleValue()) {
-        auto result = Style::BuilderConverter::convertFontStyleFromValue(value);
-        return { result.value_or(normalItalicValue()), result.value_or(normalItalicValue()) };
-    }
+    if (value.isFontStyleValue())
+        return FontSelectionRange { Style::BuilderConverter::convertFontStyleFromValue(value).value_or(normalItalicValue()) };
 
-    ASSERT(value.isFontStyleRangeValue());
     auto& rangeValue = downcast<CSSFontStyleRangeValue>(value);
-    ASSERT(rangeValue.fontStyleValue->isValueID());
-    auto valueID = rangeValue.fontStyleValue->valueID();
+    auto keyword = rangeValue.fontStyleValue->valueID();
     if (!rangeValue.obliqueValues) {
-        if (valueID == CSSValueNormal)
-            return { normalItalicValue(), normalItalicValue() };
-        ASSERT(valueID == CSSValueItalic || valueID == CSSValueOblique);
-        return { italicValue(), italicValue() };
+        if (keyword == CSSValueNormal)
+            return FontSelectionRange { normalItalicValue() };
+        ASSERT(keyword == CSSValueItalic || keyword == CSSValueOblique);
+        return FontSelectionRange { italicValue() };
     }
-    ASSERT(valueID == CSSValueOblique);
+    ASSERT(keyword == CSSValueOblique);
     auto length = rangeValue.obliqueValues->length();
-    if (length == 1) {
-        auto& primitiveValue = downcast<CSSPrimitiveValue>(*rangeValue.obliqueValues->item(0));
-        FontSelectionValue result(primitiveValue.value<float>(CSSUnitType::CSS_DEG));
-        return { result, result };
-    }
-    ASSERT(length == 2);
-    auto& primitiveValue1 = downcast<CSSPrimitiveValue>(*rangeValue.obliqueValues->item(0));
-    auto& primitiveValue2 = downcast<CSSPrimitiveValue>(*rangeValue.obliqueValues->item(1));
-    FontSelectionValue result1(primitiveValue1.value<float>(CSSUnitType::CSS_DEG));
-    FontSelectionValue result2(primitiveValue2.value<float>(CSSUnitType::CSS_DEG));
-    return { result1, result2 };
+    ASSERT(length == 1 || length == 2);
+    auto angleAtIndex = [&] (size_t index) {
+        return Style::BuilderConverter::convertFontStyleAngle(*rangeValue.obliqueValues->itemWithoutBoundsCheck(index));
+    };
+    if (length == 1)
+        return FontSelectionRange { angleAtIndex(0) };
+    return { angleAtIndex(0), angleAtIndex(1) };
 }
 
 void CSSFontFace::setStyle(CSSValue& style)

Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp

@@ -260,63 +260,68 @@ RefPtr<CSSValueList> consumeFontFaceSrc(CSSParserTokenRange& range, const CSSPar
     return values;
 }
 
-RefPtr<CSSFontStyleValue> consumeFontStyle(CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSValuePool& pool)
-{
-    if (auto result = CSSPropertyParserHelpers::consumeFontStyleRaw(range, cssParserMode)) {
 #if ENABLE(VARIATION_FONTS)
-        if (result->style == CSSValueOblique && result->angle) {
-            return CSSFontStyleValue::create(pool.createIdentifierValue(CSSValueOblique),
-                pool.createValue(result->angle->value, result->angle->type));
-        }
-#endif
-        return CSSFontStyleValue::create(pool.createIdentifierValue(result->style));
-    }
-    return nullptr;
-}
 
-#if ENABLE(VARIATION_FONTS)
-static RefPtr<CSSPrimitiveValue> consumeFontStyleKeywordValue(CSSParserTokenRange& range, CSSValuePool& pool)
+static RefPtr<CSSPrimitiveValue> consumeFontStyleAngle(CSSParserTokenRange& range, CSSParserMode mode, CSSValuePool& pool)
 {
-    return CSSPropertyParserHelpers::consumeIdentWorkerSafe<CSSValueNormal, CSSValueItalic, CSSValueOblique>(range, pool);
+    auto rangeAfterAngle = range;
+    auto angle = CSSPropertyParserHelpers::consumeAngleWorkerSafe(rangeAfterAngle, mode, pool);
+    if (!angle)
+        return nullptr;
+    if (!angle->isCalculated() && !CSSPropertyParserHelpers::isFontStyleAngleInRange(angle->doubleValue(CSSUnitType::CSS_DEG)))
+        return nullptr;
+    range = rangeAfterAngle;
+    return angle;
 }
 
-RefPtr<CSSFontStyleRangeValue> consumeFontStyleRange(CSSParserTokenRange& range, CSSParserMode cssParserMode, CSSValuePool& pool)
+RefPtr<CSSFontStyleRangeValue> consumeFontStyleRange(CSSParserTokenRange& range, CSSParserMode mode, CSSValuePool& pool)
 {
-    auto keyword = consumeFontStyleKeywordValue(range, pool);
+    auto keyword = CSSPropertyParserHelpers::consumeIdentWorkerSafe<CSSValueNormal, CSSValueItalic, CSSValueOblique>(range, pool);
     if (!keyword)
         return nullptr;
 
     if (keyword->valueID() != CSSValueOblique || range.atEnd())
         return CSSFontStyleRangeValue::create(keyword.releaseNonNull());
 
-    // FIXME: This should probably not allow the unitless zero.
-    if (auto firstAngle = CSSPropertyParserHelpers::consumeAngleWorkerSafe(range, cssParserMode, pool, CSSPropertyParserHelpers::UnitlessQuirk::Forbid, CSSPropertyParserHelpers::UnitlessZeroQuirk::Allow)) {
-        auto firstAngleInDegrees = firstAngle->doubleValue(CSSUnitType::CSS_DEG);
-        if (!CSSPropertyParserHelpers::isFontStyleAngleInRange(firstAngleInDegrees))
-            return nullptr;
-        if (range.atEnd()) {
-            auto result = CSSValueList::createSpaceSeparated();
-            result->append(firstAngle.releaseNonNull());
-            return CSSFontStyleRangeValue::create(keyword.releaseNonNull(), WTFMove(result));
-        }
+    auto rangeAfterAngles = range;
+    auto firstAngle = consumeFontStyleAngle(rangeAfterAngles, mode, pool);
+    if (!firstAngle)
+        return nullptr;
+
+    auto result = CSSValueList::createSpaceSeparated();
+    result->append(firstAngle.releaseNonNull());
 
-        // FIXME: This should probably not allow the unitless zero.
-        auto secondAngle = CSSPropertyParserHelpers::consumeAngleWorkerSafe(range, cssParserMode, pool, CSSPropertyParserHelpers::UnitlessQuirk::Forbid, CSSPropertyParserHelpers::UnitlessZeroQuirk::Allow);
+    if (!rangeAfterAngles.atEnd()) {
+        auto secondAngle = consumeFontStyleAngle(rangeAfterAngles, mode, pool);
         if (!secondAngle)
             return nullptr;
-        auto secondAngleInDegrees = secondAngle->doubleValue(CSSUnitType::CSS_DEG);
-        if (!CSSPropertyParserHelpers::isFontStyleAngleInRange(secondAngleInDegrees))
-            return nullptr;
-        auto result = CSSValueList::createSpaceSeparated();
-        result->append(firstAngle.releaseNonNull());
         result->append(secondAngle.releaseNonNull());
-        return CSSFontStyleRangeValue::create(keyword.releaseNonNull(), WTFMove(result));
     }
 
-    return nullptr;
+    range = rangeAfterAngles;
+    return CSSFontStyleRangeValue::create(keyword.releaseNonNull(), WTFMove(result));
 }
+
+#endif
+
+RefPtr<CSSFontStyleValue> consumeFontStyle(CSSParserTokenRange& range, CSSParserMode mode, CSSValuePool& pool)
+{
+    auto keyword = CSSPropertyParserHelpers::consumeIdentWorkerSafe<CSSValueNormal, CSSValueItalic, CSSValueOblique>(range, pool);
+    if (!keyword)
+        return nullptr;
+
+#if ENABLE(VARIATION_FONTS)
+    if (keyword->valueID() == CSSValueOblique && !range.atEnd()) {
+        if (auto angle = consumeFontStyleAngle(range, mode, pool))
+            return CSSFontStyleValue::create(keyword.releaseNonNull(), angle.releaseNonNull());
+    }
+#else
+    UNUSED_PARAM(mode);
 #endif
 
+    return CSSFontStyleValue::create(keyword.releaseNonNull());
+}
+
 static RefPtr<CSSPrimitiveValue> consumeFontWeightAbsoluteKeywordValue(CSSParserTokenRange& range, CSSValuePool& pool)
 {
     return CSSPropertyParserHelpers::consumeIdentWorkerSafe<CSSValueNormal, CSSValueBold>(range, pool);

Source/WebCore/style/StyleBuilderConverter.h

@@ -144,6 +144,7 @@ class BuilderConverter {
     static bool convertSmoothScrolling(BuilderState&, const CSSValue&);
     static FontSelectionValue convertFontWeightFromValue(const CSSValue&);
     static FontSelectionValue convertFontStretchFromValue(const CSSValue&);
+    static FontSelectionValue convertFontStyleAngle(const CSSValue&);
     static std::optional<FontSelectionValue> convertFontStyleFromValue(const CSSValue&);
     static FontSelectionValue convertFontWeight(BuilderState&, const CSSValue&);
     static FontSelectionValue convertFontStretch(BuilderState&, const CSSValue&);
@@ -791,13 +792,11 @@ inline RefPtr<QuotesData> BuilderConverter::convertQuotes(BuilderState&, const C
 
 inline TextUnderlinePosition BuilderConverter::convertTextUnderlinePosition(BuilderState&, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     return downcast<CSSPrimitiveValue>(value);
 }
 
 inline TextUnderlineOffset BuilderConverter::convertTextUnderlineOffset(BuilderState& builderState, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
     switch (primitiveValue.valueID()) {
     case CSSValueAuto:
@@ -811,7 +810,6 @@ inline TextUnderlineOffset BuilderConverter::convertTextUnderlineOffset(BuilderS
 
 inline TextDecorationThickness BuilderConverter::convertTextDecorationThickness(BuilderState& builderState, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
     switch (primitiveValue.valueID()) {
     case CSSValueAuto:
@@ -968,7 +966,6 @@ inline ScrollSnapAlign BuilderConverter::convertScrollSnapAlign(BuilderState&, c
 
 inline ScrollSnapStop BuilderConverter::convertScrollSnapStop(BuilderState&, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     return downcast<CSSPrimitiveValue>(value);
 }
 
@@ -992,7 +989,6 @@ inline GridTrackSize BuilderConverter::createGridTrackSize(const CSSValue& value
     if (is<CSSPrimitiveValue>(value))
         return GridTrackSize(createGridTrackBreadth(downcast<CSSPrimitiveValue>(value), builderState));
 
-    ASSERT(is<CSSFunctionValue>(value));
     const auto& function = downcast<CSSFunctionValue>(value);
 
     if (function.length() == 1)
@@ -1312,7 +1308,6 @@ inline FontFeatureSettings BuilderConverter::convertFontFeatureSettings(BuilderS
 
 inline FontSelectionValue BuilderConverter::convertFontWeightFromValue(const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
 
     if (primitiveValue.isNumber())
@@ -1335,7 +1330,6 @@ inline FontSelectionValue BuilderConverter::convertFontWeightFromValue(const CSS
 
 inline FontSelectionValue BuilderConverter::convertFontStretchFromValue(const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     const auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
 
     if (primitiveValue.isPercentage())
@@ -1348,10 +1342,14 @@ inline FontSelectionValue BuilderConverter::convertFontStretchFromValue(const CS
     return normalStretchValue();
 }
 
+inline FontSelectionValue BuilderConverter::convertFontStyleAngle(const CSSValue& value)
+{
+    return FontSelectionValue { std::clamp(downcast<CSSPrimitiveValue>(value).value<float>(CSSUnitType::CSS_DEG), -90.0f, 90.0f) };
+}
+
 // The input value needs to parsed and valid, this function returns std::nullopt if the input was "normal".
 inline std::optional<FontSelectionValue> BuilderConverter::convertFontStyleFromValue(const CSSValue& value)
 {
-    ASSERT(is<CSSFontStyleValue>(value));
     const auto& fontStyleValue = downcast<CSSFontStyleValue>(value);
 
     auto valueID = fontStyleValue.fontStyleValue->valueID();
@@ -1361,13 +1359,12 @@ inline std::optional<FontSelectionValue> BuilderConverter::convertFontStyleFromV
         return italicValue();
     ASSERT(valueID == CSSValueOblique);
     if (auto* obliqueValue = fontStyleValue.obliqueValue.get())
-        return FontSelectionValue(obliqueValue->value<float>(CSSUnitType::CSS_DEG));
+        return convertFontStyleAngle(*obliqueValue);
     return italicValue();
 }
 
 inline FontSelectionValue BuilderConverter::convertFontWeight(BuilderState& builderState, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
     if (primitiveValue.isValueID()) {
         auto valueID = primitiveValue.valueID();
@@ -1617,7 +1614,6 @@ inline std::optional<Length> BuilderConverter::convertLineHeight(BuilderState& b
 
 inline FontPalette BuilderConverter::convertFontPalette(BuilderState&, const CSSValue& value)
 {
-    ASSERT(is<CSSPrimitiveValue>(value));
     const auto& primitiveValue = downcast<CSSPrimitiveValue>(value);
     switch (primitiveValue.valueID()) {
     case CSSValueNormal: