Fix highlight text decorations to work with all decoration types and colors
authormegan_gardner@apple.com <megan_gardner@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2020 18:40:24 +0000 (18:40 +0000)
committermegan_gardner@apple.com <megan_gardner@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 12 Feb 2020 18:40:24 +0000 (18:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207601

Reviewed by Dean Jackson.

Source/WebCore:

MarkedText styles were incorrectly setting styles, and colors were being
calculated incorrectly.

Extended http/wpt/css/css-highlight-api/highlight-text-decorations.html.

* rendering/InlineTextBox.cpp:
(WebCore::InlineTextBox::resolveStyleForMarkedText):
Correctly pass information about text decorations to MarkedTexts styles.

* rendering/TextDecorationPainter.cpp:
(WebCore::collectStylesForRenderer):
(WebCore::TextDecorationPainter::decorationColor):
(WebCore::decorationColor): Deleted.
Expose decorationColor calculator for use in InlineTextBox.
* rendering/TextDecorationPainter.h:

LayoutTests:

* http/wpt/css/css-highlight-api/highlight-text-decorations-expected.html:
* http/wpt/css/css-highlight-api/highlight-text-decorations.html:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256451 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/wpt/css/css-highlight-api/highlight-text-decorations-expected.html
LayoutTests/http/wpt/css/css-highlight-api/highlight-text-decorations.html
Source/WebCore/ChangeLog
Source/WebCore/rendering/InlineTextBox.cpp
Source/WebCore/rendering/TextDecorationPainter.cpp
Source/WebCore/rendering/TextDecorationPainter.h

index 1a8f3b1..31dfe3e 100644 (file)
@@ -1,3 +1,13 @@
+2020-02-12  Megan Gardner  <megan_gardner@apple.com>
+
+        Fix highlight text decorations to work with all decoration types and colors
+        https://bugs.webkit.org/show_bug.cgi?id=207601
+
+        Reviewed by Dean Jackson.
+
+        * http/wpt/css/css-highlight-api/highlight-text-decorations-expected.html:
+        * http/wpt/css/css-highlight-api/highlight-text-decorations.html:
+
 2020-02-12  Per Arne Vollan  <pvollan@apple.com>
 
         [iOS] Deny mach lookup access to view service in the WebContent process
index 22ed6c6..c32167a 100644 (file)
@@ -7,12 +7,24 @@
         }
         .style1 {
             text-decoration: underline;
+            text-decoration-color: red;
+            color: blue;
+        }
+        .style2 {
+            text-decoration: line-through;
+            text-decoration-color: violet;
+            text-decoration-style: double;
+        }
+        .style3 {
+            text-decoration: overline;
+            text-decoration-color: orange;
+            text-decoration-style: dotted;
         }
         /* FIXME: There is a discrepency for how the underlines are displayed at the end of the line, leading to a pixel different in this text. Find a real fix, but in the meantime, obscure the offending pixel <rdar://problem/59327965> https://bugs.webkit.org/show_bug.cgi?id=207512*/
         .obscurer1 {
           position: absolute;
-          top: 35px;
-          left: 100px;
+          top: 25px;
+          left: 95px;
           width: 10px;
           height: 10px;
           background: grey;
@@ -28,7 +40,7 @@
     </style>
 </head>
 <body>
-    O<span class="style1">n</span>e t<span class="style1">w</span>o th<span class="style1">ree</span>
+    O<span class="style1">n</span>e t<span class="style2">w</span>o th<span class="style3">ree</span>
     <div class='obscurer1'></div>
     <div class='obscurer2'></div>
 </body>
index b519a9f..244bfc9 100644 (file)
         }
         ::highlight(example-highlight1) {
             text-decoration: underline;
+            text-decoration-color: red;
+            color: blue;
+        }
+        ::highlight(example-highlight2) {
+            text-decoration: line-through;
+            text-decoration-color: violet;
+            text-decoration-style: double;
+        }
+        ::highlight(example-highlight3) {
+            text-decoration: overline;
+            text-decoration-color: orange;
+            text-decoration-style: dotted;
         }
         /* FIXME: There is a discrepency for how the underlines are displayed at the end of the line, leading to a pixel different in this text. Find a real fix, but in the meantime, obscure the offending pixel https://bugs.webkit.org/show_bug.cgi?id=207512 <rdar://problem/59327965> */
         .obscurer1 {
           position: absolute;
-          top: 35px;
-          left: 100px;
+          top: 25px;
+          left: 95px;
           width: 10px;
           height: 10px;
           background: grey;
     <span id="text1">One two three</span>
     <div class='obscurer1'></div>
     <div class='obscurer2'></div>
+
     <script>
         let textElement = document.getElementById('text1');
         let highlightRangeGroup1 = new HighlightRangeGroup(new StaticRange({startContainer: textElement.childNodes[0], startOffset: 1, endContainer: textElement.childNodes[0], endOffset: 2}));
-        highlightRangeGroup1.add(new StaticRange({startContainer: textElement.childNodes[0], startOffset: 5, endContainer: textElement.childNodes[0], endOffset: 6}));
-        highlightRangeGroup1.add(new StaticRange({startContainer: textElement.childNodes[0], startOffset: 10, endContainer: textElement.childNodes[0], endOffset: 13}));
+        let highlightRangeGroup2 = new HighlightRangeGroup(new StaticRange({startContainer: textElement.childNodes[0], startOffset: 5, endContainer: textElement.childNodes[0], endOffset: 6}));
+        let highlightRangeGroup3 = new HighlightRangeGroup(new StaticRange({startContainer: textElement.childNodes[0], startOffset: 10, endContainer: textElement.childNodes[0], endOffset: 13}));
 
         CSS.highlights.set("example-highlight1", highlightRangeGroup1);
+        CSS.highlights.set("example-highlight2", highlightRangeGroup2);
+        CSS.highlights.set("example-highlight3", highlightRangeGroup3);
     </script>
 </body>
 </html>
index 4532e0d..475f9b4 100644 (file)
@@ -1,3 +1,26 @@
+2020-02-12  Megan Gardner  <megan_gardner@apple.com>
+
+        Fix highlight text decorations to work with all decoration types and colors
+        https://bugs.webkit.org/show_bug.cgi?id=207601
+
+        Reviewed by Dean Jackson.
+
+        MarkedText styles were incorrectly setting styles, and colors were being 
+        calculated incorrectly.
+
+        Extended http/wpt/css/css-highlight-api/highlight-text-decorations.html.
+
+        * rendering/InlineTextBox.cpp:
+        (WebCore::InlineTextBox::resolveStyleForMarkedText):
+        Correctly pass information about text decorations to MarkedTexts styles.
+        
+        * rendering/TextDecorationPainter.cpp:
+        (WebCore::collectStylesForRenderer):
+        (WebCore::TextDecorationPainter::decorationColor):
+        (WebCore::decorationColor): Deleted.
+        Expose decorationColor calculator for use in InlineTextBox. 
+        * rendering/TextDecorationPainter.h:
+
 2020-02-12  Chris Dumez  <cdumez@apple.com>
 
         RELEASE_ASSERT() under WebSWClientConnection::didResolveRegistrationPromise()
index 7ff992d..407c819 100644 (file)
@@ -826,14 +826,22 @@ auto InlineTextBox::resolveStyleForMarkedText(const MarkedText& markedText, cons
             style.textStyles.fillColor = renderStyle->computedStrokeColor();
             style.textStyles.strokeColor = renderStyle->computedStrokeColor();
             
-            auto color = renderStyle->visitedDependentColorWithColorFilter(CSSPropertyWebkitTextFillColor);
+            auto color = TextDecorationPainter::decorationColor(*renderStyle.get());
             auto decorationStyle = renderStyle->textDecorationStyle();
             auto decorations = renderStyle->textDecorationsInEffect();
 
-            if (decorations.containsAny({ TextDecoration::Underline, TextDecoration::Overline, TextDecoration::LineThrough })) {
+            if (decorations.contains(TextDecoration::Underline)) {
                 style.textDecorationStyles.underlineColor = color;
                 style.textDecorationStyles.underlineStyle = decorationStyle;
             }
+            if (decorations.contains(TextDecoration::Overline)) {
+                style.textDecorationStyles.overlineColor = color;
+                style.textDecorationStyles.overlineStyle = decorationStyle;
+            }
+            if (decorations.contains(TextDecoration::LineThrough)) {
+                style.textDecorationStyles.linethroughColor = color;
+                style.textDecorationStyles.linethroughStyle = decorationStyle;
+            }
         }
         break;
     case MarkedText::DraggedContent:
index 369c63c..77870a3 100644 (file)
@@ -316,26 +316,10 @@ void TextDecorationPainter::paintTextDecoration(const TextRun& textRun, const Fl
         m_context.clearShadow();
 }
 
-static Color decorationColor(const RenderStyle& style)
-{
-    // Check for text decoration color first.
-    Color result = style.visitedDependentColorWithColorFilter(CSSPropertyTextDecorationColor);
-    if (result.isValid())
-        return result;
-    if (style.hasPositiveStrokeWidth()) {
-        // Prefer stroke color if possible but not if it's fully transparent.
-        result = style.computedStrokeColor();
-        if (result.isVisible())
-            return result;
-    }
-    
-    return style.visitedDependentColorWithColorFilter(CSSPropertyWebkitTextFillColor);
-}
-
 static void collectStylesForRenderer(TextDecorationPainter::Styles& result, const RenderObject& renderer, OptionSet<TextDecoration> remainingDecorations, bool firstLineStyle, PseudoId pseudoId)
 {
     auto extractDecorations = [&] (const RenderStyle& style, OptionSet<TextDecoration> decorations) {
-        auto color = decorationColor(style);
+        auto color = TextDecorationPainter::decorationColor(style);
         auto decorationStyle = style.textDecorationStyle();
 
         if (decorations.contains(TextDecoration::Underline)) {
@@ -387,6 +371,22 @@ static void collectStylesForRenderer(TextDecorationPainter::Styles& result, cons
         extractDecorations(styleForRenderer(*current), remainingDecorations);
 }
 
+Color TextDecorationPainter::decorationColor(const RenderStyle& style)
+{
+    // Check for text decoration color first.
+    auto result = style.visitedDependentColorWithColorFilter(CSSPropertyTextDecorationColor);
+    if (result.isValid())
+        return result;
+    if (style.hasPositiveStrokeWidth()) {
+        // Prefer stroke color if possible but not if it's fully transparent.
+        result = style.computedStrokeColor();
+        if (result.isVisible())
+            return result;
+    }
+    
+    return style.visitedDependentColorWithColorFilter(CSSPropertyWebkitTextFillColor);
+}
+
 OptionSet<TextDecoration> TextDecorationPainter::textDecorationsInEffectForStyle(const TextDecorationPainter::Styles& style)
 {
     OptionSet<TextDecoration> decorations;
index 2559022..224ed7c 100644 (file)
@@ -64,6 +64,7 @@ public:
         TextDecorationStyle overlineStyle;
         TextDecorationStyle linethroughStyle;
     };
+    static Color decorationColor(const RenderStyle&);
     static OptionSet<TextDecoration> textDecorationsInEffectForStyle(const Styles&);
     static Styles stylesForRenderer(const RenderObject&, OptionSet<TextDecoration> requestedDecorations, bool firstLineStyle = false, PseudoId = PseudoId::None);