CSS Custom Highlights text decoration does not respect priority

https://bugs.webkit.org/show_bug.cgi?id=259321 rdar://112494779 Reviewed by Wenson Hsieh. Before if StyledMarkedText was of higher priority, just overrides textDecorationStyles. Added a function to compute the textDecorationStyles. Added WPT ref test for painting unclashing lower priority text decoration over higher priority highlight without text decorations. * LayoutTests/imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001-expected.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001-ref.html: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001.html: Added. * Source/WebCore/rendering/StyledMarkedText.cpp: (WebCore::computeStylesForTextDecorations): (WebCore::coalesceAdjacentWithSameRanges): Canonical link: https://commits.webkit.org/266184@main
WebKit · Jul 20, 2023 · bf94cf5 · bf94cf5
1 parent 9d3aa20
commit bf94cf5
Show file tree

Hide file tree

Showing 4 changed files with 98 additions and 3 deletions.
diff --git a/...ghlight-api/painting/custom-highlight-painting-priority-text-decoration-001-expected.html b/...ghlight-api/painting/custom-highlight-painting-priority-text-decoration-001-expected.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Highlight API Test: ::highlight text-decoration paints over higher priority when not clashing</title>
+    <style>
+        #yellow-highlight {
+            background-color: yellow;
+            text-decoration: underline;
+            text-decoration-color: red;
+        }
+    </style>
+</head>
+<body>
+    <span id="yellow-highlight">Yellow</span>
+</body>
+</html>
diff --git a/...ss-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001-ref.html b/...ss-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001-ref.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Highlight API Test: ::highlight text-decoration paints over higher priority when not clashing</title>
+    <style>
+        #yellow-highlight {
+            background-color: yellow;
+            text-decoration: underline;
+            text-decoration-color: red;
+        }
+    </style>
+</head>
+<body>
+    <span id="yellow-highlight">Yellow</span>
+</body>
+</html>
diff --git a/...ss/css-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001.html b/...ss/css-highlight-api/painting/custom-highlight-painting-priority-text-decoration-001.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Highlight API Test: ::highlight text-decoration paints over higher priority when not clashing</title>
+    <link rel="help" href="https://www.w3.org/TR/css-highlight-api-1/#priorities">
+    <link rel="match" href="custom-highlight-painting-priority-text-decoration-001-ref.html">
+    <style>
+    ::highlight(yellow-highlight) {
+        background-color: yellow;
+    }
+    ::highlight(underline-highlight) {
+        text-decoration: underline;
+        text-decoration-color: red;
+    }
+    </style>
+</head>
+<body>
+    <span id="yellow-highlight">Yellow</span>
+
+    <script>
+        let yellow = document.getElementById("yellow-highlight");
+        let highlightYellow = new Highlight(new StaticRange({
+            startContainer: yellow.childNodes[0],
+            startOffset: 0,
+            endContainer: yellow.childNodes[0],
+            endOffset: 6
+        }));
+        let highlightUnderline = new Highlight(new StaticRange({
+            startContainer: yellow.childNodes[0],
+            startOffset: 0,
+            endContainer: yellow.childNodes[0],
+            endOffset: 6
+        }));
+        CSS.highlights.set("yellow-highlight", highlightYellow);
+        CSS.highlights.set("underline-highlight", highlightUnderline);
+        highlightYellow.priority = 10;
+    </script>
+</body>
+</html>
diff --git a/Source/WebCore/rendering/StyledMarkedText.cpp b/Source/WebCore/rendering/StyledMarkedText.cpp
@@ -122,6 +122,30 @@ StyledMarkedText::Style StyledMarkedText::computeStyleForUnmarkedMarkedText(cons
     return style;
 }
 
+static TextDecorationPainter::Styles computeStylesForTextDecorations(const TextDecorationPainter::Styles& previousTextDecorationStyles, const TextDecorationPainter::Styles& currentTextDecorationStyles)
+{
+    auto textDecorations = TextDecorationPainter::textDecorationsInEffectForStyle(currentTextDecorationStyles);
+
+    if (textDecorations.isEmpty())
+        return previousTextDecorationStyles;
+
+    auto textDecorationStyles = previousTextDecorationStyles;
+
+    if (textDecorations.contains(TextDecorationLine::Underline)) {
+        textDecorationStyles.underline.color = currentTextDecorationStyles.underline.color;
+        textDecorationStyles.underline.decorationStyle = currentTextDecorationStyles.underline.decorationStyle;
+    }
+    if (textDecorations.contains(TextDecorationLine::Overline)) {
+        textDecorationStyles.overline.color = currentTextDecorationStyles.overline.color;
+        textDecorationStyles.overline.decorationStyle = currentTextDecorationStyles.overline.decorationStyle;
+    }
+    if (textDecorations.contains(TextDecorationLine::LineThrough)) {
+        textDecorationStyles.linethrough.color = currentTextDecorationStyles.linethrough.color;
+        textDecorationStyles.linethrough.decorationStyle = currentTextDecorationStyles.linethrough.decorationStyle;
+    }
+    return textDecorationStyles;
+}
+
 static Vector<StyledMarkedText> coalesceAdjacentWithSameRanges(Vector<StyledMarkedText>&& styledTexts)
 {
     ASSERT(!styledTexts.isEmpty());
@@ -146,10 +170,10 @@ static Vector<StyledMarkedText> coalesceAdjacentWithSameRanges(Vector<StyledMark
 
             if (previousStyledMarkedText.priority <= it->priority) {
                 previousStyledMarkedText.priority = it->priority;
-                // If highlight, take textDecorationStyles of latest StyledMarkedText.
-                // FIXME: Check for taking textDecorationStyles needs to be changed to accommodate other MarkedText type.
+                // If highlight, combine textDecorationStyles accordingly.
+                // FIXME: Check for taking textDecorationStyles needs to accommodate other MarkedText type.
                 if (!it->highlightName.isNull())
-                    previousStyledMarkedText.style.textDecorationStyles = it->style.textDecorationStyles;
+                    previousStyledMarkedText.style.textDecorationStyles = computeStylesForTextDecorations(previousStyledMarkedText.style.textDecorationStyles, it->style.textDecorationStyles);
                 // If higher or same priority and opaque, override background color.
                 if (it->style.backgroundColor.isOpaque())
                     previousStyledMarkedText.style.backgroundColor = it->style.backgroundColor;