Skip to content
Permalink
Browse files
Fix handling of font-variant: normal/none shorthand 
https://bugs.webkit.org/show_bug.cgi?id=244219
<rdar://98998615>

Reviewed by Antti Koivisto.

- `font-variant: normal/none` should reset font-variant-numeric and font-variant-alternates
- Fix computed style of `font-variant: none` to not expand `font-variant-ligatures`

Tests:
- fast/text/font-variant-shorthand.html
- imported/w3c/web-platform-tests/css/css-fonts/inheritance.html
- imported/w3c/web-platform-tests/css/css-fonts/font-variant-01.html (still failing for different reason)
- imported/w3c/web-platform-tests/css/css-fonts/font-variant-02.html (still failing for different reason)

* LayoutTests/fast/text/font-variant-shorthand-expected.txt:
* LayoutTests/fast/text/font-variant-shorthand.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/inheritance-expected.txt:
* Source/WebCore/css/CSSComputedStyleDeclaration.cpp:
(WebCore::ComputedStyleExtractor::fontVariantShorthandValue):
(WebCore::ComputedStyleExtractor::valueForPropertyInStyle):
(WebCore::fontVariantFromStyle): Deleted.
* Source/WebCore/css/CSSComputedStyleDeclaration.h:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeFontVariantShorthand):

Canonical link: https://commits.webkit.org/253731@main
  • Loading branch information
@nt1m
nt1m committed Aug 24, 2022
1 parent 0488369 commit d1256a00e8cc12f4bd1e496108cb8b9c98f31442
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 186 deletions.
4 LayoutTests/fast/text/font-variant-shorthand-expected.txt
View file
@@ -35,7 +35,9 @@ PASS window.getComputedStyle(document.getElementById('t14')).getPropertyValue('f
PASS window.getComputedStyle(document.getElementById('t15')).getPropertyValue('font-variant') is "normal"
PASS window.getComputedStyle(document.getElementById('t16')).getPropertyValue('font-variant') is "common-ligatures super small-caps lining-nums historical-forms simplified"
PASS window.getComputedStyle(document.getElementById('t17')).getPropertyValue('font-variant') is "normal"
PASS window.getComputedStyle(document.getElementById('t17')).getPropertyValue('font-variant-caps') is "normal"
PASS window.getComputedStyle(document.getElementById('t18')).getPropertyValue('font-variant') is "none"
PASS window.getComputedStyle(document.getElementById('t19')).getPropertyValue('font-variant') is "normal"
PASS window.getComputedStyle(document.getElementById('t19')).getPropertyValue('font-variant-caps') is "normal"
PASS successfullyParsed is true

TEST COMPLETE
30 LayoutTests/fast/text/font-variant-shorthand.html
View file
@@ -72,6 +72,28 @@
font-variant-alternates: historical-forms;
font-variant-east-asian: simplified;
}
#t17-container {
font-variant-ligatures: common-ligatures;
font-variant-position: super;
font-variant-caps: small-caps;
font-variant-numeric: lining-nums;
font-variant-alternates: historical-forms;
font-variant-east-asian: simplified;
}
#t17 {
font-variant: normal;
}
#t18-container {
font-variant-ligatures: common-ligatures;
font-variant-position: super;
font-variant-caps: small-caps;
font-variant-numeric: lining-nums;
font-variant-alternates: historical-forms;
font-variant-east-asian: simplified;
}
#t18 {
font-variant: none;
}
</style>
</head>
<body>
@@ -92,7 +114,9 @@
<div class="test" id="t14">Hello</div>
<div class="test" id="t15">Hello</div>
<div class="test" id="t16">Hello</div>
<div class="test" id="t17">Hello</div>
<div class="test" id="t17-container"><div id="t17">Hello</div></div>
<div class="test" id="t18-container"><div id="t18">Hello</div></div>
<div class="test" id="t19">Hello</div>
<script>
description("This test makes sure that the two shorthand properties which set font-variant-caps get resolved correctly.");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t1')).getPropertyValue('font-variant-caps')", "small-caps");
@@ -129,7 +153,9 @@

shouldBeEqualToString("window.getComputedStyle(document.getElementById('t16')).getPropertyValue('font-variant')", "common-ligatures super small-caps lining-nums historical-forms simplified");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t17')).getPropertyValue('font-variant')", "normal");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t17')).getPropertyValue('font-variant-caps')", "normal");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t18')).getPropertyValue('font-variant')", "none");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t19')).getPropertyValue('font-variant')", "normal");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t19')).getPropertyValue('font-variant-caps')", "normal");
</script>
<script src="../../resources/js-test-post.js"></script>
</body>
2 LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/inheritance-expected.txt
View file
@@ -19,7 +19,7 @@ PASS Property font-style inherits
FAIL Property font-synthesis has initial value weight style assert_equals: expected "weight style" but got "weight style small-caps"
PASS Property font-synthesis inherits
PASS Property font-variant has initial value normal
FAIL Property font-variant inherits assert_equals: expected "none" but got "no-common-ligatures no-discretionary-ligatures no-historical-ligatures no-contextual"
PASS Property font-variant inherits
PASS Property font-variant-alternates has initial value normal
PASS Property font-variant-alternates inherits
PASS Property font-variant-caps has initial value normal
192 Source/WebCore/css/CSSComputedStyleDeclaration.cpp
View file
@@ -2188,190 +2188,18 @@ static Ref<CSSFontStyleValue> fontStyleFromStyle(const RenderStyle& style)
return ComputedStyleExtractor::fontStyleFromStyleValue(style.fontDescription().italic(), style.fontDescription().fontStyleAxis());
}

static Ref<CSSValue> fontVariantFromStyle(const RenderStyle& style)
Ref<CSSValue> ComputedStyleExtractor::fontVariantShorthandValue()
{
if (style.fontDescription().variantSettings().isAllNormal())
return CSSValuePool::singleton().createIdentifierValue(CSSValueNormal);

auto list = CSSValueList::createSpaceSeparated();

switch (style.fontDescription().variantCommonLigatures()) {
case FontVariantLigatures::Normal:
break;
case FontVariantLigatures::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueCommonLigatures));
break;
case FontVariantLigatures::No:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueNoCommonLigatures));
break;
}

switch (style.fontDescription().variantDiscretionaryLigatures()) {
case FontVariantLigatures::Normal:
break;
case FontVariantLigatures::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueDiscretionaryLigatures));
break;
case FontVariantLigatures::No:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueNoDiscretionaryLigatures));
break;
}

switch (style.fontDescription().variantHistoricalLigatures()) {
case FontVariantLigatures::Normal:
break;
case FontVariantLigatures::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueHistoricalLigatures));
break;
case FontVariantLigatures::No:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueNoHistoricalLigatures));
break;
}

switch (style.fontDescription().variantContextualAlternates()) {
case FontVariantLigatures::Normal:
break;
case FontVariantLigatures::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueContextual));
break;
case FontVariantLigatures::No:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueNoContextual));
break;
}

switch (style.fontDescription().variantPosition()) {
case FontVariantPosition::Normal:
break;
case FontVariantPosition::Subscript:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueSub));
break;
case FontVariantPosition::Superscript:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueSuper));
break;
}

switch (style.fontDescription().variantCaps()) {
case FontVariantCaps::Normal:
break;
case FontVariantCaps::Small:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueSmallCaps));
break;
case FontVariantCaps::AllSmall:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueAllSmallCaps));
break;
case FontVariantCaps::Petite:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValuePetiteCaps));
break;
case FontVariantCaps::AllPetite:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueAllPetiteCaps));
break;
case FontVariantCaps::Unicase:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueUnicase));
break;
case FontVariantCaps::Titling:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueTitlingCaps));
break;
auto shorthand = fontVariantShorthand();
for (size_t i = 0; i < shorthand.length(); ++i) {
auto value = propertyValue(shorthand.properties()[i], DoNotUpdateLayout);
if (is<CSSPrimitiveValue>(value) && downcast<CSSPrimitiveValue>(*value).valueID() == CSSValueNormal)
continue;
list->append(value.releaseNonNull());
}

switch (style.fontDescription().variantNumericFigure()) {
case FontVariantNumericFigure::Normal:
break;
case FontVariantNumericFigure::LiningNumbers:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueLiningNums));
break;
case FontVariantNumericFigure::OldStyleNumbers:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueOldstyleNums));
break;
}

switch (style.fontDescription().variantNumericSpacing()) {
case FontVariantNumericSpacing::Normal:
break;
case FontVariantNumericSpacing::ProportionalNumbers:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueProportionalNums));
break;
case FontVariantNumericSpacing::TabularNumbers:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueTabularNums));
break;
}

switch (style.fontDescription().variantNumericFraction()) {
case FontVariantNumericFraction::Normal:
break;
case FontVariantNumericFraction::DiagonalFractions:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueDiagonalFractions));
break;
case FontVariantNumericFraction::StackedFractions:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueStackedFractions));
break;
}

switch (style.fontDescription().variantNumericOrdinal()) {
case FontVariantNumericOrdinal::Normal:
break;
case FontVariantNumericOrdinal::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueOrdinal));
break;
}

switch (style.fontDescription().variantNumericSlashedZero()) {
case FontVariantNumericSlashedZero::Normal:
break;
case FontVariantNumericSlashedZero::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueSlashedZero));
break;
}

switch (style.fontDescription().variantAlternates()) {
case FontVariantAlternates::Normal:
break;
case FontVariantAlternates::HistoricalForms:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueHistoricalForms));
break;
}

switch (style.fontDescription().variantEastAsianVariant()) {
case FontVariantEastAsianVariant::Normal:
break;
case FontVariantEastAsianVariant::Jis78:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueJis78));
break;
case FontVariantEastAsianVariant::Jis83:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueJis83));
break;
case FontVariantEastAsianVariant::Jis90:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueJis90));
break;
case FontVariantEastAsianVariant::Jis04:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueJis04));
break;
case FontVariantEastAsianVariant::Simplified:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueSimplified));
break;
case FontVariantEastAsianVariant::Traditional:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueTraditional));
break;
}

switch (style.fontDescription().variantEastAsianWidth()) {
case FontVariantEastAsianWidth::Normal:
break;
case FontVariantEastAsianWidth::Full:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueFullWidth));
break;
case FontVariantEastAsianWidth::Proportional:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueProportionalWidth));
break;
}

switch (style.fontDescription().variantEastAsianRuby()) {
case FontVariantEastAsianRuby::Normal:
break;
case FontVariantEastAsianRuby::Yes:
list->append(CSSValuePool::singleton().createIdentifierValue(CSSValueRuby));
break;
}

if (!list->length())
return CSSValuePool::singleton().createIdentifierValue(CSSValueNormal);
return list;
}

@@ -3279,7 +3107,7 @@ RefPtr<CSSValue> ComputedStyleExtractor::valueForPropertyInStyle(const RenderSty
case CSSPropertyFontStretch:
return fontStretchFromStyle(style);
case CSSPropertyFontVariant:
return fontVariantFromStyle(style);
return fontVariantShorthandValue();
case CSSPropertyFontWeight:
return fontNonKeywordWeightFromStyle(style);
case CSSPropertyFontPalette:
1 Source/WebCore/css/CSSComputedStyleDeclaration.h
View file
@@ -100,6 +100,7 @@ class ComputedStyleExtractor {
Ref<CSSValue> getBackgroundShorthandValue();
Ref<CSSValue> getMaskShorthandValue();
Ref<CSSValueList> getCSSPropertyValuesForGridShorthand(const StylePropertyShorthand&);
Ref<CSSValue> fontVariantShorthandValue();

RefPtr<Element> m_element;
PseudoId m_pseudoElementSpecifier;
2 Source/WebCore/css/parser/CSSPropertyParser.cpp
View file
@@ -5331,7 +5331,9 @@ bool CSSPropertyParser::consumeFontVariantShorthand(bool important)
{
if (identMatches<CSSValueNormal, CSSValueNone>(m_range.peek().id())) {
addProperty(CSSPropertyFontVariantLigatures, CSSPropertyFontVariant, consumeIdent(m_range).releaseNonNull(), important);
addProperty(CSSPropertyFontVariantNumeric, CSSPropertyFontVariant, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
addProperty(CSSPropertyFontVariantCaps, CSSPropertyFontVariant, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
addProperty(CSSPropertyFontVariantAlternates, CSSPropertyFontVariant, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
addProperty(CSSPropertyFontVariantEastAsian, CSSPropertyFontVariant, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
addProperty(CSSPropertyFontVariantPosition, CSSPropertyFontVariant, CSSValuePool::singleton().createIdentifierValue(CSSValueNormal), important, true);
return m_range.atEnd();

0 comments on commit d1256a0

Please sign in to comment.