Highlight API: Don't paint unstyled custom highlights

The custom highlight API spec [1] currently states that an unstyled custom highlight should inherit the styles of its originating element. The Blink implementation follows this. However, this behavior can lead to some unintuitive results where unstyled highlights can affect the painting of the originating element, or paint over other highlights. This issue was pointed out in [2], and spec changes to fix this are pending editor review. A goal of these changes are that unstyled custom highlights do not affect rendering at all. This change updates the custom highlights implementation to achieve this by giving them a default 'transparent' foreground and background color. Some of our tests were validating the earlier unintuitive behavior involving overlapping ranges. These are updated, along with the addition of a new test to validate that a single unstyled highlight doesn't affect rendering. [1] https://drafts.csswg.org/css-highlight-api-1/#default-styles [2] w3c/csswg-drafts#6779 Bug: 1295271 Change-Id: Id7dc877c90c08240fbe79edeeeec551cdc5508c1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3449336 Reviewed-by: Delan Azabani <dazabani@igalia.com> Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Dan Clark <daniec@microsoft.com> Cr-Commit-Position: refs/heads/main@{#969007}
chromium · Feb 9, 2022 · 6ebd275 · 6ebd275
1 parent 3937dba
commit 6ebd275
Show file tree

Hide file tree

Showing 7 changed files with 55 additions and 24 deletions.
diff --git a/third_party/blink/renderer/core/paint/highlight_painting_utils.cc b/third_party/blink/renderer/core/paint/highlight_painting_utils.cc
@@ -92,10 +92,11 @@ Color HighlightThemeForegroundColor(const Document& document,
       return LayoutTheme::GetTheme().PlatformTextSearchColor(
           false /* active match */, style.UsedColorScheme());
     case kPseudoIdHighlight:
-      // TODO(ffiori): not assigning any visual effects to custom highlights by
-      // default as the spec doesn't define it. See
-      // https://github.com/w3c/csswg-drafts/issues/6375.
-      return style.VisitedDependentColor(color_property);
+      // TODO(crbug.com/1147859): Unstyled custom highlights should not be
+      // painted, so here we make the color default to transparent. When the
+      // highlight painting code is updated to match the spec, this should
+      // instead return the equivalent of 'currentColor'.
+      return Color::kTransparent;
     default:
       NOTREACHED();
       return Color();
@@ -115,10 +116,7 @@ Color HighlightThemeBackgroundColor(const Document& document,
     case kPseudoIdTargetText:
       return Color(shared_highlighting::kFragmentTextBackgroundColorARGB);
     case kPseudoIdHighlight:
-      // TODO(ffiori): not assigning any visual effects to custom highlights by
-      // default as the spec doesn't define it. See
-      // https://github.com/w3c/csswg-drafts/issues/6375.
-      return style.VisitedDependentColor(GetCSSPropertyBackgroundColor());
+      return Color::kTransparent;
     default:
       NOTREACHED();
       return Color();

diff --git a/...nal/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-003-ref.html b/...nal/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-003-ref.html
@@ -0,0 +1,10 @@
+<!DOCTYPE html>
+<meta charset="UTF-8">
+<style>
+  div {
+    color: #00000080;
+  }
+</style>
+<body>
+<div>foobar</div>
+</body>
diff --git a/...xternal/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-003.html b/...xternal/wpt/css/css-highlight-api/painting/custom-highlight-painting-inheritance-003.html
@@ -0,0 +1,23 @@
+<!DOCTYPE html>
+<meta charset="UTF-8">
+<title>CSS Highlight API Test: Inheritance</title>
+<link rel="help" href="https://drafts.csswg.org/css-highlight-api-1/">
+<link rel="match" href="custom-highlight-painting-inheritance-003-ref.html">
+<meta name="assert" value="Unstyled highlights should not be visible">
+<style>
+  div {
+    color: #00000080;
+    background-color: red;
+    width: 0;
+  }
+</style>
+<body>
+<div id="target">foobar</div>
+<script>
+  const node = document.getElementById("target");
+  let r = new Range();
+  r.setStart(node.firstChild, 0);
+  r.setEnd(node.firstChild, 3);
+  CSS.highlights.set("div-highlight", new Highlight(r));
+</script>
+</body>
diff --git a/.../css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-001-ref.html b/.../css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-001-ref.html
@@ -4,9 +4,13 @@
   body {
     font-size: 3em;
     font-weight: bolder;
+
+  }
+  #affected1 {
+    color: red;
   }
   #affected2 {
     color: green;
   }
 </style>
-<body><span>This should have default style (black).</span><span id="affected2">This should have 'highlight2' style (green).</span>
+<body><span id="affected1">This should have 'highlight1' style (red).</span><span id="affected2">This should have 'highlight2' style (green).</span>
diff --git a/.../css/css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-001.html b/.../css/css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-001.html
@@ -16,21 +16,19 @@
     color: green;
   }
 </style>
-<body><span id="affected1">This should have default style (black).</span><span id="affected2">This should have 'highlight2' style (green).</span>
+<body><span id="affected1">This should have 'highlight1' style (red).</span><span id="affected2">This should have 'highlight2' style (green).</span>
 <script>
   /*
     This test is meant to verify that:
       - UAs must not define any styles for custom highlight pseudo-elements in
       the default UA stylesheet.
-      - A custom highlight pseudo-element inherits the styles of its originating
-      element.
+      - UAs do not paint unstyled custom highlights.
 
     In this test, highlight2 has higher priority because it was registered
     later, so it is painted over highlight1. This includes painting for
-    span#affected1, where there is no CSS rule for highlight2. The result is
-    that highlight2 paints on top for span#affected1, using the originating
-    element's color. Thus it appears as if no highlight was painted at all for
-    span#affected1.
+    span#affected1, where there is no CSS rule for highlight2. But since unstyled
+    custom highlights are not painted, span#affected1 is still painted with the
+    styles for highlight1.
 
     See https://drafts.csswg.org/css-highlight-api-1/#default-styles
   */

diff --git a/.../css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-002-ref.html b/.../css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-002-ref.html
@@ -10,7 +10,8 @@
     background-color: rgb(0, 255, 0, 0.5);
   }
   #affected2 {
+    color: red;
     background-color: rgba(100, 0, 100, 0.5);
   }
 </style>
-<body><div><span id="affected1">Green on lime.</span><span id="affected2">Black on maroon.</span></div>
+<body><div><span id="affected1">Green on lime.</span><span id="affected2">Red on maroon.</span></div>
diff --git a/.../css/css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-002.html b/.../css/css-highlight-api/painting/custom-highlight-painting-overlapping-highlights-002.html
@@ -18,21 +18,18 @@
     background-color: rgba(100, 0, 100, 0.5);
   }
 </style>
-<body><div><span id="affected1">Green on lime.</span><span id="affected2">Black on maroon.</span></div>
+<body><div><span id="affected1">Green on lime.</span><span id="affected2">Red on maroon.</span></div>
 <script>
   /*
     This test is meant to verify that:
       - UAs must not define any styles for custom highlight pseudo-elements in
       the default UA stylesheet.
-      - A custom highlight pseudo-element inherits the styles of its originating
-      element.
+      - UAs do not paint unstyled custom highlights.
 
     In this test, highlight1 has higher priority, so it is painted over
     highlight2. This includes painting for span#affected2, where there is no CSS
-    rule for highlight1. The result is that highlight1 paints on top for
-    span#affected2, using the originating element's color (black) and background
-    color (transparent). Thus, span#affected2 gets text color black, and the
-    background remains the same color as that applied by highlight2 (maroon).
+    rule for highlight1. But since unstyled custom highlights are not painted,
+    span#affected2 is still painted with the styles for highlight2.
 
     See https://drafts.csswg.org/css-highlight-api-1/#default-styles
   */