Text selection color is hard to see in dark mode web views.

https://bugs.webkit.org/show_bug.cgi?id=188260
rdar://problem/42721294

Reviewed by Simon Fraser.

Stop using blendWithWhite() to transform the AppKit selection color in dark mode.
Using an alpha of 80% gives good contrast, and still works good for selections over images.

* platform/graphics/Color.cpp:
(WebCore::Color::blendWithWhite const): Mark new colors as semantic if the original is.
(WebCore::Color::colorWithAlpha const): Ditto.
* rendering/RenderElement.cpp:
(WebCore::RenderElement::selectionBackgroundColor const): Use transformSelectionBackgroundColor.
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::activeSelectionBackgroundColor const): Use transformSelectionBackgroundColor.
(WebCore::RenderTheme::inactiveSelectionBackgroundColor const): Ditto.
(WebCore::RenderTheme::transformSelectionBackgroundColor const): Added. Just blend with white.
* rendering/RenderTheme.h:
* rendering/RenderThemeMac.h:
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::transformSelectionBackgroundColor const): Added. Use an alpha with the color
in dark mode, otherwise fallback to RenderTheme.
(WebCore::RenderThemeMac::systemColor const): Use activeListBoxSelectionBackgroundColor()
and activeSelectionBackgroundColor() instead of caching the colors again. Update hardcoded color.

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

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 17a65c8..e680396 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,31 @@
+2018-08-02  Timothy Hatcher  <timothy@apple.com>
+
+        Text selection color is hard to see in dark mode web views.
+        https://bugs.webkit.org/show_bug.cgi?id=188260
+        rdar://problem/42721294
+
+        Reviewed by Simon Fraser.
+
+        Stop using blendWithWhite() to transform the AppKit selection color in dark mode.
+        Using an alpha of 80% gives good contrast, and still works good for selections over images.
+
+        * platform/graphics/Color.cpp:
+        (WebCore::Color::blendWithWhite const): Mark new colors as semantic if the original is.
+        (WebCore::Color::colorWithAlpha const): Ditto.
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::selectionBackgroundColor const): Use transformSelectionBackgroundColor.
+        * rendering/RenderTheme.cpp:
+        (WebCore::RenderTheme::activeSelectionBackgroundColor const): Use transformSelectionBackgroundColor.
+        (WebCore::RenderTheme::inactiveSelectionBackgroundColor const): Ditto.
+        (WebCore::RenderTheme::transformSelectionBackgroundColor const): Added. Just blend with white.
+        * rendering/RenderTheme.h:
+        * rendering/RenderThemeMac.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::transformSelectionBackgroundColor const): Added. Use an alpha with the color
+        in dark mode, otherwise fallback to RenderTheme.
+        (WebCore::RenderThemeMac::systemColor const): Use activeListBoxSelectionBackgroundColor()
+        and activeSelectionBackgroundColor() instead of caching the colors again. Update hardcoded color.
+
 2018-08-02  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][Floating] Use displayBox.rectWithMargin().bottom instead of displayBox.bottom() to where applicable.

diff --git a/Source/WebCore/platform/graphics/Color.cpp b/Source/WebCore/platform/graphics/Color.cpp
index 3170103..5ef11b0 100644 (file)
--- a/Source/WebCore/platform/graphics/Color.cpp
+++ b/Source/WebCore/platform/graphics/Color.cpp
@@ -500,6 +500,9 @@ Color Color::blendWithWhite() const
         if (r >= 0 && g >= 0 && b >= 0)
             break;
     }
+
+    if (isSemantic())
+        newColor.setIsSemantic();
     return newColor;
 }
 
@@ -515,7 +518,11 @@ Color Color::colorWithAlpha(float alpha) const
         return Color { m_colorData.extendedColor->red(), m_colorData.extendedColor->green(), m_colorData.extendedColor->blue(), alpha, m_colorData.extendedColor->colorSpace() };
 
     int newAlpha = alpha * 255;
-    return Color { red(), green(), blue(), newAlpha };
+
+    Color result = { red(), green(), blue(), newAlpha };
+    if (isSemantic())
+        result.setIsSemantic();
+    return result;
 }
 
 void Color::getRGBA(float& r, float& g, float& b, float& a) const

diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
index 75a4f75..74facee 100644 (file)
--- a/Source/WebCore/rendering/RenderElement.cpp
+++ b/Source/WebCore/rendering/RenderElement.cpp
@@ -1407,11 +1407,11 @@ Color RenderElement::selectionBackgroundColor() const
         return Color();
 
     if (frame().selection().shouldShowBlockCursor() && frame().selection().isCaret())
-        return style().visitedDependentColorWithColorFilter(CSSPropertyColor).blendWithWhite();
+        return theme().transformSelectionBackgroundColor(style().visitedDependentColorWithColorFilter(CSSPropertyColor), document().styleColorOptions());
 
     std::unique_ptr<RenderStyle> pseudoStyle = selectionPseudoStyle();
     if (pseudoStyle && pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor).isValid())
-        return pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor).blendWithWhite();
+        return theme().transformSelectionBackgroundColor(pseudoStyle->visitedDependentColorWithColorFilter(CSSPropertyBackgroundColor), document().styleColorOptions());
 
     if (frame().selection().isFocusedAndActive())
         return theme().activeSelectionBackgroundColor(document().styleColorOptions());

diff --git a/Source/WebCore/rendering/RenderTheme.cpp b/Source/WebCore/rendering/RenderTheme.cpp
index 89d6fbf..56d403b 100644 (file)
--- a/Source/WebCore/rendering/RenderTheme.cpp
+++ b/Source/WebCore/rendering/RenderTheme.cpp
@@ -584,7 +584,7 @@ Color RenderTheme::activeSelectionBackgroundColor(OptionSet<StyleColor::Options>
 {
     auto& cache = colorCache(options);
     if (!cache.activeSelectionBackgroundColor.isValid())
-        cache.activeSelectionBackgroundColor = platformActiveSelectionBackgroundColor(options).blendWithWhite();
+        cache.activeSelectionBackgroundColor = transformSelectionBackgroundColor(platformActiveSelectionBackgroundColor(options), options);
     return cache.activeSelectionBackgroundColor;
 }
 
@@ -592,10 +592,15 @@ Color RenderTheme::inactiveSelectionBackgroundColor(OptionSet<StyleColor::Option
 {
     auto& cache = colorCache(options);
     if (!cache.inactiveSelectionBackgroundColor.isValid())
-        cache.inactiveSelectionBackgroundColor = platformInactiveSelectionBackgroundColor(options).blendWithWhite();
+        cache.inactiveSelectionBackgroundColor = transformSelectionBackgroundColor(platformInactiveSelectionBackgroundColor(options), options);
     return cache.inactiveSelectionBackgroundColor;
 }
 
+Color RenderTheme::transformSelectionBackgroundColor(const Color& color, OptionSet<StyleColor::Options>) const
+{
+    return color.blendWithWhite();
+}
+
 Color RenderTheme::activeSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
 {
     auto& cache = colorCache(options);

diff --git a/Source/WebCore/rendering/RenderTheme.h b/Source/WebCore/rendering/RenderTheme.h
index 5ef5bfc..e2248b3 100644 (file)
--- a/Source/WebCore/rendering/RenderTheme.h
+++ b/Source/WebCore/rendering/RenderTheme.h
@@ -139,6 +139,7 @@ public:
     // Text selection colors.
     Color activeSelectionBackgroundColor(OptionSet<StyleColor::Options>) const;
     Color inactiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const;
+    virtual Color transformSelectionBackgroundColor(const Color&, OptionSet<StyleColor::Options>) const;
     Color activeSelectionForegroundColor(OptionSet<StyleColor::Options>) const;
     Color inactiveSelectionForegroundColor(OptionSet<StyleColor::Options>) const;
 
@@ -415,8 +416,6 @@ protected:
         Color systemVisitedLinkColor;
         Color systemFocusRingColor;
         Color systemControlAccentColor;
-        Color systemSelectedTextBackgroundColor;
-        Color systemSelectedContentBackgroundColor;
 
         Color activeSelectionBackgroundColor;
         Color inactiveSelectionBackgroundColor;

diff --git a/Source/WebCore/rendering/RenderThemeMac.h b/Source/WebCore/rendering/RenderThemeMac.h
index 3fad8c1..5c18d4d 100644 (file)
--- a/Source/WebCore/rendering/RenderThemeMac.h
+++ b/Source/WebCore/rendering/RenderThemeMac.h
@@ -55,6 +55,7 @@ public:
 
     Color platformActiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;
     Color platformActiveSelectionForegroundColor(OptionSet<StyleColor::Options>) const final;
+    Color transformSelectionBackgroundColor(const Color&, OptionSet<StyleColor::Options>) const final;
     Color platformInactiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;
     Color platformInactiveSelectionForegroundColor(OptionSet<StyleColor::Options>) const final;
     Color platformActiveListBoxSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;

diff --git a/Source/WebCore/rendering/RenderThemeMac.mm b/Source/WebCore/rendering/RenderThemeMac.mm
index 06c0c68..eeeff91 100644 (file)
--- a/Source/WebCore/rendering/RenderThemeMac.mm
+++ b/Source/WebCore/rendering/RenderThemeMac.mm
@@ -338,6 +338,18 @@ Color RenderThemeMac::platformInactiveSelectionBackgroundColor(OptionSet<StyleCo
 #endif
 }
 
+Color RenderThemeMac::transformSelectionBackgroundColor(const Color& color, OptionSet<StyleColor::Options> options) const
+{
+    LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDarkAppearance));
+    if (localAppearance.usingDarkAppearance()) {
+        // Use an alpha value that is similar to results from blendWithWhite() on light colors.
+        static const float darkAppearanceAlpha = 0.8;
+        return !color.isOpaque() ? color : color.colorWithAlpha(darkAppearanceAlpha);
+    }
+
+    return RenderTheme::transformSelectionBackgroundColor(color, options);
+}
+
 bool RenderThemeMac::supportsSelectionForegroundColors(OptionSet<StyleColor::Options> options) const
 {
     LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDarkAppearance));
@@ -585,21 +597,11 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
 #endif
 
         case CSSValueAppleSystemSelectedContentBackground:
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
-            return systemAppearanceColor(cache.systemSelectedContentBackgroundColor, @selector(selectedContentBackgroundColor));
-#else
-            return systemAppearanceColor(cache.systemSelectedContentBackgroundColor, @selector(alternateSelectedControlColor));
-#endif
+            return activeListBoxSelectionBackgroundColor(options);
 
         case CSSValueAppleSystemSelectedTextBackground:
         case CSSValueHighlight:
-            // Can't use systemAppearanceColor() since blendWithWhite() needs called before caching as a semantic color.
-            if (!cache.systemSelectedTextBackgroundColor.isValid()) {
-                Color systemColor = semanticColorFromNSColor([NSColor selectedTextBackgroundColor]);
-                cache.systemSelectedTextBackgroundColor = Color(systemColor.blendWithWhite().rgb(), Color::Semantic);
-            }
-
-            return cache.systemSelectedTextBackgroundColor;
+            return activeSelectionBackgroundColor(options);
 
         default:
             // Handle other system colors below, that don't need special system appearance handling.
@@ -783,7 +785,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
         case CSSValueAppleSystemSelectedTextBackground:
             // Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
             if (localAppearance.usingDarkAppearance())
-                return Color(0xCC0F3C6E, Color::Semantic);
+                return Color(0xCC3F638B, Color::Semantic);
             return Color(0x9980BCFE, Color::Semantic);
 
 #if __MAC_OS_X_VERSION_MIN_REQUIRED < 101300