Skip to content
Permalink
Browse files
Allow font-variant east-asian shorthand in any position
https://bugs.webkit.org/show_bug.cgi?id=245972

Reviewed by Darin Adler.

The property font-variant-east-asian can be used in any position
in the font-variant shorthand ; it should not check
that the range is finished.

https://drafts.csswg.org/css-fonts-4/#font-variant-prop

* LayoutTests/fast/text/font-variant-shorthand-expected.txt:
* LayoutTests/fast/text/font-variant-shorthand.html:

Add tests which fail before this change.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-east-asian-invalid.html:

Imported from WPT.

* LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-variant-valid-expected.txt:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::consumeFontVariantEastAsian):

Canonical link: https://commits.webkit.org/255134@main
  • Loading branch information
mdubet authored and nt1m committed Oct 4, 2022
1 parent db1f321 commit fd7b349b71efa48b759a7ceddc8ddc336874c895
Show file tree
Hide file tree
Showing 6 changed files with 101 additions and 57 deletions.
@@ -38,6 +38,10 @@ PASS window.getComputedStyle(document.getElementById('t17')).getPropertyValue('f
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 window.getComputedStyle(document.getElementById('t20')).getPropertyValue('font-variant') is "common-ligatures simplified"
PASS window.getComputedStyle(document.getElementById('t21')).getPropertyValue('font-variant-east-asian') is "normal"
PASS window.getComputedStyle(document.getElementById('t22')).getPropertyValue('font-variant-east-asian') is "normal"
PASS window.getComputedStyle(document.getElementById('t23')).getPropertyValue('font-variant-east-asian') is "normal"
PASS successfullyParsed is true

TEST COMPLETE
@@ -94,6 +94,18 @@
#t18 {
font-variant: none;
}
#t20 {
font-variant: simplified common-ligatures;
}
#t21 {
font-variant-east-asian: full-width garbage;
}
#t22 {
font-variant-east-asian: full-width none;
}
#t23 {
font-variant-east-asian: garbage full-width;
}
</style>
</head>
<body>
@@ -117,6 +129,10 @@
<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>
<div class="test" id="t20">Hello</div>
<div class="test" id="t21">Hello</div>
<div class="test" id="t22">Hello</div>
<div class="test" id="t23">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");
@@ -156,6 +172,10 @@
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");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t20')).getPropertyValue('font-variant')", "common-ligatures simplified");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t21')).getPropertyValue('font-variant-east-asian')", "normal");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t22')).getPropertyValue('font-variant-east-asian')", "normal");
shouldBeEqualToString("window.getComputedStyle(document.getElementById('t23')).getPropertyValue('font-variant-east-asian')", "normal");
</script>
<script src="../../resources/js-test-post.js"></script>
</body>
@@ -2,4 +2,10 @@
PASS e.style['font-variant-east-asian'] = "normal ruby" should not set the property value
PASS e.style['font-variant-east-asian'] = "jis78 jis83" should not set the property value
PASS e.style['font-variant-east-asian'] = "full-width proportional-width" should not set the property value
PASS e.style['font-variant-east-asian'] = "normal garbage" should not set the property value
PASS e.style['font-variant-east-asian'] = "normal none" should not set the property value
PASS e.style['font-variant-east-asian'] = "normal 30px" should not set the property value
PASS e.style['font-variant-east-asian'] = "full-width garbage" should not set the property value
PASS e.style['font-variant-east-asian'] = "full-width none" should not set the property value
PASS e.style['font-variant-east-asian'] = "full-width 30px" should not set the property value

@@ -16,6 +16,14 @@
test_invalid_value('font-variant-east-asian', 'jis78 jis83');

test_invalid_value('font-variant-east-asian', 'full-width proportional-width');

test_invalid_value('font-variant-east-asian', 'normal garbage');
test_invalid_value('font-variant-east-asian', 'normal none');
test_invalid_value('font-variant-east-asian', 'normal 30px');

test_invalid_value('font-variant-east-asian', 'full-width garbage');
test_invalid_value('font-variant-east-asian', 'full-width none');
test_invalid_value('font-variant-east-asian', 'full-width 30px');
</script>
</body>
</html>
@@ -44,5 +44,5 @@ PASS e.style['font-variant'] = "ruby" should set the property value
PASS e.style['font-variant'] = "sub" should set the property value
PASS e.style['font-variant'] = "super" should set the property value
FAIL e.style['font-variant'] = "common-ligatures discretionary-ligatures historical-ligatures contextual small-caps stylistic(flowing) lining-nums proportional-nums diagonal-fractions ordinal slashed-zero jis78 full-width ruby sub" should set the property value assert_not_equals: property should be set got disallowed value ""
FAIL e.style['font-variant'] = "super proportional-width jis83 stacked-fractions tabular-nums oldstyle-nums historical-forms all-small-caps no-contextual no-historical-ligatures no-discretionary-ligatures no-common-ligatures" should set the property value assert_not_equals: property should be set got disallowed value ""
FAIL e.style['font-variant'] = "super proportional-width jis83 stacked-fractions tabular-nums oldstyle-nums historical-forms all-small-caps no-contextual no-historical-ligatures no-discretionary-ligatures no-common-ligatures" should set the property value assert_equals: serialization should be canonical expected "no-common-ligatures no-discretionary-ligatures no-historical-ligatures no-contextual all-small-caps historical-forms oldstyle-nums tabular-nums stacked-fractions jis83 proportional-width super" but got "no-contextual no-historical-ligatures no-discretionary-ligatures no-common-ligatures historical-forms all-small-caps jis83 proportional-width stacked-fractions tabular-nums oldstyle-nums super"

@@ -702,70 +702,76 @@ static RefPtr<CSSValue> consumeFontVariantEastAsian(CSSParserTokenRange& range)
if (range.peek().id() == CSSValueNormal)
return consumeIdent(range);

RefPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
std::optional<FontVariantEastAsianVariant> variant;
std::optional<FontVariantEastAsianWidth> width;
std::optional<FontVariantEastAsianRuby> ruby;

while (!range.atEnd()) {
if (range.peek().type() != IdentToken)
return nullptr;
auto parseSomethingWithoutError = [&range, &variant, &width, &ruby] () {
bool hasParsedSomething = false;

auto id = range.peek().id();
while (true) {
if (range.peek().type() != IdentToken)
return hasParsedSomething;

switch (range.peek().id()) {
case CSSValueJis78:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Jis78;
break;
case CSSValueJis83:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Jis83;
break;
case CSSValueJis90:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Jis90;
break;
case CSSValueJis04:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Jis04;
break;
case CSSValueSimplified:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Simplified;
break;
case CSSValueTraditional:
if (variant)
return false;
variant = FontVariantEastAsianVariant::Traditional;
break;
case CSSValueFullWidth:
if (width)
return false;
width = FontVariantEastAsianWidth::Full;
break;
case CSSValueProportionalWidth:
if (width)
return false;
width = FontVariantEastAsianWidth::Proportional;
break;
case CSSValueRuby:
if (ruby)
return false;
ruby = FontVariantEastAsianRuby::Yes;
break;
default:
return hasParsedSomething;
}

switch (id) {
case CSSValueJis78:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Jis78;
break;
case CSSValueJis83:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Jis83;
break;
case CSSValueJis90:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Jis90;
break;
case CSSValueJis04:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Jis04;
break;
case CSSValueSimplified:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Simplified;
break;
case CSSValueTraditional:
if (variant)
return nullptr;
variant = FontVariantEastAsianVariant::Traditional;
break;
case CSSValueFullWidth:
if (width)
return nullptr;
width = FontVariantEastAsianWidth::Full;
break;
case CSSValueProportionalWidth:
if (width)
return nullptr;
width = FontVariantEastAsianWidth::Proportional;
break;
case CSSValueRuby:
if (ruby)
return nullptr;
ruby = FontVariantEastAsianRuby::Yes;
break;
default:
return nullptr;
range.consumeIncludingWhitespace();
hasParsedSomething = true;
}

range.consumeIncludingWhitespace();
}
};

if (!parseSomethingWithoutError())
return nullptr;

RefPtr<CSSValueList> values = CSSValueList::createSpaceSeparated();
switch (variant.value_or(FontVariantEastAsianVariant::Normal)) {
case FontVariantEastAsianVariant::Normal:
break;

0 comments on commit fd7b349

Please sign in to comment.