Skip to content

Commit ef82a01

Browse files
vitorrorizvitorroriz
committed
Refactoring font-face src parsing
https://bugs.webkit.org/show_bug.cgi?id=250471 rdar://104135235 Reviewed by Myles C. Maxfield. Refactoring font-face src parsing such that we parse each component separated by commas, individually. If all components can't be parsed, then the src descriptor is invalidated. This was already done before, but now each component parser, such as consumeFontFaceSrcURI() and consumeFontFaceSrcLocal() will receive a subrange, and the whole subrange has to be parsed for the component to be valid. This will fix test failures we had when parsing format() in a URI component. The optional format() token can optionally follow a url() token. Before, if garbage was following the url() token, we would ignore it and still validate the component with an empty format. However, we would not consume the range being parsed, and if the next token was not a Comma, we would invalidate the whole descriptor. * LayoutTests/fast/css/font-face-src-parsing-expected.txt: Update on which src descriptors are valid. * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-format-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-list-expected.txt: Missing failures should pass now. * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-tech-expected.txt: We don't support tech() yet but we need to rebaseline here. Increment tests will pass now. * Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp: (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrcURI): (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrcLocal): Component is invalidated if the whole range is not consumed. (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrc): * LayoutTests/fast/css/font-face-src-parsing-expected.txt: * LayoutTests/fast/css/font-face-src-parsing.html: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-format-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-list-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-tech-expected.txt: * Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp: (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrcURI): (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrcLocal): (WebCore::CSSPropertyParserHelpersWorkerSafe::consumeFontFaceSrc): Canonical link: https://commits.webkit.org/258870@main
1 parent 4ecc989 commit ef82a01

File tree

6 files changed

+40
-29
lines changed

6 files changed

+40
-29
lines changed

‎LayoutTests/fast/css/font-face-src-parsing-expected.txt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,11 @@ Valid rules from the stylesheet:
1717
@font-face { src: url("font.ttf"); }
1818
@font-face { src: url("font.ttf"), local("font2"); }
1919
@font-face { src: local("font"); }
20+
@font-face { src: url("font.ttf"); }
21+
@font-face { src: url("foo.ttf"); }
2022
Invalid rules from the stylesheet:
2123

2224
@font-face { }
2325
@font-face { }
2426
@font-face { }
25-
@font-face { }
26-
@font-face { }
2727

‎LayoutTests/fast/css/font-face-src-parsing.html‎

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -45,23 +45,22 @@
4545
@font-face {
4646
src: ,local(font);
4747
}
48+
@font-face {
49+
src: url(font.ttf), invalid();
50+
}
51+
@font-face {
52+
src: url(foo.ttf), invalid;
53+
}
4854
</style>
4955

5056
<!-- Invalid src descriptor rules. -->
5157
<style>
5258
@font-face {
5359
src: url(font.ttf invalid);
5460
}
55-
@font-face {
56-
src: url(font.ttf), invalid();
57-
}
5861
@font-face {
5962
src: url(foo.ttf) invalid;
6063
}
61-
62-
@font-face {
63-
src: url(foo.ttf), invalid;
64-
}
6564
@font-face {
6665
src: url(foo.ttf) "invalid";
6766
}

‎LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-format-expected.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ PASS Check that src: url("foo.ttf") format("svg"), url("bar.html") is valid
2828
PASS Check that src: url("foo.ttf") format(svg), url("bar.html") is valid
2929
PASS Check that src: url("foo.ttf") format(xyzz 200px), url("bar.html") is valid
3030
PASS Check that src: url("foo.ttf") format(xyzz), url("bar.html") is valid
31-
FAIL Check that src: url("foo.ttf") dummy(xyzzy), url("bar.html") is valid assert_equals: expected true but got false
31+
PASS Check that src: url("foo.ttf") dummy(xyzzy), url("bar.html") is valid
3232
PASS Check that src: url("foo.ttf") format(), url("bar.html") is valid
3333
PASS Check that src: url("foo.ttf") format(none), url("bar.html") is valid
3434

‎LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-list-expected.txt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,13 @@ PASS Check that src: local(inherit), url(foo.ttf) is valid
33
PASS Check that src: local("myfont"), local(unset) is valid
44
PASS Check that src: local(), url(foo.ttf) is valid
55
PASS Check that src: local(12px monospace), url(foo.ttf) is valid
6-
FAIL Check that src: local("myfont") format(opentype), url(foo.ttf) is valid assert_equals: expected true but got false
7-
FAIL Check that src: url(not a valid url/bar.ttf), url(foo.ttf) is valid assert_equals: expected true but got false
6+
PASS Check that src: local("myfont") format(opentype), url(foo.ttf) is valid
7+
PASS Check that src: url(not a valid url/bar.ttf), url(foo.ttf) is valid
88
PASS Check that src: url(foo.ttf) format(bad), url(foo.ttf) is valid
9-
FAIL Check that src: url(foo.ttf) tech(unknown), url(foo.ttf) is valid assert_equals: expected true but got false
9+
PASS Check that src: url(foo.ttf) tech(unknown), url(foo.ttf) is valid
1010
PASS Check that src: url(foo.ttf), url(something.ttf) format(broken) is valid
1111
PASS Check that src: /* an empty component */, url(foo.ttf) is valid
12-
FAIL Check that src: local(""), url(foo.ttf), unparseable-garbage, local("another font name") is valid assert_equals: expected true but got false
12+
PASS Check that src: local(""), url(foo.ttf), unparseable-garbage, local("another font name") is valid
1313
PASS Check that src: local(), local(initial) is invalid
1414
PASS Check that src: local("textfont") format(opentype), local("emoji") tech(color-COLRv0) is invalid
1515
PASS Check that src: local(), /*empty*/, url(should be quoted.ttf), junk is invalid

‎LayoutTests/imported/w3c/web-platform-tests/css/css-fonts/parsing/font-face-src-tech-expected.txt‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -26,14 +26,14 @@ PASS Check that src: url("foo.ttf") tech(normal) is invalid
2626
PASS Check that src: url("foo.ttf") tech(xyzzy) is invalid
2727
FAIL Check that src: url("foo.ttf") format(opentype) tech(features-opentype) is valid assert_equals: expected true but got false
2828
PASS Check that src: url("foo.ttf") tech(features-opentype) format(opentype) is invalid
29-
FAIL Check that src: url("foo.ttf") tech(incremental), url("bar.html") is valid assert_equals: expected true but got false
30-
FAIL Check that src: url("foo.ttf") tech(incremental, color-SVG, features-graphite, features-aat), url("bar.html") is valid assert_equals: expected true but got false
31-
FAIL Check that src: url("foo.ttf") tech(color-SVG, features-graphite), url("bar.html") is valid assert_equals: expected true but got false
32-
FAIL Check that src: url("foo.ttf") tech(color-SVG), url("bar.html") is valid assert_equals: expected true but got false
33-
FAIL Check that src: url("foo.ttf") tech(features-graphite), url("bar.html") is valid assert_equals: expected true but got false
29+
PASS Check that src: url("foo.ttf") tech(incremental), url("bar.html") is valid
30+
PASS Check that src: url("foo.ttf") tech(incremental, color-SVG, features-graphite, features-aat), url("bar.html") is valid
31+
PASS Check that src: url("foo.ttf") tech(color-SVG, features-graphite), url("bar.html") is valid
32+
PASS Check that src: url("foo.ttf") tech(color-SVG), url("bar.html") is valid
33+
PASS Check that src: url("foo.ttf") tech(features-graphite), url("bar.html") is valid
3434
PASS Check that src: url("foo.ttf") dummy("opentype") tech(variations) is invalid
3535
PASS Check that src: url("foo.ttf") dummy("opentype") dummy(variations) is invalid
3636
PASS Check that src: url("foo.ttf") format(opentype) tech(features-opentype) dummy(something) is invalid
3737
FAIL Check that src: url("foo.ttf") format(dummy), url("foo.ttf") tech(variations) is valid assert_equals: expected true but got false
38-
FAIL Check that src: url("foo.ttf") tech(color), url("bar.html") is valid assert_equals: expected true but got false
38+
PASS Check that src: url("foo.ttf") tech(color), url("bar.html") is valid
3939

‎Source/WebCore/css/parser/CSSPropertyParserWorkerSafe.cpp‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -223,6 +223,8 @@ static RefPtr<CSSFontFaceSrcResourceValue> consumeFontFaceSrcURI(CSSParserTokenR
223223
return nullptr;
224224
format = arg.value().toString();
225225
}
226+
if (!range.atEnd())
227+
return nullptr;
226228

227229
return CSSFontFaceSrcResourceValue::create(WTFMove(location), WTFMove(format), context.isContentOpaque ? LoadedFromOpaqueSource::Yes : LoadedFromOpaqueSource::No);
228230
}
@@ -232,13 +234,13 @@ static RefPtr<CSSValue> consumeFontFaceSrcLocal(CSSParserTokenRange& range)
232234
CSSParserTokenRange args = CSSPropertyParserHelpers::consumeFunction(range);
233235
if (args.peek().type() == StringToken) {
234236
auto& arg = args.consumeIncludingWhitespace();
235-
if (!args.atEnd())
237+
if (!args.atEnd() || !range.atEnd())
236238
return nullptr;
237239
return CSSFontFaceSrcLocalValue::create(arg.value().toAtomString());
238240
}
239241
if (args.peek().type() == IdentToken) {
240242
AtomString familyName = CSSPropertyParserHelpers::concatenateFamilyName(args);
241-
if (familyName.isNull() || !args.atEnd())
243+
if (familyName.isNull() || !args.atEnd() || !range.atEnd())
242244
return nullptr;
243245
return CSSFontFaceSrcLocalValue::create(WTFMove(familyName));
244246
}
@@ -249,16 +251,26 @@ RefPtr<CSSValueList> consumeFontFaceSrc(CSSParserTokenRange& range, const CSSPar
249251
{
250252
RefPtr<CSSValueList> values = CSSValueList::createCommaSeparated();
251253

252-
do {
254+
auto consumeSrcListComponent = [&](CSSParserTokenRange& range) -> RefPtr<CSSValue> {
253255
const CSSParserToken& token = range.peek();
254-
RefPtr<CSSValue> parsedValue;
256+
if (token.type() == CSSParserTokenType::UrlToken || token.functionId() == CSSValueUrl)
257+
return consumeFontFaceSrcURI(range, context);
255258
if (token.functionId() == CSSValueLocal)
256-
parsedValue = consumeFontFaceSrcLocal(range);
257-
else
258-
parsedValue = consumeFontFaceSrcURI(range, context);
259-
if (parsedValue)
259+
return consumeFontFaceSrcLocal(range);
260+
return nullptr;
261+
};
262+
while (!range.atEnd()) {
263+
auto begin = range.begin();
264+
while (!range.atEnd() && range.peek().type() != CSSParserTokenType::CommaToken)
265+
range.consumeComponentValue();
266+
267+
auto subrange = range.makeSubRange(begin, &range.peek());
268+
if (auto parsedValue = consumeSrcListComponent(subrange))
260269
values->append(parsedValue.releaseNonNull());
261-
} while (CSSPropertyParserHelpers::consumeCommaIncludingWhitespace(range));
270+
271+
if (!range.atEnd())
272+
range.consumeIncludingWhitespace();
273+
}
262274
return values->size() ? values : nullptr;
263275
}
264276

0 commit comments

Comments
 (0)