Allow custom properties in highlight pseudos

This is a workaround for the use case of custom properties defined and used in the same rule for ::selection pseudos. The use case arises from tooling, particularly Tailwind CSS, and it caused a stable regression when Highlight Inheritance was launched. This change restores the original behavior in that highlight pseudos can use custom properties defined in the highlight itself. The custom properties are not inherited through the highlight inheritance chain, so this change does not result in confusion about the source of custom properties when Highlight Inheritance is enabled: the properties still come from the originating element and then the highlight pseudo itself, never it's parent highlight. CSS Spec PR: w3c/csswg-drafts#11528 Fixed: 381125910 Change-Id: I0f89e6b8ad96d097ce1e2b39c179a270d472991f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6053860 Reviewed-by: Anders Hartvoll Ruud <[email protected]> Commit-Queue: Stephen Chenney <[email protected]> Cr-Commit-Position: refs/heads/main@{#1413271}
chromium · Jan 30, 2025 · d6f74a7 · d6f74a7
1 parent 76ec390
commit d6f74a7
Show file tree

Hide file tree

Showing 9 changed files with 63 additions and 38 deletions.
diff --git a/third_party/blink/renderer/core/css/properties/longhands/custom_property.cc b/third_party/blink/renderer/core/css/properties/longhands/custom_property.cc
@@ -122,15 +122,9 @@ void CustomProperty::ApplyValue(StyleResolverState& state,
   ComputedStyleBuilder& builder = state.StyleBuilder();
   DCHECK(!value.IsCSSWideKeyword());
 
-  // Highlight Pseudos do not allow custom property definitions.
-  // Properties are copied from the originating element when the
-  // style is created.
-  if (state.UsesHighlightPseudoInheritance()) {
-    if (builder.StyleType() == kPseudoIdSelection) {
-      UseCounter::Count(state.GetDocument(),
-                        WebFeature::kSelectionCustomProperty);
-    }
-    return;
+  if (builder.StyleType() == kPseudoIdSelection) {
+    UseCounter::Count(state.GetDocument(),
+                      WebFeature::kSelectionCustomProperty);
   }
 
   builder.SetHasVariableDeclaration();

diff --git a/third_party/blink/renderer/core/css/resolver/style_resolver.cc b/third_party/blink/renderer/core/css/resolver/style_resolver.cc
@@ -1402,10 +1402,10 @@ void StyleResolver::InitStyle(Element& element,
 
     // Highlight Pseudos may use var() references but those must be resolved
     // against the originating element. Share the variables from the originating
-    // style.
-    state.StyleBuilder().CopyInheritedVariablesFrom(
+    // style and remove any from the highlight chain.
+    state.StyleBuilder().SetInheritedVariablesFrom(
         state.OriginatingElementStyle());
-    state.StyleBuilder().CopyNonInheritedVariablesFrom(
+    state.StyleBuilder().SetNonInheritedVariablesFrom(
         state.OriginatingElementStyle());
   } else {
     state.CreateNewStyle(

diff --git a/third_party/blink/renderer/core/highlight/highlight_style_utils_test.cc b/third_party/blink/renderer/core/highlight/highlight_style_utils_test.cc
@@ -349,8 +349,8 @@ TEST_F(HighlightStyleUtilsTest, CustomPropertyInheritance) {
       :root {
         --root-color: green;
       }
-      ::selection {
-        /* This rule should not apply */
+      :root::selection {
+        /* Should not affect div::selection */
         --selection-color: blue;
       }
       div::selection {

diff --git a/third_party/blink/renderer/core/style/computed_style.cc b/third_party/blink/renderer/core/style/computed_style.cc
@@ -3040,21 +3040,17 @@ ComputedStyleBuilder::MutableNonInheritedVariables() {
   return *variables;
 }
 
-void ComputedStyleBuilder::CopyInheritedVariablesFrom(
+void ComputedStyleBuilder::SetInheritedVariablesFrom(
     const ComputedStyle* style) {
-  if (style->InheritedVariablesInternal()) {
-    has_own_inherited_variables_ = false;
-    MutableInheritedVariablesInternal() = style->InheritedVariablesInternal();
-  }
+  MutableInheritedVariablesInternal() = style->InheritedVariablesInternal();
+  has_own_inherited_variables_ = false;
 }
 
-void ComputedStyleBuilder::CopyNonInheritedVariablesFrom(
+void ComputedStyleBuilder::SetNonInheritedVariablesFrom(
     const ComputedStyle* style) {
-  if (style->NonInheritedVariablesInternal()) {
-    has_own_non_inherited_variables_ = false;
-    MutableNonInheritedVariablesInternal() =
-        style->NonInheritedVariablesInternal();
-  }
+  MutableNonInheritedVariablesInternal() =
+      style->NonInheritedVariablesInternal();
+  has_own_non_inherited_variables_ = false;
 }
 
 STATIC_ASSERT_ENUM(cc::OverscrollBehavior::Type::kAuto,

diff --git a/third_party/blink/renderer/core/style/computed_style.h b/third_party/blink/renderer/core/style/computed_style.h
@@ -3326,8 +3326,8 @@ class ComputedStyleBuilder final : public ComputedStyleBuilderBase {
                                    bool is_inherited_property) const;
   CORE_EXPORT StyleInheritedVariables& MutableInheritedVariables();
   CORE_EXPORT StyleNonInheritedVariables& MutableNonInheritedVariables();
-  void CopyInheritedVariablesFrom(const ComputedStyle*);
-  void CopyNonInheritedVariablesFrom(const ComputedStyle*);
+  void SetInheritedVariablesFrom(const ComputedStyle*);
+  void SetNonInheritedVariablesFrom(const ComputedStyle*);
   CORE_EXPORT void SetVariableData(const AtomicString& name,
                                    CSSVariableData* value,
                                    bool is_inherited_property) {

diff --git a/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-008.html b/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-008.html
@@ -11,7 +11,7 @@
     --background-color: green;
     --decoration-color: yellow;
   }
-  ::selection {
+  :root::selection {
     --background-color: cyan;
     --decoration-color: magenta;
   }
@@ -22,10 +22,10 @@
     text-decoration-color: var(--decoration-color, red);
   }
   span {
-    --background-color: blue;
+    --background-color: purple;
   }
   span::selection {
-    --background-color: purple;
+    --background-color: blue;
     background-color: var(--background-color, red);
   }
 </style>

diff --git a/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-009.html b/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-009.html
@@ -4,17 +4,17 @@
 <link rel="author" title="Stephen Chenney" href="mailto:[email protected]">
 <link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#highlight-cascade">
 <link rel="help" href="https://github.com/w3c/csswg-drafts/issues/6641">
-<meta name="assert" content="This test verifies that custom properties used in highlight pseudos are taken from the originating element.">
+<meta name="assert" content="This test verifies that custom properties used in highlight pseudos are taken from the highlight and originating element.">
 <script src="../support/selections.js"></script>
 <link rel="stylesheet" href="../support/highlights.css">
 <script src="/resources/testharness.js"></script>
 <script src="/resources/testharnessreport.js"></script>
 <style>
-  :root {
+  body {
     --background-color: green;
     --decoration-color: green;
   }
-  ::selection {
+  body::selection {
     --decoration-color: purple;
   }
   div::selection {
@@ -28,14 +28,17 @@
 <script>
     selectNodeContents(document.querySelector("body"));
 
+    const div_style = getComputedStyle(document.querySelector("div"));
     const body_selection = getComputedStyle(document.querySelector("body"), "::selection");
     const div_selection = getComputedStyle(document.querySelector("div"), "::selection");
     test(() => void assert_equals(body_selection.getPropertyValue("--background-color"), "green"),
         "body ::selection uses the originating custom property");
-    test(() => void assert_equals(body_selection.getPropertyValue("--decoration-color"), "green"),
+    test(() => void assert_equals(body_selection.getPropertyValue("--decoration-color"), "purple"),
         "body ::selection does not use its own custom property");
     test(() => void assert_equals(div_selection.getPropertyValue("--decoration-color"), "green"),
         "div::selection uses the originating element custom property");
-    test(() => void assert_equals(div_selection.getPropertyValue("--background-color"), "green"),
+    test(() => void assert_equals(div_selection.getPropertyValue("--background-color"), "blue"),
         "div::selection does not use its own custom property");
+    test(() => void assert_equals(div_style.getPropertyValue("--background-color"), "green"),
+        "div::selection properties are not present on the originating element");
 </script>
diff --git a/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-011.html b/.../blink/web_tests/external/wpt/css/css-pseudo/highlight-cascade/highlight-cascade-011.html
@@ -0,0 +1,31 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Pseudo-Elements Test: highlight cascade: inheritance of custom properties</title>
+<link rel="author" title="Stephen Chenney" href="mailto:[email protected]">
+<link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#highlight-cascade">
+<link rel="help" href="https://github.com/w3c/csswg-drafts/issues/6641">
+<meta name="assert" content="This test verifies that custom properties used in highlight pseudos are taken from the highlight and originating element.">
+<script src="../support/selections.js"></script>
+<link rel="stylesheet" href="../support/highlights.css">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<style>
+  :root::selection {
+    --background-color: red;
+  }
+  div::selection {
+    background-color: var(--background-color, green);
+  }
+</style>
+<body>
+  <div>Some text</div>
+</body>
+<script>
+    selectNodeContents(document.querySelector("body"));
+
+    const div_selection = getComputedStyle(document.querySelector("div"), "::selection");
+    test(() => void assert_equals(div_selection.backgroundColor, "rgb(0, 128, 0)"),
+        "div::selection does not inherit custom properties from the highlight parent");
+    test(() => void assert_equals(div_selection.getPropertyValue("--background-color"), ""),
+        "--background-color has no computed value on div::selection");
+</script>
diff --git a/third_party/blink/web_tests/external/wpt/css/css-pseudo/highlight-styling-001.html b/third_party/blink/web_tests/external/wpt/css/css-pseudo/highlight-styling-001.html
@@ -4,18 +4,19 @@
 <link rel="author" title="Delan Azabani" href="mailto:[email protected]">
 <link rel="help" href="https://drafts.csswg.org/css-pseudo-4/#highlight-styling">
 <link rel="match" href="highlight-styling-001-ref.html">
-<meta name="assert" value="This test verifies that ::selection styles cannot set custom properties.">
+<meta name="assert" value="This test verifies that ::selection styles can set custom properties and they over-ride the originating element.">
 <script src="support/selections.js"></script>
 <link rel="stylesheet" href="support/highlights.css">
 <style>
     main {
+        --x: red;
         font-size: 7em;
         margin: 0.5em;
     }
     main::selection {
-        --x: red;
+        --x: green;
         color: white;
-        background-color: var(--x, green);
+        background-color: var(--x, blue);
     }
 </style>
 <p>Test passes if the text below is white on green.