Attachment 346406 Details for Bug 188260 – Patch

[patch] Patch

bug-188260-20180802123425.patch (text/plain), 11.02 KB, created by Timothy Hatcher on 2018-08-02 12:34:25 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Timothy Hatcher

Created: 2018-08-02 12:34:25 PDT

Size: 11.02 KB

patch

obsolete

>Subversion Revision: 234481
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index bb97c08c988d15b528ef6397d4e6ab6088d90ded..6c5f170e87b4c02b6145762d6fa88db8dfb1541f 100644
>--- 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-01  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Revert back to only one list for the all the floatings.
>diff --git a/Source/WebCore/platform/graphics/Color.cpp b/Source/WebCore/platform/graphics/Color.cpp
>index 31701036b624fed49354bae95e82ded1d12a02e4..5ef11b08e70aa4ffc967686b44fca9bb5671c218 100644
>--- 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 75a4f758b8cee7ddace04d89b8dd67204a9e636a..74facee1d2a61f21fcbb126249f1021a68a7d7a0 100644
>--- 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 89d6fbfb17f4a35685302001df3d41df95983240..56d403ba247d17c0d2dbec32ff94fefa4c58fb48 100644
>--- 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 5ef5bfc18671fa6dc96416a092eb326975de52b4..e2248b33ffa208a8111fcac5a62731ad4f435de6 100644
>--- 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 3fad8c1e0812ecbc759a6a7ff529c40d382fb41d..5c18d4daf281accadd4eea1a1d67ea821c073b76 100644
>--- 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 06c0c68adffdfb1fd304c5eba4fc2f039eec5372..eeeff91523812083e568d1cce697b0a91935e823 100644
>--- 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

Actions: View | Formatted Diff | Diff

Attachments on bug 188260: 346389 | 346390 | 346391 | 346406