Title: [224780] trunk
Revision
224780
Author
[email protected]
Date
2017-11-13 14:13:34 -0800 (Mon, 13 Nov 2017)

Log Message

The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed.
https://bugs.webkit.org/show_bug.cgi?id=174737

Reviewed by Antti Koivisto.

Source/WebCore:

Previously, the stroke width and stroke color would independently fall back to the -webkit-text-stroke-width and
-webkit-text-stroke-color values if stroke-width and/or stroke-color were not explicitly specified. This is
problematic, since we might end up mixing the new stroke properties with the legacy Webkit stroke properties.
The new strategy is to use the stroke-width and stroke-color value combination only if stroke-color has been
explicitly specified. This should work well, since there will be no visible stroke when specifying only
stroke-width (because stroke-color by default is transparent). We can then safely fall back to the legacy Webkit
stroke value combination. In the case where only stroke-color is specified, we should use the stroke-width/
stroke-color combination, since stroke-width by default is 1, and we will then have a visible stroke. 

No new tests, existing tests have been updated to reflect behavior changes.

* rendering/TextDecorationPainter.cpp:
(WebCore::decorationColor):
* rendering/TextPaintStyle.cpp:
(WebCore::computeTextPaintStyle):
(WebCore::computeTextSelectionPaintStyle):
* rendering/style/RenderStyle.cpp:
(WebCore::RenderStyle::computedStrokeWidth const):
(WebCore::RenderStyle::hasPositiveStrokeWidth const):
(WebCore::RenderStyle::computedStrokeColor const):
* rendering/style/RenderStyle.h:

LayoutTests:

* fast/css/stroke-color-expected.html:
* fast/css/stroke-color-fallback-expected.html:
* fast/css/stroke-color-fallback.html:
* fast/css/stroke-color.html:
* fast/css/stroke-width-expected.html:
* fast/css/stroke-width-fallback-expected.html:
* fast/css/stroke-width-fallback.html:
* fast/css/stroke-width.html:

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (224779 => 224780)


--- trunk/LayoutTests/ChangeLog	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/ChangeLog	2017-11-13 22:13:34 UTC (rev 224780)
@@ -1,3 +1,19 @@
+2017-11-13  Per Arne Vollan  <[email protected]>
+
+        The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed.
+        https://bugs.webkit.org/show_bug.cgi?id=174737
+
+        Reviewed by Antti Koivisto.
+
+        * fast/css/stroke-color-expected.html:
+        * fast/css/stroke-color-fallback-expected.html:
+        * fast/css/stroke-color-fallback.html:
+        * fast/css/stroke-color.html:
+        * fast/css/stroke-width-expected.html:
+        * fast/css/stroke-width-fallback-expected.html:
+        * fast/css/stroke-width-fallback.html:
+        * fast/css/stroke-width.html:
+
 2017-11-13  Commit Queue  <[email protected]>
 
         Unreviewed, rolling out r224763.

Modified: trunk/LayoutTests/fast/css/stroke-color-expected.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-color-expected.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-color-expected.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -8,6 +8,7 @@
             color: gray;
             text-shadow: none;
             -webkit-text-stroke-width: 10px;
+            display: inline;
         }
     </style>
 </head>

Modified: trunk/LayoutTests/fast/css/stroke-color-fallback-expected.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-color-fallback-expected.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-color-fallback-expected.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -7,7 +7,8 @@
             font-size: 80px;
             color: gray;
             text-shadow: none;
-            -webkit-text-stroke-width: 10px;
+            stroke-width: 10px;
+            display: inline;
         }
     
         stroke-color-class {

Modified: trunk/LayoutTests/fast/css/stroke-color-fallback.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-color-fallback.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-color-fallback.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -7,10 +7,12 @@
             font-size: 80px;
             color: gray;
             text-shadow: none;
+            stroke-width: 10px;
             -webkit-text-stroke-width: 10px;
+            display: inline;
         }
     
-        stroke-color-class {
+        div.stroke-color-class {
             stroke-color: green;
         }
     </style>

Modified: trunk/LayoutTests/fast/css/stroke-color.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-color.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-color.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -7,7 +7,8 @@
             font-size: 80px;
             color: gray;
             text-shadow: none;
-            -webkit-text-stroke-width: 10px;
+            stroke-width: 10px;
+            display: inline;
         }
     </style>
 </head>

Modified: trunk/LayoutTests/fast/css/stroke-width-expected.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-width-expected.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-width-expected.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -8,6 +8,7 @@
             color: gray;
             text-shadow: none;
             -webkit-text-stroke-color: blue;
+            display: inline;
         }
     </style>
 </head>

Modified: trunk/LayoutTests/fast/css/stroke-width-fallback-expected.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-width-fallback-expected.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-width-fallback-expected.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -7,25 +7,30 @@
             font-size: 80px;
             color: gray;
             text-shadow: none;
-            -webkit-text-stroke-color: blue;
+            display: inline
         }
     </style>
 </head>
 <body>
 
-<div style="stroke-width: 10px;">&#x25fc;</div>
-<div style="stroke-width: 25px;">&#x25fc;</div>
-<div style="stroke-width: 25px;">&#x25fc;</div>
-<div style="stroke-width: 30px;">&#x25fc;</div>
-<div style="stroke-width: 25px;">&#x25fc;</div>
+<div>&#x25fc;</div>
+<div style="-webkit-text-stroke-width: 25px; -webkit-text-stroke-color: green;">&#x25fc;</div>
+<div style="stroke-width: 1px; stroke-color: green;">&#x25fc;</div>
+<div style="stroke-width: 10px; stroke-color: blue;">&#x25fc;</div>
+<div style="stroke-width: 1px; stroke-color: green;">&#x25fc;</div>
+<div style="stroke-width: 10px; stroke-color: blue;">&#x25fc;</div>
+<div style="stroke-width: 25px; stroke-color: green;">&#x25fc;</div>
 
-<div style="stroke-width: 25px;"><span>&#x25fc;</span></div>
-<div style="stroke-width: 25px;"><span>&#x25fc;</span></div>
-<div style="stroke-width: 10px;"><span>&#x25fc;</span></div>
-<div style="stroke-width: 25px;"><span>&#x25fc;</span></div>
-<div style="stroke-width: 25px;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 10px; stroke-color: blue;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 25px; stroke-color: green;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 10px; stroke-color: blue;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 25px; stroke-color: green;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 10px; stroke-color: blue;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 1px; stroke-color: green;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 10px; stroke-color: blue;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 25px; stroke-color: green;"><span>&#x25fc;</span></div>
 
-<div style="stroke-width: 25px;">&#x25fc;</div>
+<div style="stroke-width: 10px; stroke-color: blue;">&#x25fc;</div>
 
 </body>
 </html>

Modified: trunk/LayoutTests/fast/css/stroke-width-fallback.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-width-fallback.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-width-fallback.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -8,9 +8,10 @@
             color: gray;
             text-shadow: none;
             -webkit-text-stroke-color: blue;
+            display: inline
         }
     
-        stroke-width-class {
+        div.stroke-width-class {
             stroke-width: 25px;
         }
     </style>
@@ -17,17 +18,22 @@
 </head>
 <body>
 
+<div style="stroke-width: 25px;">&#x25fc;</div>
+<div style="stroke-width: 25px; stroke-color: green;">&#x25fc;</div>
+<div style="stroke-color: green;">&#x25fc;</div>
 <div style="-webkit-text-stroke-width: 10px;">&#x25fc;</div>
+<div style="-webkit-text-stroke-width: 10px; stroke-color: green;">&#x25fc;</div>
 <div style="stroke-width: 25px; -webkit-text-stroke-width: 10px;">&#x25fc;</div>
-<div style="-webkit-text-stroke-width: 10px; stroke-width: 25px;">&#x25fc;</div>
-<div style="-webkit-text-stroke-width: 10px; stroke-width: 25px; stroke-width: 30px;">&#x25fc;</div>
-<div style="-webkit-text-stroke-width: 10px; stroke-width: 25px; -webkit-text-stroke-width: 30px;">&#x25fc;</div>
+<div style="stroke-width: 25px; -webkit-text-stroke-width: 10px; stroke-color: green;">&#x25fc;</div>
 
 <div style="stroke-width: 25px;"><span style="-webkit-text-stroke-width: 10px;">&#x25fc;</span></div>
+<div style="stroke-width: 25px; stroke-color: green;"><span style="-webkit-text-stroke-width: 10px;">&#x25fc;</span></div>
 <div style="-webkit-text-stroke-width: 10px;"><span style="stroke-width: 25px;">&#x25fc;</span></div>
+<div style="-webkit-text-stroke-width: 10px; stroke-color: green;"><span style="stroke-width: 25px;">&#x25fc;</span></div>
 <div style="-webkit-text-stroke-width: 10px;"><span>&#x25fc;</span></div>
+<div style="-webkit-text-stroke-width: 10px; stroke-color: green;"><span>&#x25fc;</span></div>
 <div style="stroke-width: 25px; -webkit-text-stroke-width: 10px;"><span>&#x25fc;</span></div>
-<div style="-webkit-text-stroke-width: 10px; stroke-width: 25px;"><span>&#x25fc;</span></div>
+<div style="stroke-width: 25px; -webkit-text-stroke-width: 10px; stroke-color: green;"><span>&#x25fc;</span></div>
 
 <div class="stroke-width-class" style="-webkit-text-stroke-width: 10px;">&#x25fc;</div>
 

Modified: trunk/LayoutTests/fast/css/stroke-width.html (224779 => 224780)


--- trunk/LayoutTests/fast/css/stroke-width.html	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/LayoutTests/fast/css/stroke-width.html	2017-11-13 22:13:34 UTC (rev 224780)
@@ -7,7 +7,8 @@
             font-size: 80px;
             color: gray;
             text-shadow: none;
-            -webkit-text-stroke-color: blue;
+            stroke-color: blue;
+            display: inline;
         }
     </style>
 </head>

Modified: trunk/Source/WebCore/ChangeLog (224779 => 224780)


--- trunk/Source/WebCore/ChangeLog	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/Source/WebCore/ChangeLog	2017-11-13 22:13:34 UTC (rev 224780)
@@ -1,3 +1,32 @@
+2017-11-13  Per Arne Vollan  <[email protected]>
+
+        The css properties stroke-width/stroke-color and -webkit-text-stroke-width/-webkit-text-stroke-color should not be mixed.
+        https://bugs.webkit.org/show_bug.cgi?id=174737
+
+        Reviewed by Antti Koivisto.
+
+        Previously, the stroke width and stroke color would independently fall back to the -webkit-text-stroke-width and
+        -webkit-text-stroke-color values if stroke-width and/or stroke-color were not explicitly specified. This is
+        problematic, since we might end up mixing the new stroke properties with the legacy Webkit stroke properties.
+        The new strategy is to use the stroke-width and stroke-color value combination only if stroke-color has been
+        explicitly specified. This should work well, since there will be no visible stroke when specifying only
+        stroke-width (because stroke-color by default is transparent). We can then safely fall back to the legacy Webkit
+        stroke value combination. In the case where only stroke-color is specified, we should use the stroke-width/
+        stroke-color combination, since stroke-width by default is 1, and we will then have a visible stroke. 
+
+        No new tests, existing tests have been updated to reflect behavior changes.
+
+        * rendering/TextDecorationPainter.cpp:
+        (WebCore::decorationColor):
+        * rendering/TextPaintStyle.cpp:
+        (WebCore::computeTextPaintStyle):
+        (WebCore::computeTextSelectionPaintStyle):
+        * rendering/style/RenderStyle.cpp:
+        (WebCore::RenderStyle::computedStrokeWidth const):
+        (WebCore::RenderStyle::hasPositiveStrokeWidth const):
+        (WebCore::RenderStyle::computedStrokeColor const):
+        * rendering/style/RenderStyle.h:
+
 2017-11-13  Gabriel Ivascu  <[email protected]>
 
         [GTK] Automatically adjust font size when gtk-xft-dpi changes

Modified: trunk/Source/WebCore/rendering/TextDecorationPainter.cpp (224779 => 224780)


--- trunk/Source/WebCore/rendering/TextDecorationPainter.cpp	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/Source/WebCore/rendering/TextDecorationPainter.cpp	2017-11-13 22:13:34 UTC (rev 224780)
@@ -358,7 +358,7 @@
         return result;
     if (style.hasPositiveStrokeWidth()) {
         // Prefer stroke color if possible but not if it's fully transparent.
-        result = style.visitedDependentColor(style.hasExplicitlySetStrokeColor() ? CSSPropertyStrokeColor :  CSSPropertyWebkitTextStrokeColor);
+        result = style.computedStrokeColor();
         if (result.isVisible())
             return result;
     }

Modified: trunk/Source/WebCore/rendering/TextPaintStyle.cpp (224779 => 224780)


--- trunk/Source/WebCore/rendering/TextPaintStyle.cpp	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/Source/WebCore/rendering/TextPaintStyle.cpp	2017-11-13 22:13:34 UTC (rev 224780)
@@ -107,7 +107,7 @@
     if (forceBackgroundToWhite)
         paintStyle.fillColor = adjustColorForVisibilityOnBackground(paintStyle.fillColor, Color::white);
 
-    paintStyle.strokeColor = lineStyle.visitedDependentColor(lineStyle.hasExplicitlySetStrokeColor() ? CSSPropertyStrokeColor : CSSPropertyWebkitTextStrokeColor);
+    paintStyle.strokeColor = lineStyle.computedStrokeColor();
 
     // Make the text stroke color legible against a white background
     if (forceBackgroundToWhite)
@@ -162,7 +162,7 @@
             selectionPaintStyle.strokeWidth = strokeWidth;
         }
 
-        Color stroke = paintInfo.forceTextColor() ? paintInfo.forcedTextColor() : pseudoStyle->visitedDependentColor(pseudoStyle->hasExplicitlySetStrokeColor() ? CSSPropertyStrokeColor : CSSPropertyWebkitTextStrokeColor);
+        Color stroke = paintInfo.forceTextColor() ? paintInfo.forcedTextColor() : pseudoStyle->computedStrokeColor();
         if (stroke != selectionPaintStyle.strokeColor) {
             if (!paintSelectedTextOnly)
                 paintSelectedTextSeparately = true;

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.cpp (224779 => 224780)


--- trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.cpp	2017-11-13 22:13:34 UTC (rev 224780)
@@ -2361,7 +2361,10 @@
 
 float RenderStyle::computedStrokeWidth(const IntSize& viewportSize) const
 {
-    if (!hasExplicitlySetStrokeWidth())
+    // Use the stroke-width and stroke-color value combination only if stroke-color has been explicitly specified.
+    // Since there will be no visible stroke when stroke-color is not specified (transparent by default), we fall
+    // back to the legacy Webkit text stroke combination in that case.
+    if (!hasExplicitlySetStrokeColor())
         return textStrokeWidth();
     
     const Length& length = strokeWidth();
@@ -2389,4 +2392,12 @@
     return strokeWidth().isPositive();
 }
 
+Color RenderStyle::computedStrokeColor() const
+{
+    CSSPropertyID propertyID = CSSPropertyStrokeColor;
+    if (!hasExplicitlySetStrokeColor())
+        propertyID = CSSPropertyWebkitTextStrokeColor;
+    return visitedDependentColor(propertyID);
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/rendering/style/RenderStyle.h (224779 => 224780)


--- trunk/Source/WebCore/rendering/style/RenderStyle.h	2017-11-13 22:05:27 UTC (rev 224779)
+++ trunk/Source/WebCore/rendering/style/RenderStyle.h	2017-11-13 22:13:34 UTC (rev 224780)
@@ -1275,7 +1275,8 @@
     void setHasExplicitlySetStrokeColor(bool v) { SET_VAR(m_rareInheritedData, hasSetStrokeColor, static_cast<unsigned>(v)); }
     bool hasExplicitlySetStrokeColor() const { return m_rareInheritedData->hasSetStrokeColor; };
     static Color initialStrokeColor() { return Color(Color::transparent); }
-    
+    Color computedStrokeColor() const;
+
     float strokeMiterLimit() const { return m_rareInheritedData->miterLimit; }
     void setStrokeMiterLimit(float f) { SET_VAR(m_rareInheritedData, miterLimit, f); }
     static float initialStrokeMiterLimit() { return defaultMiterLimit; }
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to