Skip to content
Permalink
Browse files
REGRESSION (248115@main): list-style shorthand property doesn't wor…
…k with `inside none` but with `none inside`

https://bugs.webkit.org/show_bug.cgi?id=243991
<rdar://98748112>

Reviewed by Cameron McCormack.

none can be either list-style-type or list-style-image, and longhands can be in any order. We need to distinguish them when parsing.

Also, this fixes the bug where `none garbage` was valid, when it is actually invalid.

* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-type-computed-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-type-computed.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-type-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-type-valid.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-valid-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-lists/parsing/list-style-valid.html:
* Source/WebCore/css/parser/CSSPropertyParser.cpp:
(WebCore::CSSPropertyParser::consumeListStyleShorthand):
(WebCore::CSSPropertyParser::parseShorthand):
* Source/WebCore/css/parser/CSSPropertyParser.h:

Canonical link: https://commits.webkit.org/254282@main
  • Loading branch information
nt1m committed Sep 8, 2022
1 parent 291e6f5 commit bf6ad141e2f3b51243013f01f5d4c30ddaeda907
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 8 deletions.
@@ -3,6 +3,8 @@ PASS Property list-style-type value 'none'
PASS Property list-style-type value 'disc'
PASS Property list-style-type value 'circle'
PASS Property list-style-type value 'square'
PASS Property list-style-type value 'disclosure-open'
PASS Property list-style-type value 'disclosure-closed'
PASS Property list-style-type value 'decimal'
PASS Property list-style-type value 'decimal-leading-zero'
PASS Property list-style-type value 'lower-roman'
@@ -17,6 +17,8 @@
test_computed_value('list-style-type', 'disc');
test_computed_value('list-style-type', 'circle');
test_computed_value('list-style-type', 'square');
test_computed_value('list-style-type', 'disclosure-open');
test_computed_value('list-style-type', 'disclosure-closed');
test_computed_value('list-style-type', 'decimal');
test_computed_value('list-style-type', 'decimal-leading-zero');
test_computed_value('list-style-type', 'lower-roman');
@@ -3,6 +3,8 @@ PASS e.style['list-style-type'] = "none" should set the property value
PASS e.style['list-style-type'] = "disc" should set the property value
PASS e.style['list-style-type'] = "circle" should set the property value
PASS e.style['list-style-type'] = "square" should set the property value
PASS e.style['list-style-type'] = "disclosure-open" should set the property value
PASS e.style['list-style-type'] = "disclosure-closed" should set the property value
PASS e.style['list-style-type'] = "decimal" should set the property value
PASS e.style['list-style-type'] = "decimal-leading-zero" should set the property value
PASS e.style['list-style-type'] = "lower-roman" should set the property value
@@ -17,6 +17,8 @@
test_valid_value('list-style-type', 'disc');
test_valid_value('list-style-type', 'circle');
test_valid_value('list-style-type', 'square');
test_valid_value('list-style-type', 'disclosure-open');
test_valid_value('list-style-type', 'disclosure-closed');
test_valid_value('list-style-type', 'decimal');
test_valid_value('list-style-type', 'decimal-leading-zero');
test_valid_value('list-style-type', 'lower-roman');
@@ -1,7 +1,19 @@

PASS e.style['list-style'] = "none" should set the property value
FAIL e.style['list-style'] = "disc outside none" should set the property value assert_equals: serialization should be canonical expected "outside" but got "outside none disc"
PASS e.style['list-style'] = "inside" should set the property value
FAIL e.style['list-style'] = "inside disc" should set the property value assert_equals: serialization should be canonical expected "inside" but got "inside disc"
PASS e.style['list-style'] = "inside none" should set the property value
PASS e.style['list-style'] = "inside none none" should set the property value
PASS e.style['list-style'] = "none inside none" should set the property value
PASS e.style['list-style'] = "none none inside" should set the property value
PASS e.style['list-style'] = "none inside" should set the property value
PASS e.style['list-style'] = "url(\"https://example.com/\")" should set the property value
PASS e.style['list-style'] = "none url(\"https://example.com/\")" should set the property value
FAIL e.style['list-style'] = "url(\"https://example.com/\") disc" should set the property value assert_equals: serialization should be canonical expected "url(\"https://example.com/\")" but got "url(\"https://example.com/\") disc"
FAIL e.style['list-style'] = "url(\"https://example.com/\") disc outside" should set the property value assert_equals: serialization should be canonical expected "url(\"https://example.com/\")" but got "outside url(\"https://example.com/\") disc"
PASS e.style['list-style'] = "square" should set the property value
PASS e.style['list-style'] = "square url(\"https://example.com/\") inside" should set the property value
PASS e.style['list-style'] = "square linear-gradient(red,blue) inside" should set the property value
FAIL e.style['list-style'] = "disc radial-gradient(circle, #006, #00a 90%, #0000af 100%,white 100%) inside" should set the property value assert_equals: serialization should be canonical expected "inside radial-gradient(circle, rgb(0, 0, 102), rgb(0, 0, 170) 90%, rgb(0, 0, 175) 100%, white 100%)" but got "inside radial-gradient(circle, rgb(0, 0, 102), rgb(0, 0, 170) 90%, rgb(0, 0, 175) 100%, white 100%) disc"

@@ -12,12 +12,24 @@
<body>
<script>
test_valid_value('list-style', 'none');
test_valid_value('list-style', 'disc outside none', 'outside');

test_valid_value('list-style', 'inside');
test_valid_value('list-style', 'inside disc', 'inside');
test_valid_value('list-style', 'inside none');
test_valid_value('list-style', 'inside none none', 'inside none');
test_valid_value('list-style', 'none inside none', 'inside none');
test_valid_value('list-style', 'none none inside', 'inside none');
test_valid_value('list-style', 'none inside', 'inside none');
test_valid_value('list-style', 'url("https://example.com/")');
test_valid_value('list-style', 'none url("https://example.com/")', 'url("https://example.com/") none');
test_valid_value('list-style', 'url("https://example.com/") disc', 'url("https://example.com/")');
test_valid_value('list-style', 'url("https://example.com/") disc outside', 'url("https://example.com/")');
test_valid_value('list-style', 'square');

test_valid_value('list-style', 'square url("https://example.com/") inside', 'inside url("https://example.com/") square');
test_valid_value('list-style', 'square linear-gradient(red,blue) inside', 'inside linear-gradient(red, blue) square');
test_valid_value('list-style', 'disc radial-gradient(circle, #006, #00a 90%, #0000af 100%,white 100%) inside', 'inside radial-gradient(circle, rgb(0, 0, 102), rgb(0, 0, 170) 90%, rgb(0, 0, 175) 100%, white 100%)');
</script>
</body>
</html>
@@ -6364,6 +6364,61 @@ bool CSSPropertyParser::consumeOffset(bool important)
return m_range.atEnd();
}

bool CSSPropertyParser::consumeListStyleShorthand(bool important)
{
auto& valuePool = CSSValuePool::singleton();
RefPtr<CSSValue> parsedPosition;
RefPtr<CSSValue> parsedImage;
RefPtr<CSSValue> parsedType;
unsigned noneCount = 0;

while (!m_range.atEnd()) {
if (m_range.peek().id() == CSSValueNone) {
++noneCount;
consumeIdent(m_range);
continue;
}
if (!parsedPosition) {
if (auto position = parseSingleValue(CSSPropertyListStylePosition, CSSPropertyListStyle)) {
parsedPosition = position;
continue;
}
}
if (!parsedImage) {
if (auto image = parseSingleValue(CSSPropertyListStyleImage, CSSPropertyListStyle)) {
parsedImage = image;
continue;
}
}
if (!parsedType) {
if (auto type = parseSingleValue(CSSPropertyListStyleType, CSSPropertyListStyle)) {
parsedType = type;
continue;
}
}
return false;
}

if (noneCount > (!parsedImage + !parsedType))
return false;

// Use the implicit initial value for list-style-image, to serialize to "none" instead of "none none".
if (noneCount == 2) {
parsedImage = valuePool.createImplicitInitialValue();
parsedType = valuePool.createIdentifierValue(CSSValueNone);
} else if (noneCount == 1) {
if (!parsedImage)
parsedImage = parsedType ? valuePool.createIdentifierValue(CSSValueNone) : valuePool.createImplicitInitialValue();
if (!parsedType)
parsedType = valuePool.createIdentifierValue(CSSValueNone);
}

addPropertyWithImplicitDefault(CSSPropertyListStylePosition, CSSPropertyListStyle, WTFMove(parsedPosition), valuePool.createImplicitInitialValue(), important);
addPropertyWithImplicitDefault(CSSPropertyListStyleImage, CSSPropertyListStyle, WTFMove(parsedImage), valuePool.createImplicitInitialValue(), important);
addPropertyWithImplicitDefault(CSSPropertyListStyleType, CSSPropertyListStyle, WTFMove(parsedType), valuePool.createImplicitInitialValue(), important);
return m_range.atEnd();
}

bool CSSPropertyParser::parseShorthand(CSSPropertyID property, bool important)
{
switch (property) {
@@ -6497,14 +6552,7 @@ bool CSSPropertyParser::parseShorthand(CSSPropertyID property, bool important)
case CSSPropertyColumnRule:
return consumeShorthandGreedily(columnRuleShorthand(), important);
case CSSPropertyListStyle:
if (m_range.peek().id() == CSSValueNone) {
auto& valuePool = CSSValuePool::singleton();
addProperty(CSSPropertyListStylePosition, CSSPropertyListStyle, valuePool.createImplicitInitialValue(), important);
addProperty(CSSPropertyListStyleImage, CSSPropertyListStyle, valuePool.createImplicitInitialValue(), important);
addProperty(CSSPropertyListStyleType, CSSPropertyListStyle, valuePool.createIdentifierValue(CSSValueNone), important);
return true;
}
return consumeShorthandGreedily(listStyleShorthand(), important);
return consumeListStyleShorthand(important);
case CSSPropertyBorderRadius:
case CSSPropertyWebkitBorderRadius: {
RefPtr<CSSPrimitiveValue> horizontalRadii[4];
@@ -123,6 +123,7 @@ class CSSPropertyParser {
bool consumePerspectiveOrigin(bool important);
bool consumePrefixedPerspective(bool important);
bool consumeOffset(bool important);
bool consumeListStyleShorthand(bool important);

bool consumeOverscrollBehaviorShorthand(bool important);

0 comments on commit bf6ad14

Please sign in to comment.