Semantic colors should not be transformed by color-filter
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Jun 2018 20:52:49 +0000 (20:52 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 15 Jun 2018 20:52:49 +0000 (20:52 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186566
<rdar://problem/40705739>

Reviewed by Simon Fraser.

Source/WebCore:

Test: css3/color-filters/color-filter-ignore-semantic.html

* platform/graphics/Color.h:
(WebCore::Color::Color):
(WebCore::Color::isSemantic const):
(WebCore::Color::setIsSemantic):

Add a bit to Color to indicate it originated from a semantic color name.
Note that a color compares unequal to the semantic version of the same color.

* platform/graphics/filters/FilterOperations.cpp:
(WebCore::FilterOperations::transformColor const):
* platform/graphics/mac/ColorMac.h:
* platform/graphics/mac/ColorMac.mm:
(WebCore::semanticColorFromNSColor):
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::systemColor const):

Set the bit for semantic system colors.

* rendering/RenderTreeAsText.cpp:
(WebCore::RenderTreeAsText::writeRenderObject):

Fix up the output to avoid unneccary render tree dump changes.

LayoutTests:

* css3/color-filters/color-filter-ignore-semantic-expected.html: Added.
* css3/color-filters/color-filter-ignore-semantic.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/css3/color-filters/color-filter-ignore-semantic-expected.html [new file with mode: 0644]
LayoutTests/css3/color-filters/color-filter-ignore-semantic.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/Color.h
Source/WebCore/platform/graphics/filters/FilterOperations.cpp
Source/WebCore/platform/graphics/mac/ColorMac.h
Source/WebCore/platform/graphics/mac/ColorMac.mm
Source/WebCore/rendering/RenderThemeMac.mm
Source/WebCore/rendering/RenderTreeAsText.cpp

index 4e48e1eddf0b117f23b8316bb27d53e17e066f2a..90c556b25848895a83d2c7a800849fe848f034eb 100644 (file)
@@ -1,3 +1,14 @@
+2018-06-15  Antti Koivisto  <antti@apple.com>
+
+        Semantic colors should not be transformed by color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=186566
+        <rdar://problem/40705739>
+
+        Reviewed by Simon Fraser.
+
+        * css3/color-filters/color-filter-ignore-semantic-expected.html: Added.
+        * css3/color-filters/color-filter-ignore-semantic.html: Added.
+
 2018-06-15  Carlos Alberto Lopez Perez  <clopez@igalia.com>
 
         [GTK] Mark tests that are failing on the EWS test queue (v3)
diff --git a/LayoutTests/css3/color-filters/color-filter-ignore-semantic-expected.html b/LayoutTests/css3/color-filters/color-filter-ignore-semantic-expected.html
new file mode 100644 (file)
index 0000000..6ac2916
--- /dev/null
@@ -0,0 +1,36 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<html>
+    <head>
+        <title>CSS Test: Don't apply -apple-color-filter to semantic colors</title>
+        <link rel="author" title="Apple" href="http://www.apple.com/">
+        <link rel="match" href="-apple-color-filter-ignore-semantic-expected.html">
+
+        <style type="text/css">
+            .test
+            {
+                width: 100px;
+                height: 100px;
+                margin: 10px;
+                float: left;
+            }
+        </style>
+    </head>
+    <body>
+        <div class="test" style="background-color: rgb(0, 255, 255);"></div>
+        <div class="test" style="background-color: rgb(0, 255, 255);"></div>
+        <div class="test" style="background-color: text"></div>
+        <div class="test" style="background-color: highlight"></div>
+        <div class="test" style="background-color: window"></div>
+        <div class="test" style="background-color: -apple-system-text-background"></div>
+        <div class="test" style="background-color: -apple-system-header-text"></div>
+        <div class="test" style="background-color: -apple-system-alternate-selected"></div>
+        <div class="test" style="background-color: -apple-system-label"></div>
+        <div class="test" style="background-color: -apple-system-secondary-label"></div>
+        <div class="test" style="background-color: -apple-system-tertiary-label"></div>
+        <div class="test" style="background-color: -apple-system-quaternary-label"></div>
+        <div class="test" style="background-color: -apple-system-grid"></div>
+        <div class="test" style="background-color: -apple-system-blue-color"></div>
+        <div class="test" style="background-color: -apple-system-even-alternating-content-background"></div>
+        <div class="test" style="background-color: -apple-system-odd-alternating-content-background"></div>
+    </body>
+</html>
diff --git a/LayoutTests/css3/color-filters/color-filter-ignore-semantic.html b/LayoutTests/css3/color-filters/color-filter-ignore-semantic.html
new file mode 100644 (file)
index 0000000..fef11f9
--- /dev/null
@@ -0,0 +1,39 @@
+<!DOCTYPE html><!-- webkit-test-runner [ enableColorFilter=true ] -->
+<html>
+    <head>
+        <title>CSS Test: Don't apply -apple-color-filter to semantic colors</title>
+        <link rel="author" title="Apple" href="http://www.apple.com/">
+        <link rel="match" href="-apple-color-filter-ignore-semantic-expected.html">
+
+        <style type="text/css">
+            .test
+            {
+                width: 100px;
+                height: 100px;
+                margin: 10px;
+                float: left;
+                -apple-color-filter: invert();
+            }
+        </style>
+    </head>
+    <body>
+        <!--Filtered-->
+        <div class="test" style="background-color: rgb(255, 0, 0)"></div>
+        <div class="test" style="background-color: red"></div>
+        <!--Not filtered-->
+        <div class="test" style="background-color: text"></div>
+        <div class="test" style="background-color: highlight"></div>
+        <div class="test" style="background-color: window"></div>
+        <div class="test" style="background-color: -apple-system-text-background"></div>
+        <div class="test" style="background-color: -apple-system-header-text"></div>
+        <div class="test" style="background-color: -apple-system-alternate-selected"></div>
+        <div class="test" style="background-color: -apple-system-label"></div>
+        <div class="test" style="background-color: -apple-system-secondary-label"></div>
+        <div class="test" style="background-color: -apple-system-tertiary-label"></div>
+        <div class="test" style="background-color: -apple-system-quaternary-label"></div>
+        <div class="test" style="background-color: -apple-system-grid"></div>
+        <div class="test" style="background-color: -apple-system-blue-color"></div>
+        <div class="test" style="background-color: -apple-system-even-alternating-content-background"></div>
+        <div class="test" style="background-color: -apple-system-odd-alternating-content-background"></div>
+    </body>
+</html>
index 55d39f13272f54c7bbdeac98477e430c725713f0..5e0577b6c4b974f56e6b6b296c9cc9fb25861f39 100644 (file)
@@ -1049,6 +1049,9 @@ fast/css/caret-color.html [ Skip ]
 fast/history/visited-link-caret-color.html [ Skip ]
 css3/color-filters/color-filter-caret-color.html [ Skip ]
 
+# Not relevant for iOS.
+css3/color-filters/color-filter-ignore-semantic.html [ Skip ]
+
 # Run webrtc tests on iOS 11
 imported/w3c/web-platform-tests/webrtc [ Pass ]
 fast/events/constructors/media-stream-event-constructor.html [ Pass ]
index 29a1f763f5d598f5b13fbbcf373374458962f356..b9c20361430f958503a674054b00deaf89a31569 100644 (file)
@@ -1,3 +1,36 @@
+2018-06-15  Antti Koivisto  <antti@apple.com>
+
+        Semantic colors should not be transformed by color-filter
+        https://bugs.webkit.org/show_bug.cgi?id=186566
+        <rdar://problem/40705739>
+
+        Reviewed by Simon Fraser.
+
+        Test: css3/color-filters/color-filter-ignore-semantic.html
+
+        * platform/graphics/Color.h:
+        (WebCore::Color::Color):
+        (WebCore::Color::isSemantic const):
+        (WebCore::Color::setIsSemantic):
+
+        Add a bit to Color to indicate it originated from a semantic color name.
+        Note that a color compares unequal to the semantic version of the same color.
+
+        * platform/graphics/filters/FilterOperations.cpp:
+        (WebCore::FilterOperations::transformColor const):
+        * platform/graphics/mac/ColorMac.h:
+        * platform/graphics/mac/ColorMac.mm:
+        (WebCore::semanticColorFromNSColor):
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::systemColor const):
+
+        Set the bit for semantic system colors.
+
+        * rendering/RenderTreeAsText.cpp:
+        (WebCore::RenderTreeAsText::writeRenderObject):
+
+        Fix up the output to avoid unneccary render tree dump changes.
+
 2018-06-15  Chris Dumez  <cdumez@apple.com>
 
         Add API test coverage for SW RegistrationDatabase destruction and fix issues found by the test
index 72fae22a586a27185b7b2af4e1f249494031bba8..66609cc2d40cf2803ee54d72457b4be8f393469d 100644 (file)
@@ -119,6 +119,14 @@ public:
             setRGB(color);
     }
 
+    enum SemanticTag { Semantic };
+
+    Color(RGBA32 color, SemanticTag)
+    {
+        setRGB(color);
+        setIsSemantic();
+    }
+
     Color(int r, int g, int b)
     {
         setRGB(r, g, b);
@@ -240,6 +248,9 @@ public:
     Color colorWithAlpha(float) const;
     Color opaqueColor() const { return colorWithAlpha(1.0f); }
 
+    // True if the color originated from a CSS semantic color name.
+    bool isSemantic() const { return !isExtended() && (m_colorData.rgbaAndFlags & isSemanticRBGAColorBit); }
+
 #if PLATFORM(GTK)
     Color(const GdkColor&);
     // We can't sensibly go back to GdkColor without losing the alpha value
@@ -296,6 +307,7 @@ public:
 private:
     void setRGB(int r, int g, int b) { setRGB(makeRGB(r, g, b)); }
     void setRGB(RGBA32);
+    void setIsSemantic() { m_colorData.rgbaAndFlags |= isSemanticRBGAColorBit; }
 
     // 0x_______00 is an ExtendedColor pointer.
     // 0x_______01 is an invalid RGBA32.
@@ -304,6 +316,7 @@ private:
     static const uint64_t invalidRGBAColor = 0x1;
     static const uint64_t validRGBAColorBit = 0x2;
     static const uint64_t validRGBAColor = 0x3;
+    static const uint64_t isSemanticRBGAColorBit = 0x4;
 
     static const uint64_t deletedHashValue = 0xFFFFFFFFFFFFFFFD;
     static const uint64_t emptyHashValue = 0xFFFFFFFFFFFFFFFB;
index 0273ac2337459090f6e600e2f2c6cd9c8f6324ec..14c473a3835fc8187f680ff3d461410ae0471540 100644 (file)
@@ -124,6 +124,9 @@ bool FilterOperations::transformColor(Color& color) const
 {
     if (isEmpty() || !color.isValid())
         return false;
+    // Color filter does not apply to semantic CSS colors (like "Windowframe").
+    if (color.isSemantic())
+        return false;
 
     FloatComponents components;
     color.getRGBA(components.components[0], components.components[1], components.components[2], components.components[3]);
index f4e83e0f387cf9c5063987cc239b479d4c8b9a6f..cf98a26cc0f5b5d4a3b869f83fb27bb5eaf3280c 100644 (file)
@@ -37,6 +37,7 @@ OBJC_CLASS NSColor;
 namespace WebCore {
 
 WEBCORE_EXPORT Color colorFromNSColor(NSColor *);
+Color semanticColorFromNSColor(NSColor *);
 WEBCORE_EXPORT NSColor *nsColor(const Color&);
 
 WEBCORE_EXPORT bool usesTestModeFocusRingColor();
index b6df6f43b8765ab68fd1c95f8b7fb427f73d7e73..1ff5b723bd9227b6d1269cdf94e63d22522b7f60 100644 (file)
@@ -95,6 +95,11 @@ Color colorFromNSColor(NSColor *color)
     return Color(makeRGBAFromNSColor(color));
 }
 
+Color semanticColorFromNSColor(NSColor *color)
+{
+    return Color(makeRGBAFromNSColor(color), Color::Semantic);
+}
+
 NSColor *nsColor(const Color& color)
 {
     if (!color.isValid()) {
index 1e334a1b5061c9dc76544d5242d5a58629e92dfd..0c881c50f714d01fcf54d98ce78f6d942baf76af 100644 (file)
@@ -505,7 +505,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
         // Only use NSColor when the system appearance is desired, otherwise use RenderTheme's default.
         if (useSystemAppearance) {
             if (!m_systemVisitedLinkColor.isValid())
-                m_systemVisitedLinkColor = colorFromNSColor([NSColor systemPurpleColor]);
+                m_systemVisitedLinkColor = semanticColorFromNSColor([NSColor systemPurpleColor]);
             return m_systemVisitedLinkColor;
         }
 
@@ -667,7 +667,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
 
         if (auto selector = selectCocoaColor()) {
             if (auto color = wtfObjcMsgSend<NSColor *>([NSColor class], selector))
-                return colorFromNSColor(color);
+                return semanticColorFromNSColor(color);
         }
 
         switch (cssValueID) {
@@ -695,7 +695,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
             NSArray<NSColor *> *alternateColors = [NSColor controlAlternatingRowBackgroundColors];
 #endif
             ASSERT(alternateColors.count >= 2);
-            return colorFromNSColor(alternateColors[0]);
+            return semanticColorFromNSColor(alternateColors[0]);
         }
 
         case CSSValueAppleSystemOddAlternatingContentBackground: {
@@ -705,7 +705,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
             NSArray<NSColor *> *alternateColors = [NSColor controlAlternatingRowBackgroundColors];
 #endif
             ASSERT(alternateColors.count >= 2);
-            return colorFromNSColor(alternateColors[1]);
+            return semanticColorFromNSColor(alternateColors[1]);
         }
 
         case CSSValueBackground:
index 35f1a1dd95880c22bcfd9234a310a727c891c11b..f822a84f0dac7a7a51f66baab82d8e8faf9a23e7 100644 (file)
@@ -242,23 +242,23 @@ void RenderTreeAsText::writeRenderObject(TextStream& ts, const RenderObject& o,
 
         if (o.parent()) {
             Color color = o.style().visitedDependentColor(CSSPropertyColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyColor) != color)
+            if (o.parent()->style().visitedDependentColor(CSSPropertyColor).rgb() != color.rgb())
                 ts << " [color=" << color.nameForRenderTreeAsText() << "]";
 
             // Do not dump invalid or transparent backgrounds, since that is the default.
             Color backgroundColor = o.style().visitedDependentColor(CSSPropertyBackgroundColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyBackgroundColor) != backgroundColor
+            if (o.parent()->style().visitedDependentColor(CSSPropertyBackgroundColor).rgb() != backgroundColor.rgb()
                 && backgroundColor.isValid() && backgroundColor.rgb())
                 ts << " [bgcolor=" << backgroundColor.nameForRenderTreeAsText() << "]";
             
             Color textFillColor = o.style().visitedDependentColor(CSSPropertyWebkitTextFillColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextFillColor) != textFillColor
-                && textFillColor.isValid() && textFillColor != color && textFillColor.rgb())
+            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextFillColor).rgb() != textFillColor.rgb()  
+                && textFillColor.isValid() && textFillColor.rgb() != color.rgb() && textFillColor.rgb())
                 ts << " [textFillColor=" << textFillColor.nameForRenderTreeAsText() << "]";
 
             Color textStrokeColor = o.style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor);
-            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor) != textStrokeColor
-                && textStrokeColor.isValid() && textStrokeColor != color && textStrokeColor.rgb())
+            if (o.parent()->style().visitedDependentColor(CSSPropertyWebkitTextStrokeColor).rgb() != textStrokeColor.rgb()
+                && textStrokeColor.isValid() && textStrokeColor.rgb() != color.rgb() && textStrokeColor.rgb())
                 ts << " [textStrokeColor=" << textStrokeColor.nameForRenderTreeAsText() << "]";
 
             if (o.parent()->style().textStrokeWidth() != o.style().textStrokeWidth() && o.style().textStrokeWidth() > 0)