Skip to content

Commit

Permalink
[CSS] Invalid @Property rule should be ignored at parse-time
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=263007
rdar://116803886

Reviewed by Antti Koivisto and Tim Nguyen.

This patch moves most of the validity checks from rule-registration-time (during rule set building)
to parse-time, thus preventing invalid @Property rules to appear in the CSSOM.

* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-cssom-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-cssom.html:
* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-properties-values-api/at-property.html:
* Source/WebCore/css/parser/CSSParserImpl.cpp:
(WebCore::CSSParserImpl::consumePropertyRule):
* Source/WebCore/style/CustomPropertyRegistry.cpp:
(WebCore::Style::CustomPropertyRegistry::registerFromStylesheet):

Canonical link: https://commits.webkit.org/269466@main
  • Loading branch information
mdubet committed Oct 18, 2023
1 parent 9e25f51 commit 31ae975
Show file tree
Hide file tree
Showing 6 changed files with 226 additions and 122 deletions.
Original file line number Diff line number Diff line change
@@ -1,64 +1,41 @@

PASS Rule for --no-descriptors is invalid
PASS Rule for --no-syntax is invalid
PASS Rule for --no-inherits is invalid
PASS Rule for --no-initial-color-value is invalid
PASS Rule for --syntax-only is invalid
PASS Rule for --inherits-only is invalid
PASS Rule for --initial-value-only is invalid
PASS Rule for --valid has expected cssText
PASS Rule for --valid-reverse has expected cssText
PASS Rule for --valid-universal has expected cssText
PASS Rule for --valid-whitespace has expected cssText
PASS Rule for --vALId has expected cssText
PASS Rule for --no-descriptors has expected cssText
PASS Rule for --no-syntax has expected cssText
PASS Rule for --no-inherits has expected cssText
PASS Rule for --no-initial-value has expected cssText
PASS Rule for --syntax-only has expected cssText
PASS Rule for --inherits-only has expected cssText
PASS Rule for --initial-value-only has expected cssText
PASS Rule for --no-initial-universal-value has expected cssText
PASS Rule for --tab tab has expected cssText
PASS CSSRule.type returns 0
PASS Rule for --valid returns expected value for CSSPropertyRule.name
PASS Rule for --valid-reverse returns expected value for CSSPropertyRule.name
PASS Rule for --valid-universal returns expected value for CSSPropertyRule.name
PASS Rule for --valid-whitespace returns expected value for CSSPropertyRule.name
PASS Rule for --vALId returns expected value for CSSPropertyRule.name
PASS Rule for --no-descriptors returns expected value for CSSPropertyRule.name
PASS Rule for --no-syntax returns expected value for CSSPropertyRule.name
PASS Rule for --no-inherits returns expected value for CSSPropertyRule.name
PASS Rule for --no-initial-value returns expected value for CSSPropertyRule.name
PASS Rule for --syntax-only returns expected value for CSSPropertyRule.name
PASS Rule for --inherits-only returns expected value for CSSPropertyRule.name
PASS Rule for --initial-value-only returns expected value for CSSPropertyRule.name
PASS Rule for --no-initial-universal-value returns expected value for CSSPropertyRule.name
PASS Rule for --valid returns expected value for CSSPropertyRule.syntax
PASS Rule for --valid-reverse returns expected value for CSSPropertyRule.syntax
PASS Rule for --valid-universal returns expected value for CSSPropertyRule.syntax
PASS Rule for --valid-whitespace returns expected value for CSSPropertyRule.syntax
PASS Rule for --vALId returns expected value for CSSPropertyRule.syntax
PASS Rule for --no-descriptors returns expected value for CSSPropertyRule.syntax
PASS Rule for --no-syntax returns expected value for CSSPropertyRule.syntax
PASS Rule for --no-inherits returns expected value for CSSPropertyRule.syntax
PASS Rule for --no-initial-value returns expected value for CSSPropertyRule.syntax
PASS Rule for --syntax-only returns expected value for CSSPropertyRule.syntax
PASS Rule for --inherits-only returns expected value for CSSPropertyRule.syntax
PASS Rule for --initial-value-only returns expected value for CSSPropertyRule.syntax
PASS Rule for --no-initial-universal-value returns expected value for CSSPropertyRule.syntax
PASS Rule for --valid returns expected value for CSSPropertyRule.inherits
PASS Rule for --valid-reverse returns expected value for CSSPropertyRule.inherits
PASS Rule for --valid-universal returns expected value for CSSPropertyRule.inherits
PASS Rule for --valid-whitespace returns expected value for CSSPropertyRule.inherits
PASS Rule for --vALId returns expected value for CSSPropertyRule.inherits
PASS Rule for --no-descriptors returns expected value for CSSPropertyRule.inherits
PASS Rule for --no-syntax returns expected value for CSSPropertyRule.inherits
PASS Rule for --no-inherits returns expected value for CSSPropertyRule.inherits
PASS Rule for --no-initial-value returns expected value for CSSPropertyRule.inherits
PASS Rule for --syntax-only returns expected value for CSSPropertyRule.inherits
PASS Rule for --inherits-only returns expected value for CSSPropertyRule.inherits
PASS Rule for --initial-value-only returns expected value for CSSPropertyRule.inherits
PASS Rule for --no-initial-universal-value returns expected value for CSSPropertyRule.inherits
PASS Rule for --valid returns expected value for CSSPropertyRule.initialValue
PASS Rule for --valid-reverse returns expected value for CSSPropertyRule.initialValue
PASS Rule for --valid-universal returns expected value for CSSPropertyRule.initialValue
PASS Rule for --valid-whitespace returns expected value for CSSPropertyRule.initialValue
PASS Rule for --vALId returns expected value for CSSPropertyRule.initialValue
PASS Rule for --no-descriptors returns expected value for CSSPropertyRule.initialValue
PASS Rule for --no-syntax returns expected value for CSSPropertyRule.initialValue
PASS Rule for --no-inherits returns expected value for CSSPropertyRule.initialValue
PASS Rule for --no-initial-value returns expected value for CSSPropertyRule.initialValue
PASS Rule for --syntax-only returns expected value for CSSPropertyRule.initialValue
PASS Rule for --inherits-only returns expected value for CSSPropertyRule.initialValue
PASS Rule for --initial-value-only returns expected value for CSSPropertyRule.initialValue
PASS Rule for --no-initial-universal-value returns expected value for CSSPropertyRule.initialValue

Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
inherits: false;
}
@property --valid-whitespace {
syntax: " <color>+ ";
syntax: " <color># ";
inherits: false;
initial-value: red, blue;
}
Expand All @@ -38,10 +38,14 @@
syntax: "<color> | none";
initial-value: red;
}
@property --no-initial-value {
@property --no-initial-color-value {
syntax: "<color> | none";
inherits: false;
}
@property --no-initial-universal-value {
syntax: "*";
inherits: false;
}
@property --syntax-only {
syntax: "<color> | none";
}
Expand All @@ -52,7 +56,10 @@
initial-value: red;
}
/* U+0009 CHARACTER TABULATION */
@property --tab\9 tab { }
@property --tab\9 tab {
syntax: "*";
inherits: true;
}
</style>
<script>

Expand All @@ -66,6 +73,13 @@
return null;
}

function test_invalid(name) {
test(() => {
let rule = find_at_property_rule(name);
assert_true(!rule);
}, `Rule for ${name} is invalid`);
}

function test_css_text(name, expected) {
test(() => {
let rule = find_at_property_rule(name);
Expand Down Expand Up @@ -106,22 +120,26 @@
}, `Rule for ${name} returns expected value for CSSPropertyRule.initialValue`);
}

// Invalid @property rules.
test_invalid('--no-descriptors');
test_invalid('--no-syntax');
test_invalid('--no-inherits');
test_invalid('--no-initial-color-value');
test_invalid('--syntax-only', '@property --syntax-only { syntax: "<color> | none"; }');
test_invalid('--inherits-only', '@property --inherits-only { inherits: true; }');
test_invalid('--initial-value-only', '@property --initial-value-only { initial-value: red; }');

// CSSPropertyRule.cssText

test_css_text('--valid', '@property --valid { syntax: "<color> | none"; inherits: false; initial-value: red; }');
test_css_text('--valid-reverse', '@property --valid-reverse { syntax: "<length>"; inherits: true; initial-value: 0px; }');
test_css_text('--valid-universal', '@property --valid-universal { syntax: "*"; inherits: false; }');
test_css_text('--valid-whitespace', '@property --valid-whitespace { syntax: " <color>+ "; inherits: false; initial-value: red, blue; }');
test_css_text('--valid-whitespace', '@property --valid-whitespace { syntax: " <color># "; inherits: false; initial-value: red, blue; }');
test_css_text('--vALId', '@property --vALId { syntax: "<color> | none"; inherits: false; initial-value: red; }');

test_css_text('--no-descriptors', '@property --no-descriptors { }');
test_css_text('--no-syntax', '@property --no-syntax { inherits: false; initial-value: red; }');
test_css_text('--no-inherits', '@property --no-inherits { syntax: "<color> | none"; initial-value: red; }');
test_css_text('--no-initial-value', '@property --no-initial-value { syntax: "<color> | none"; inherits: false; }');
test_css_text('--syntax-only', '@property --syntax-only { syntax: "<color> | none"; }');
test_css_text('--inherits-only', '@property --inherits-only { inherits: true; }');
test_css_text('--initial-value-only', '@property --initial-value-only { initial-value: red; }');
test_css_text('--tab\ttab', '@property --tab\\9 tab { }');
test_css_text('--no-initial-universal-value', '@property --no-initial-universal-value { syntax: "*"; inherits: false; }');

test_css_text('--tab\ttab', '@property --tab\\9 tab { syntax: "*"; inherits: true; }');

// CSSRule.type

Expand All @@ -138,29 +156,17 @@
test_name('--valid-whitespace');
test_name('--vALId');

test_name('--no-descriptors');
test_name('--no-syntax');
test_name('--no-inherits');
test_name('--no-initial-value');
test_name('--syntax-only');
test_name('--inherits-only');
test_name('--initial-value-only');
test_name('--no-initial-universal-value');

// CSSPropertyRule.syntax

test_syntax('--valid', '<color> | none');
test_syntax('--valid-reverse', '<length>');
test_syntax('--valid-universal', '*');
test_syntax('--valid-whitespace', ' <color>+ ');
test_syntax('--valid-whitespace', ' <color># ');
test_syntax('--vALId', '<color> | none');

test_syntax('--no-descriptors', '');
test_syntax('--no-syntax', '');
test_syntax('--no-inherits', '<color> | none');
test_syntax('--no-initial-value', '<color> | none');
test_syntax('--syntax-only', '<color> | none');
test_syntax('--inherits-only', '');
test_syntax('--initial-value-only', '');
test_syntax('--no-initial-universal-value', '*');

// CSSPropertyRule.inherits

Expand All @@ -170,13 +176,7 @@
test_inherits('--valid-whitespace', false);
test_inherits('--vALId', false);

test_inherits('--no-descriptors', false);
test_inherits('--no-syntax', false);
test_inherits('--no-inherits', false);
test_inherits('--no-initial-value', false);
test_inherits('--syntax-only', false);
test_inherits('--inherits-only', true);
test_inherits('--initial-value-only', false);
test_inherits('--no-initial-universal-value', false);

// CSSPropertyRule.initialValue

Expand All @@ -186,12 +186,6 @@
test_initial_value('--valid-whitespace', 'red, blue');
test_initial_value('--vALId', 'red');

test_initial_value('--no-descriptors', null);
test_initial_value('--no-syntax', 'red');
test_initial_value('--no-inherits', 'red');
test_initial_value('--no-initial-value', null);
test_initial_value('--syntax-only', null);
test_initial_value('--inherits-only', null);
test_initial_value('--initial-value-only', 'red');
test_initial_value('--no-initial-universal-value', null);

</script>
Original file line number Diff line number Diff line change
Expand Up @@ -2,27 +2,49 @@
PASS Attribute 'syntax' returns expected value for ["<color>"]
PASS Attribute 'syntax' returns expected value for ["<color> | none"]
PASS Attribute 'syntax' returns expected value for ["<color># | <image> | none"]
PASS Attribute 'syntax' returns expected value for ["foo | <length>#"]
PASS Attribute 'syntax' returns expected value for ["foo | bar | baz"]
PASS Attribute 'syntax' returns expected value for ["*"]
PASS Attribute 'syntax' returns expected value for ["notasyntax"]
PASS Attribute 'syntax' returns expected value for [red]
PASS Attribute 'syntax' returns expected value for [rgb(255, 0, 0)]
PASS Attribute 'syntax' returns expected value for [<color>]
PASS Attribute 'syntax' returns expected value for [foo | bar]
PASS Attribute 'syntax' returns expected value for ["*"]
PASS Attribute 'syntax' returns expected value for [" * "]
PASS Attribute 'syntax' returns expected value for ["* "]
PASS Attribute 'syntax' returns expected value for [" * "]
PASS Attribute 'syntax' returns expected value for ["red"]
PASS Attribute 'syntax' makes the @property rule invalid for ["rgb(255, 0, 0)"]
PASS Attribute 'syntax' makes the @property rule invalid for [<color>]
PASS Attribute 'syntax' makes the @property rule invalid for [foo | bar]
PASS Attribute 'syntax' makes the @property rule invalid for ["default"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Default"]
PASS Attribute 'syntax' makes the @property rule invalid for ["initial"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Initial"]
PASS Attribute 'syntax' makes the @property rule invalid for ["inherit"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Inherit"]
PASS Attribute 'syntax' makes the @property rule invalid for ["unset"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Unset"]
PASS Attribute 'syntax' makes the @property rule invalid for ["revert"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Revert"]
PASS Attribute 'syntax' makes the @property rule invalid for ["revert-layer"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Revert-layer"]
PASS Attribute 'syntax' makes the @property rule invalid for ["foo bar"]
PASS Attribute 'syntax' makes the @property rule invalid for ["Foo <length>"]
PASS Attribute 'syntax' makes the @property rule invalid for ["foo, bar"]
PASS Attribute 'syntax' makes the @property rule invalid for ["<length> <percentage>"]
PASS Attribute 'syntax' makes the @property rule invalid for ["|<length>"]
PASS Attribute 'initial-value' returns expected value for [10px]
PASS Attribute 'initial-value' returns expected value for [rgb(1, 2, 3)]
PASS Attribute 'initial-value' returns expected value for [red]
PASS Attribute 'initial-value' returns expected value for [foo]
PASS Attribute 'initial-value' returns expected value for [if(){}]
PASS Attribute 'initial-value' returns expected value for [var(--x)]
PASS Attribute 'initial-value' makes the @property rule invalid for [3em]
FAIL Attribute 'initial-value' makes the @property rule invalid for [var(--x)] assert_true: expected true got false
PASS Attribute 'inherits' returns expected value for [true]
PASS Attribute 'inherits' returns expected value for [false]
PASS Attribute 'inherits' returns expected value for [none]
PASS Attribute 'inherits' returns expected value for [0]
PASS Attribute 'inherits' returns expected value for [1]
PASS Attribute 'inherits' returns expected value for ["true"]
PASS Attribute 'inherits' returns expected value for ["false"]
PASS Attribute 'inherits' returns expected value for [calc(0)]
PASS Attribute 'inherits' makes the @property rule invalid for [none]
PASS Attribute 'inherits' makes the @property rule invalid for [0]
PASS Attribute 'inherits' makes the @property rule invalid for [1]
PASS Attribute 'inherits' makes the @property rule invalid for ["true"]
PASS Attribute 'inherits' makes the @property rule invalid for ["false"]
PASS Attribute 'inherits' makes the @property rule invalid for [calc(0)]
PASS Invalid property name does not parse [foo]
PASS Invalid property name does not parse [-foo]
PASS Rule applied [*, if(){}, false]
Expand Down
Loading

0 comments on commit 31ae975

Please sign in to comment.