Attachment 343669 Details for Bug 187072 – Patch

[patch] Patch

bug-187072-20180626183321.patch (text/plain), 16.74 KB, created by Timothy Hatcher on 2018-06-26 18:33:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Timothy Hatcher

Created: 2018-06-26 18:33:22 PDT

Size: 16.74 KB

patch

obsolete

>Subversion Revision: 233232
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0470a5e19d8c777a21b69daa0364a365e3c0facb..cb5d1072aabc41b7aa53b6e8cbff5493ee294a06 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,38 @@
>+2018-06-26  Timothy Hatcher  <timothy@apple.com>
>+
>+        Find on page selection color isn't adapted for dark mode.
>+        https://bugs.webkit.org/show_bug.cgi?id=187072
>+        rdar://problem/40354841
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * platform/mac/LocalDefaultSystemAppearance.h:
>+        (WebCore::LocalDefaultSystemAppearance::usingDarkAppearance const): Added.
>+        * platform/mac/LocalDefaultSystemAppearance.mm:
>+        (WebCore::LocalDefaultSystemAppearance::LocalDefaultSystemAppearance): Set m_usingDarkAppearance.
>+        * rendering/InlineTextBox.cpp:
>+        (WebCore::InlineTextBox::paintPlatformDocumentMarkers): Use TextPaintPhase::Decoration since this
>+        matches step three of InlineTextBox::paint ("Paint fancy decorations"). This allows TextMatch to
>+        paint a forground and not end up painting during this "fancy decorations" phase.
>+        (WebCore::InlineTextBox::resolveStyleForMarkedText): Set the fillColor for TextMarker to force a
>+        dark text color which will draw over the yellow highlight.
>+        (WebCore::InlineTextBox::collectMarkedTextsForDocumentMarkers): Added support for TextPaintPhase::Decoration.
>+        Seperate DocumentMarker::TelephoneNumber and DocumentMarker::TextMatch. Have DocumentMarker::TextMatch
>+        support Forground and Background phases.
>+        * rendering/RenderTheme.cpp:
>+        (WebCore::RenderTheme::platformColorsDidChange):
>+        (WebCore::RenderTheme::activeTextSearchHighlightColor const): Added. Call the platfrom version.
>+        (WebCore::RenderTheme::inactiveTextSearchHighlightColor const): Added. Ditto.
>+        (WebCore::RenderTheme::platformActiveTextSearchHighlightColor const): Added StyleColor::Options.
>+        (WebCore::RenderTheme::platformInactiveTextSearchHighlightColor const): Ditto.
>+        * rendering/RenderTheme.h:
>+        * rendering/RenderThemeMac.h:
>+        * rendering/RenderThemeMac.mm:
>+        (WebCore::RenderThemeMac::platformActiveTextSearchHighlightColor const): Added.
>+        (WebCore::RenderThemeMac::platformInactiveTextSearchHighlightColor const): Added.
>+        (WebCore::RenderThemeMac::platformColorsDidChange): Clear new color caches.
>+        (WebCore::RenderThemeMac::systemColor const): Cache system colors by light and dark mode.
>+
> 2018-06-26  Chris Dumez  <cdumez@apple.com>
> 
>         Simplify NetworkStorageSession::getAllStorageAccessEntries()
>diff --git a/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.h b/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.h
>index a1a1845f8b6cd0f4d5e64af9a8d0692879316591..36c0a62390e8a902e7c26479b73c1a033fca3cf0 100644
>--- a/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.h
>+++ b/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.h
>@@ -38,12 +38,24 @@ namespace WebCore {
> // functions which call out into AppKit and rely on the current NSAppearance being set
> class LocalDefaultSystemAppearance {
>     WTF_MAKE_NONCOPYABLE(LocalDefaultSystemAppearance);
>+
> public:
>     WEBCORE_EXPORT LocalDefaultSystemAppearance(bool useSystemAppearance, bool useDefaultAppearance);
>     WEBCORE_EXPORT ~LocalDefaultSystemAppearance();
>+
>+    bool usingDarkAppearance() const
>+    {
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
>+        return m_usingDarkAppearance;
>+#else
>+        return false;
>+#endif
>+    }
>+
> private:
> #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
>     RetainPtr<NSAppearance> m_savedSystemAppearance;
>+    bool m_usingDarkAppearance { false };
> #endif
> };
>     
>diff --git a/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm b/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm
>index 1aba2925a4ceae918ac3197f79bf932e605d052a..667fbb4f9a39df1282b50a5f7aa9eb7e29c6da98 100644
>--- a/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm
>+++ b/Source/WebCore/platform/mac/LocalDefaultSystemAppearance.mm
>@@ -42,7 +42,9 @@ LocalDefaultSystemAppearance::LocalDefaultSystemAppearance(bool useSystemAppeara
>         return;
> 
>     m_savedSystemAppearance = [NSAppearance currentAppearance];
>-    [NSAppearance setCurrentAppearance:[NSAppearance appearanceNamed:(!useSystemAppearance || useDefaultAppearance ? NSAppearanceNameAqua : NSAppearanceNameDarkAqua)]];
>+    m_usingDarkAppearance = useSystemAppearance && !useDefaultAppearance;
>+
>+    [NSAppearance setCurrentAppearance:[NSAppearance appearanceNamed:m_usingDarkAppearance ? NSAppearanceNameDarkAqua : NSAppearanceNameAqua]];
> #else
>     UNUSED_PARAM(useSystemAppearance);
>     UNUSED_PARAM(useDefaultAppearance);
>diff --git a/Source/WebCore/rendering/InlineTextBox.cpp b/Source/WebCore/rendering/InlineTextBox.cpp
>index cea1cd00edb004e37edac5cd4f3060bacf226e6a..26977dfbff6d404d3c0d598df164e69e723ab945 100644
>--- a/Source/WebCore/rendering/InlineTextBox.cpp
>+++ b/Source/WebCore/rendering/InlineTextBox.cpp
>@@ -653,7 +653,7 @@ std::pair<unsigned, unsigned> InlineTextBox::selectionStartEnd() const
> 
> void InlineTextBox::paintPlatformDocumentMarkers(GraphicsContext& context, const FloatPoint& boxOrigin)
> {
>-    for (auto& markedText : subdivide(collectMarkedTextsForDocumentMarkers(TextPaintPhase::Foreground), OverlapStrategy::Frontmost))
>+    for (auto& markedText : subdivide(collectMarkedTextsForDocumentMarkers(TextPaintPhase::Decoration), OverlapStrategy::Frontmost))
>         paintPlatformDocumentMarker(context, boxOrigin, markedText);
> }
> 
>@@ -763,10 +763,16 @@ auto InlineTextBox::resolveStyleForMarkedText(const MarkedText& markedText, cons
>             style.backgroundColor = { 0xff - selectionBackgroundColor.red(), 0xff - selectionBackgroundColor.green(), 0xff - selectionBackgroundColor.blue() };
>         break;
>     }
>-    case MarkedText::TextMatch:
>-        style.backgroundColor = markedText.marker->isActiveMatch() ? renderer().theme().platformActiveTextSearchHighlightColor() : renderer().theme().platformInactiveTextSearchHighlightColor();
>+    case MarkedText::TextMatch: {
>+        // Text matches always use the light system appearance.
>+        OptionSet<StyleColor::Options> styleColorOptions = { StyleColor::Options::UseSystemAppearance, StyleColor::Options::UseDefaultAppearance };
>+#if PLATFORM(MAC)
>+        style.textStyles.fillColor = renderer().theme().systemColor(CSSValueAppleSystemLabel, styleColorOptions);
>+#endif
>+        style.backgroundColor = markedText.marker->isActiveMatch() ? renderer().theme().activeTextSearchHighlightColor(styleColorOptions) : renderer().theme().inactiveTextSearchHighlightColor(styleColorOptions);
>         break;
>     }
>+    }
>     StyledMarkedText styledMarkedText = markedText;
>     styledMarkedText.style = WTFMove(style);
>     return styledMarkedText;
>@@ -832,7 +838,8 @@ Vector<MarkedText> InlineTextBox::collectMarkedTextsForDraggedContent()
> 
> Vector<MarkedText> InlineTextBox::collectMarkedTextsForDocumentMarkers(TextPaintPhase phase)
> {
>-    ASSERT(phase == TextPaintPhase::Background || phase == TextPaintPhase::Foreground);
>+    ASSERT_ARG(phase, phase == TextPaintPhase::Background || phase == TextPaintPhase::Foreground || phase == TextPaintPhase::Decoration);
>+
>     if (!renderer().textNode())
>         return { };
> 
>@@ -876,19 +883,23 @@ Vector<MarkedText> InlineTextBox::collectMarkedTextsForDocumentMarkers(TextPaint
>         // FIXME: Remove the PLATFORM(IOS)-guard.
>         case DocumentMarker::DictationPhraseWithAlternatives:
> #endif
>-            if (phase == TextPaintPhase::Background)
>+            if (phase != TextPaintPhase::Decoration)
>                 continue;
>             break;
>         case DocumentMarker::TextMatch:
>             if (!renderer().frame().editor().markedTextMatchesAreHighlighted())
>                 continue;
>+            if (phase == TextPaintPhase::Decoration)
>+                continue;
>+            break;
> #if ENABLE(TELEPHONE_NUMBER_DETECTION)
>-            FALLTHROUGH;
>         case DocumentMarker::TelephoneNumber:
>-#endif
>-            if (phase == TextPaintPhase::Foreground)
>+            if (!renderer().frame().editor().markedTextMatchesAreHighlighted())
>+                continue;
>+            if (phase != TextPaintPhase::Background)
>                 continue;
>             break;
>+#endif
>         default:
>             continue;
>         }
>diff --git a/Source/WebCore/rendering/RenderTheme.cpp b/Source/WebCore/rendering/RenderTheme.cpp
>index 0988abbb593a30f76b124766a183d936e07582c1..b22ced7af2d8b0d6d9becc51deca66d6dc2aee5e 100644
>--- a/Source/WebCore/rendering/RenderTheme.cpp
>+++ b/Source/WebCore/rendering/RenderTheme.cpp
>@@ -1158,6 +1158,9 @@ void RenderTheme::platformColorsDidChange()
>     m_activeListBoxSelectionBackgroundColor = Color();
>     m_inactiveListBoxSelectionForegroundColor = Color();
> 
>+    m_activeTextSearchHighlightColor = Color();
>+    m_inactiveTextSearchHighlightColor = Color();
>+
>     Page::updateStyleForAllPagesAfterGlobalChangeInEnvironment();
> }
> 
>@@ -1283,12 +1286,26 @@ Color RenderTheme::systemColor(CSSValueID cssValueId, OptionSet<StyleColor::Opti
>     return Color();
> }
> 
>-Color RenderTheme::platformActiveTextSearchHighlightColor() const
>+Color RenderTheme::activeTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
>+{
>+    if (!m_activeTextSearchHighlightColor.isValid())
>+        m_activeTextSearchHighlightColor = platformActiveTextSearchHighlightColor(options);
>+    return m_activeTextSearchHighlightColor;
>+}
>+
>+Color RenderTheme::inactiveTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
>+{
>+    if (!m_inactiveTextSearchHighlightColor.isValid())
>+        m_inactiveTextSearchHighlightColor = platformInactiveTextSearchHighlightColor(options);
>+    return m_inactiveTextSearchHighlightColor;
>+}
>+
>+Color RenderTheme::platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const
> {
>     return Color(255, 150, 50); // Orange.
> }
> 
>-Color RenderTheme::platformInactiveTextSearchHighlightColor() const
>+Color RenderTheme::platformInactiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const
> {
>     return Color(255, 255, 0); // Yellow.
> }
>diff --git a/Source/WebCore/rendering/RenderTheme.h b/Source/WebCore/rendering/RenderTheme.h
>index 4ad3b599154bb559cfdaef31f3f9d14bf0f11f28..281b52f8235e385ee99377104102ea02a68c6cba 100644
>--- a/Source/WebCore/rendering/RenderTheme.h
>+++ b/Source/WebCore/rendering/RenderTheme.h
>@@ -145,8 +145,8 @@ public:
>     Color inactiveListBoxSelectionForegroundColor(OptionSet<StyleColor::Options>) const;
> 
>     // Highlighting colors for TextMatches.
>-    virtual Color platformActiveTextSearchHighlightColor() const;
>-    virtual Color platformInactiveTextSearchHighlightColor() const;
>+    Color activeTextSearchHighlightColor(OptionSet<StyleColor::Options>) const;
>+    Color inactiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const;
> 
>     virtual Color disabledTextColor(const Color& textColor, const Color& backgroundColor) const;
> 
>@@ -265,6 +265,10 @@ protected:
>     virtual Color platformActiveListBoxSelectionForegroundColor(OptionSet<StyleColor::Options>) const;
>     virtual Color platformInactiveListBoxSelectionForegroundColor(OptionSet<StyleColor::Options>) const;
> 
>+    // The platcform highlighting colors for TextMatches.
>+    virtual Color platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const;
>+    virtual Color platformInactiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const;
>+
>     virtual bool supportsSelectionForegroundColors() const { return true; }
>     virtual bool supportsListBoxSelectionForegroundColors() const { return true; }
> 
>@@ -404,6 +408,9 @@ private:
>     mutable Color m_inactiveListBoxSelectionBackgroundColor;
>     mutable Color m_activeListBoxSelectionForegroundColor;
>     mutable Color m_inactiveListBoxSelectionForegroundColor;
>+
>+    mutable Color m_activeTextSearchHighlightColor;
>+    mutable Color m_inactiveTextSearchHighlightColor;
> };
> 
> } // namespace WebCore
>diff --git a/Source/WebCore/rendering/RenderThemeMac.h b/Source/WebCore/rendering/RenderThemeMac.h
>index 05759b32b9e46f20469354a2abb12f968f2f3d2e..a00df42400568abda7ae935f7b0225108246907e 100644
>--- a/Source/WebCore/rendering/RenderThemeMac.h
>+++ b/Source/WebCore/rendering/RenderThemeMac.h
>@@ -62,6 +62,8 @@ public:
>     Color platformInactiveListBoxSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;
>     Color platformInactiveListBoxSelectionForegroundColor(OptionSet<StyleColor::Options>) const final;
>     Color platformFocusRingColor(OptionSet<StyleColor::Options>) const final;
>+    Color platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const final;
>+    Color platformInactiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const final;
> 
>     ScrollbarControlSize scrollbarControlSizeForPart(ControlPart) final { return SmallScrollbar; }
> 
>@@ -242,8 +244,10 @@ private:
>     bool m_isSliderThumbHorizontalPressed { false };
>     bool m_isSliderThumbVerticalPressed { false };
> 
>-    mutable HashMap<int, Color> m_systemColorCache;
>-    mutable Color m_systemVisitedLinkColor;
>+    mutable HashMap<int, Color> m_lightSystemColorCache;
>+    mutable HashMap<int, Color> m_darkSystemColorCache;
>+    mutable Color m_lightSystemVisitedLinkColor;
>+    mutable Color m_darkSystemVisitedLinkColor;
> 
>     RetainPtr<WebCoreRenderThemeNotificationObserver> m_notificationObserver;
> 
>diff --git a/Source/WebCore/rendering/RenderThemeMac.mm b/Source/WebCore/rendering/RenderThemeMac.mm
>index 0c881c50f714d01fcf54d98ce78f6d942baf76af..76e8c3abafae38330123a9cb5a5b61ca487d1496 100644
>--- a/Source/WebCore/rendering/RenderThemeMac.mm
>+++ b/Source/WebCore/rendering/RenderThemeMac.mm
>@@ -391,6 +391,22 @@ Color RenderThemeMac::platformFocusRingColor(OptionSet<StyleColor::Options> opti
>     return systemColor(CSSValueWebkitFocusRingColor, options);
> }
> 
>+Color RenderThemeMac::platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
>+{
>+    LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDefaultAppearance));
>+#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101300
>+    return colorFromNSColor([NSColor findHighlightColor]);
>+#else
>+    return colorFromNSColor([NSColor systemYellowColor]);
>+#endif
>+}
>+
>+Color RenderThemeMac::platformInactiveTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
>+{
>+    LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDefaultAppearance));
>+    return colorFromNSColor([NSColor systemOrangeColor]);
>+}
>+
> static FontSelectionValue toFontWeight(NSInteger appKitFontWeight)
> {
>     ASSERT(appKitFontWeight > 0 && appKitFontWeight < 15);
>@@ -486,8 +502,12 @@ static RGBA32 menuBackgroundColor()
> 
> void RenderThemeMac::platformColorsDidChange()
> {
>-    m_systemColorCache.clear();
>-    m_systemVisitedLinkColor = Color();
>+    m_lightSystemColorCache.clear();
>+    m_darkSystemColorCache.clear();
>+
>+    m_lightSystemVisitedLinkColor = Color();
>+    m_darkSystemVisitedLinkColor = Color();
>+
>     RenderTheme::platformColorsDidChange();
> }
> 
>@@ -504,9 +524,15 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
>     if (forVisitedLink && cssValueID == CSSValueWebkitLink) {
>         // Only use NSColor when the system appearance is desired, otherwise use RenderTheme's default.
>         if (useSystemAppearance) {
>-            if (!m_systemVisitedLinkColor.isValid())
>-                m_systemVisitedLinkColor = semanticColorFromNSColor([NSColor systemPurpleColor]);
>-            return m_systemVisitedLinkColor;
>+            if (localAppearance.usingDarkAppearance()) {
>+                if (!m_darkSystemVisitedLinkColor.isValid())
>+                    m_darkSystemVisitedLinkColor = semanticColorFromNSColor([NSColor systemPurpleColor]);
>+                return m_darkSystemVisitedLinkColor;
>+            }
>+
>+            if (!m_lightSystemVisitedLinkColor.isValid())
>+                m_lightSystemVisitedLinkColor = semanticColorFromNSColor([NSColor systemPurpleColor]);
>+            return m_lightSystemVisitedLinkColor;
>         }
> 
>         return RenderTheme::systemColor(cssValueID, options);
>@@ -514,7 +540,8 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::O
> 
>     ASSERT(!forVisitedLink);
> 
>-    return m_systemColorCache.ensure(cssValueID, [this, cssValueID, useSystemAppearance, options] () -> Color {
>+    auto& systemColorCache = localAppearance.usingDarkAppearance() ? m_darkSystemColorCache : m_lightSystemColorCache;
>+    return systemColorCache.ensure(cssValueID, [this, cssValueID, useSystemAppearance, options] () -> Color {
>         auto selectCocoaColor = [cssValueID, useSystemAppearance] () -> SEL {
>             switch (cssValueID) {
>             case CSSValueWebkitLink:

Actions: View | Formatted Diff | Diff

Attachments on bug 187072: 343664 | 343669 | 343680 | 343681 | 343683 | 343685 | 343715 | 343727 | 343733 | 343744 | 343751 | 343753 | 343817 | 343818 | 343828 | 343837 | 343845 | 343848