Title: [265198] trunk
Revision
265198
Author
an...@apple.com
Date
2020-08-03 09:18:21 -0700 (Mon, 03 Aug 2020)

Log Message

REGRESSION(r259585) Text decoration color with value currentColor miscomputed in some cases
https://bugs.webkit.org/show_bug.cgi?id=215079

Reviewed by Zalan Bujtas.

Source/WebCore:

r259585 did some refactoring that broke a special case where text decoration color comes from
'-webkit-text-fill-color' property.

Test: fast/text/text-decoration-currentcolor-fill-color.html

* rendering/TextDecorationPainter.cpp:
(WebCore::TextDecorationPainter::decorationColor):

Move resolving currentColor to RenderStyle.

* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::colorResolvingCurrentColor const):

Handle CSSPropertyTextDecorationColor as a special case here.

(WebCore::RenderStyle::visitedDependentColor const):

LayoutTests:

* fast/text/text-decoration-currentcolor-fill-color-expected.html: Added.
* fast/text/text-decoration-currentcolor-fill-color.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (265197 => 265198)


--- trunk/LayoutTests/ChangeLog	2020-08-03 16:03:10 UTC (rev 265197)
+++ trunk/LayoutTests/ChangeLog	2020-08-03 16:18:21 UTC (rev 265198)
@@ -1,3 +1,13 @@
+2020-08-03  Antti Koivisto  <an...@apple.com>
+
+        REGRESSION(r259585) Text decoration color with value currentColor miscomputed in some cases
+        https://bugs.webkit.org/show_bug.cgi?id=215079
+
+        Reviewed by Zalan Bujtas.
+
+        * fast/text/text-decoration-currentcolor-fill-color-expected.html: Added.
+        * fast/text/text-decoration-currentcolor-fill-color.html: Added.
+
 2020-08-03  Hector Lopez  <hector_i_lo...@apple.com>
 
         [ macOS wk2 Release ] media/video-background-tab-playback.html is a flaky failure

Added: trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color-expected.html (0 => 265198)


--- trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color-expected.html	2020-08-03 16:18:21 UTC (rev 265198)
@@ -0,0 +1,11 @@
+<style>
+div {
+  background-color: red;
+  -webkit-background-clip: text;
+  -webkit-text-fill-color: transparent;
+}
+a {
+  text-decoration: none;
+}
+</style>
+<div><a href="" if no underline</a></div>

Added: trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color.html (0 => 265198)


--- trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/text-decoration-currentcolor-fill-color.html	2020-08-03 16:18:21 UTC (rev 265198)
@@ -0,0 +1,8 @@
+<style>
+div {
+  background-color: red;
+  -webkit-background-clip: text;
+  -webkit-text-fill-color: transparent;
+}
+</style>
+<div><a href="" if no underline</a></div>

Modified: trunk/Source/WebCore/ChangeLog (265197 => 265198)


--- trunk/Source/WebCore/ChangeLog	2020-08-03 16:03:10 UTC (rev 265197)
+++ trunk/Source/WebCore/ChangeLog	2020-08-03 16:18:21 UTC (rev 265198)
@@ -1,3 +1,27 @@
+2020-08-03  Antti Koivisto  <an...@apple.com>
+
+        REGRESSION(r259585) Text decoration color with value currentColor miscomputed in some cases
+        https://bugs.webkit.org/show_bug.cgi?id=215079
+
+        Reviewed by Zalan Bujtas.
+
+        r259585 did some refactoring that broke a special case where text decoration color comes from
+        '-webkit-text-fill-color' property.
+
+        Test: fast/text/text-decoration-currentcolor-fill-color.html
+
+        * rendering/TextDecorationPainter.cpp:
+        (WebCore::TextDecorationPainter::decorationColor):
+
+        Move resolving currentColor to RenderStyle.
+
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::colorResolvingCurrentColor const):
+
+        Handle CSSPropertyTextDecorationColor as a special case here.
+
+        (WebCore::RenderStyle::visitedDependentColor const):
+
 2020-08-03  Clark Wang  <clark_w...@apple.com>
 
         Added Constructor to AnalyserNode

Modified: trunk/Source/WebCore/rendering/TextDecorationPainter.cpp (265197 => 265198)


--- trunk/Source/WebCore/rendering/TextDecorationPainter.cpp	2020-08-03 16:03:10 UTC (rev 265197)
+++ trunk/Source/WebCore/rendering/TextDecorationPainter.cpp	2020-08-03 16:18:21 UTC (rev 265198)
@@ -373,18 +373,7 @@
 
 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);
+    return style.visitedDependentColorWithColorFilter(CSSPropertyTextDecorationColor);
 }
 
 OptionSet<TextDecoration> TextDecorationPainter::textDecorationsInEffectForStyle(const TextDecorationPainter::Styles& style)

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (265197 => 265198)


--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2020-08-03 16:03:10 UTC (rev 265197)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2020-08-03 16:18:21 UTC (rev 265198)
@@ -2024,6 +2024,17 @@
     auto result = unresolvedColorForProperty(colorProperty, visitedLink);
 
     if (isCurrentColor(result)) {
+        if (colorProperty == CSSPropertyTextDecorationColor) {
+            if (hasPositiveStrokeWidth()) {
+                // Prefer stroke color if possible but not if it's fully transparent.
+                auto strokeColor = colorResolvingCurrentColor(effectiveStrokeColorProperty(), visitedLink);
+                if (strokeColor.isVisible())
+                    return strokeColor;
+            }
+
+            return colorResolvingCurrentColor(CSSPropertyWebkitTextFillColor, visitedLink);
+        }
+
         auto borderStyle = computeBorderStyle();
         if (!visitedLink && (borderStyle == BorderStyle::Inset || borderStyle == BorderStyle::Outset || borderStyle == BorderStyle::Ridge || borderStyle == BorderStyle::Groove))
             return SRGBA<uint8_t> { 238, 238, 238 };
@@ -2050,10 +2061,6 @@
 
     Color visitedColor = colorResolvingCurrentColor(colorProperty, true);
 
-    // Text decoration color validity is preserved (checked in RenderObject::decorationColor).
-    if (colorProperty == CSSPropertyTextDecorationColor)
-        return visitedColor;
-
     // FIXME: Technically someone could explicitly specify the color transparent, but for now we'll just
     // assume that if the background color is transparent that it wasn't set. Note that it's weird that
     // we're returning unvisited info for a visited link, but given our restriction that the alpha values
@@ -2560,10 +2567,7 @@
 
 Color RenderStyle::computedStrokeColor() const
 {
-    CSSPropertyID propertyID = CSSPropertyStrokeColor;
-    if (!hasExplicitlySetStrokeColor())
-        propertyID = CSSPropertyWebkitTextStrokeColor;
-    return visitedDependentColor(propertyID);
+    return visitedDependentColor(effectiveStrokeColorProperty());
 }
 
 } // namespace WebCore

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (265197 => 265198)


--- trunk/Source/WebCore/rendering/style/RenderStyle.h	2020-08-03 16:03:10 UTC (rev 265197)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h	2020-08-03 16:18:21 UTC (rev 265198)
@@ -1322,6 +1322,7 @@
     bool hasExplicitlySetStrokeColor() const { return m_rareInheritedData->hasSetStrokeColor; };
     static Color initialStrokeColor() { return Color::transparentBlack; }
     Color computedStrokeColor() const;
+    CSSPropertyID effectiveStrokeColorProperty() const { return hasExplicitlySetStrokeColor() ? CSSPropertyStrokeColor : CSSPropertyWebkitTextStrokeColor; }
 
     float strokeMiterLimit() const { return m_rareInheritedData->miterLimit; }
     void setStrokeMiterLimit(float f) { SET_VAR(m_rareInheritedData, miterLimit, f); }
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to