Skip to content

GitLab

T
tangled
Commit 9e45c65d authored by Anders Hartvoll Ruud's avatar Anders Hartvoll Ruud Committed by Commit Bot
Browse files
Options

[css-variables] Custom props with invalid var() should behave as 'unset' 

We currently have a (WPT-enforced) bug where custom properties that
reference guaranteed-invalid values [1] behave themselves as guaranteed-
invalid. This is not correct per spec, which requires the custom
property to behave as 'unset' in this case. This was clarified in [2],
although the spec has described this behavior long before that.

Unfortunately Firefox has the same issue. Safari on the other hand, does
implement it correctly.

[1] https://drafts.csswg.org/css-variables/#guaranteed-invalid
[2] https://github.com/w3c/csswg-drafts/issues/4075

Bug: 980930
Change-Id: I84a0da3aad6b72b574009d560eb868632769098a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2108026
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: default avatarXiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#751636}
parent 5c76ef40
Hide whitespace changes
Inline Side-by-side
Showing with 98 additions and 12 deletions
third_party/blink/renderer/core/css/resolver/style_cascade.cc
View file @ 9e45c65d
......@@ -486,15 +486,8 @@ const CSSValue* StyleCascade::ResolveCustomProperty(
if (resolver.InCycle())
return CSSInvalidVariableValue::Create();
if (!data) {
// TODO(crbug.com/980930): Treat custom properties as unset here,
// not invalid. This behavior is enforced by WPT, but violates the spec.
if (const auto* custom_property = DynamicTo<CustomProperty>(property)) {
if (!custom_property->IsRegistered())
return CSSInvalidVariableValue::Create();
}
if (!data)
return cssvalue::CSSUnsetValue::Create();
}
if (data == decl.Value())
return &decl;
......
third_party/blink/web_tests/external/wpt/css/css-env/env-in-custom-properties.tentative.html
View file @ 9e45c65d
......@@ -6,18 +6,30 @@
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
body {
#parent {
--var1: inherited;
}
#child {
--my-width: env(test, 100px);
width: var(--my-width);
--var1: env(nonexistent);
}
</style>
</head>
<body>
<div id="parent">
<div id="child"></div>
</div>
<script>
test(() => {
const style = window.getComputedStyle(document.body);
const style = window.getComputedStyle(child);
assert_equals(style.getPropertyValue("width"), "100px");
});
}, 'env() is substituted into a custom property');
test(() => {
const style = window.getComputedStyle(child);
assert_equals(style.getPropertyValue("--var1"), " inherited");
}, 'Substitution of unrecognized env() causes unset');
</script>
</body>
</html>
third_party/blink/web_tests/external/wpt/css/css-variables/variable-substitution-variable-declaration.html
View file @ 9e45c65d
......@@ -140,7 +140,7 @@
{ element: "target10", propertyName: "--varA", expectedPropertyValue: "" },
{ element: "target10", propertyName: "--varB", expectedPropertyValue: "" },
{ element: "target10", propertyName: "--varC", expectedPropertyValue: "" },
{ element: "target10", propertyName: "--varC", expectedPropertyValue: " another good one" },
];
testcases.forEach(function (testcase) {
......
third_party/blink/web_tests/external/wpt/css/css-variables/variables-substitute-guaranteed-invalid.html 0 → 100644
View file @ 9e45c65d
<!DOCTYPE html>
<title>Testing substitution of guaranteed-invalid values</title>
<link rel="help" href="https://drafts.csswg.org/css-variables/#guaranteed-invalid">
<link rel="help" href="https://drafts.csswg.org/css-variables/#cycles">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<style>
#target1 {
/* Cycle */
--var1: var(--var2);
--var2: var(--var1);
/* Reference to cycle */
--var3: var(--var1);
/* Reference to non-existent property */
--var4: var(--noexist);
}
#target1parent {
--var3: inherited;
--var4: inherited;
}
</style>
<div id="target1parent">
<div id="target1"></div>
</div>
<script>
test( function () {
let cs = getComputedStyle(target1);
assert_equals(cs.getPropertyValue('--var1'), '');
assert_equals(cs.getPropertyValue('--var2'), '');
}, 'Custom properties in a cycle are guaranteed-invalid');
test( function () {
let cs = getComputedStyle(target1);
assert_equals(cs.getPropertyValue('--var3'), ' inherited');
}, 'A custom property referencing a cycle is treated as unset');
test( function () {
let cs = getComputedStyle(target1);
assert_equals(cs.getPropertyValue('--var4'), ' inherited');
}, 'A custom property referencing a non-existent variable is treated as unset');
</script>
third_party/blink/web_tests/http/tests/devtools/elements/styles-3/styles-variables-expected.txt
View file @ 9e45c65d
......@@ -94,5 +94,26 @@ div { (user agent stylesheet)
body { (<style>)
/-- overloaded --/ --a: red;
value of --a: red
==== Computed style for ID5 ====
display: block;
block - div user agent stylesheet
[expanded]
element.style { ()
[expanded]
#id5 { (<style>)
--a: var(--b);
--b: var(--a);
[expanded]
div { (user agent stylesheet)
display: block;
======== Inherited from body ========
[expanded]
body { (<style>)
/-- overloaded --/ --a: red;
value of --a: undefined
third_party/blink/web_tests/http/tests/devtools/elements/styles-3/styles-variables.js
View file @ 9e45c65d
......@@ -28,6 +28,11 @@
--a: var(--z);
}
#id5 {
--a: var(--b);
--b: var(--a);
}
</style>
<div id="id1">
<div id="id2">
......@@ -37,6 +42,8 @@
</div>
<div id="id4">
</div>
<div id="id5">
</div>
`);
ElementsTestRunner.selectNodeAndWaitForStylesWithComputed('id1', step1);
......@@ -70,6 +77,15 @@
async function step4(node) {
TestRunner.addResult('==== Computed style for ID4 ====');
await ElementsTestRunner.dumpSelectedElementStyles(false, false);
TestRunner.cssModel.computedStylePromise(node.id).then(function(style) {
TestRunner.addResult('value of --a: ' + style.get('--a'));
ElementsTestRunner.selectNodeAndWaitForStylesWithComputed('id5', step5);
});
}
async function step5(node) {
TestRunner.addResult('==== Computed style for ID5 ====');
await ElementsTestRunner.dumpSelectedElementStyles(false, false);
TestRunner.cssModel.computedStylePromise(node.id).then(function(style) {
TestRunner.addResult('value of --a: ' + style.get('--a'));
TestRunner.completeTest();
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment