- Revision
- 233532
- Author
- [email protected]
- Date
- 2018-07-05 12:11:39 -0700 (Thu, 05 Jul 2018)
Log Message
Don't invert text color for selections in light mode.
https://bugs.webkit.org/show_bug.cgi?id=187349
rdar://problem/41297946
Reviewed by Tim Horton.
Fix color caching in RenderTheme so we don't cache a dark mode color
for a light appearance, or vise versa. Use the new color caching
in RenderThemeMac, and clear the color caches in purgeCaches.
Allow supportsSelectionForegroundColors to conditionalize on StyleColor::Options.
Return true only in dark mode.
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::activeSelectionBackgroundColor const):
(WebCore::RenderTheme::inactiveSelectionBackgroundColor const):
(WebCore::RenderTheme::activeSelectionForegroundColor const):
(WebCore::RenderTheme::inactiveSelectionForegroundColor const):
(WebCore::RenderTheme::activeListBoxSelectionBackgroundColor const):
(WebCore::RenderTheme::inactiveListBoxSelectionBackgroundColor const):
(WebCore::RenderTheme::activeListBoxSelectionForegroundColor const):
(WebCore::RenderTheme::inactiveListBoxSelectionForegroundColor const):
(WebCore::RenderTheme::purgeCaches):
(WebCore::RenderTheme::platformColorsDidChange):
(WebCore::RenderTheme::activeTextSearchHighlightColor const):
(WebCore::RenderTheme::inactiveTextSearchHighlightColor const):
* rendering/RenderTheme.h:
(WebCore::RenderTheme::supportsSelectionForegroundColors const):
(WebCore::RenderTheme::supportsListBoxSelectionForegroundColors const):
(WebCore::RenderTheme::colorCache const):
(WebCore::RenderTheme::purgeCaches): Deleted.
* rendering/RenderThemeMac.h:
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::purgeCaches):
(WebCore::RenderThemeMac::supportsSelectionForegroundColors const):
(WebCore::RenderThemeMac::platformActiveSelectionForegroundColor const):
(WebCore::RenderThemeMac::platformInactiveSelectionForegroundColor const):
(WebCore::RenderThemeMac::platformColorsDidChange):
(WebCore::RenderThemeMac::colorCache const):
(WebCore::RenderThemeMac::systemColor const):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (233531 => 233532)
--- trunk/Source/WebCore/ChangeLog 2018-07-05 18:36:25 UTC (rev 233531)
+++ trunk/Source/WebCore/ChangeLog 2018-07-05 19:11:39 UTC (rev 233532)
@@ -1,3 +1,46 @@
+2018-07-05 Timothy Hatcher <[email protected]>
+
+ Don't invert text color for selections in light mode.
+ https://bugs.webkit.org/show_bug.cgi?id=187349
+ rdar://problem/41297946
+
+ Reviewed by Tim Horton.
+
+ Fix color caching in RenderTheme so we don't cache a dark mode color
+ for a light appearance, or vise versa. Use the new color caching
+ in RenderThemeMac, and clear the color caches in purgeCaches.
+
+ Allow supportsSelectionForegroundColors to conditionalize on StyleColor::Options.
+ Return true only in dark mode.
+
+ * rendering/RenderTheme.cpp:
+ (WebCore::RenderTheme::activeSelectionBackgroundColor const):
+ (WebCore::RenderTheme::inactiveSelectionBackgroundColor const):
+ (WebCore::RenderTheme::activeSelectionForegroundColor const):
+ (WebCore::RenderTheme::inactiveSelectionForegroundColor const):
+ (WebCore::RenderTheme::activeListBoxSelectionBackgroundColor const):
+ (WebCore::RenderTheme::inactiveListBoxSelectionBackgroundColor const):
+ (WebCore::RenderTheme::activeListBoxSelectionForegroundColor const):
+ (WebCore::RenderTheme::inactiveListBoxSelectionForegroundColor const):
+ (WebCore::RenderTheme::purgeCaches):
+ (WebCore::RenderTheme::platformColorsDidChange):
+ (WebCore::RenderTheme::activeTextSearchHighlightColor const):
+ (WebCore::RenderTheme::inactiveTextSearchHighlightColor const):
+ * rendering/RenderTheme.h:
+ (WebCore::RenderTheme::supportsSelectionForegroundColors const):
+ (WebCore::RenderTheme::supportsListBoxSelectionForegroundColors const):
+ (WebCore::RenderTheme::colorCache const):
+ (WebCore::RenderTheme::purgeCaches): Deleted.
+ * rendering/RenderThemeMac.h:
+ * rendering/RenderThemeMac.mm:
+ (WebCore::RenderThemeMac::purgeCaches):
+ (WebCore::RenderThemeMac::supportsSelectionForegroundColors const):
+ (WebCore::RenderThemeMac::platformActiveSelectionForegroundColor const):
+ (WebCore::RenderThemeMac::platformInactiveSelectionForegroundColor const):
+ (WebCore::RenderThemeMac::platformColorsDidChange):
+ (WebCore::RenderThemeMac::colorCache const):
+ (WebCore::RenderThemeMac::systemColor const):
+
2018-07-05 Zalan Bujtas <[email protected]>
SimpleLineLayout::FlowContents wastes 54KB of Vector capacity on nytimes.com
Modified: trunk/Source/WebCore/rendering/RenderTheme.cpp (233531 => 233532)
--- trunk/Source/WebCore/rendering/RenderTheme.cpp 2018-07-05 18:36:25 UTC (rev 233531)
+++ trunk/Source/WebCore/rendering/RenderTheme.cpp 2018-07-05 19:11:39 UTC (rev 233532)
@@ -581,58 +581,66 @@
Color RenderTheme::activeSelectionBackgroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_activeSelectionBackgroundColor.isValid())
- m_activeSelectionBackgroundColor = platformActiveSelectionBackgroundColor(options).blendWithWhite();
- return m_activeSelectionBackgroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.activeSelectionBackgroundColor.isValid())
+ cache.activeSelectionBackgroundColor = platformActiveSelectionBackgroundColor(options).blendWithWhite();
+ return cache.activeSelectionBackgroundColor;
}
Color RenderTheme::inactiveSelectionBackgroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_inactiveSelectionBackgroundColor.isValid())
- m_inactiveSelectionBackgroundColor = platformInactiveSelectionBackgroundColor(options).blendWithWhite();
- return m_inactiveSelectionBackgroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.inactiveSelectionBackgroundColor.isValid())
+ cache.inactiveSelectionBackgroundColor = platformInactiveSelectionBackgroundColor(options).blendWithWhite();
+ return cache.inactiveSelectionBackgroundColor;
}
Color RenderTheme::activeSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_activeSelectionForegroundColor.isValid() && supportsSelectionForegroundColors())
- m_activeSelectionForegroundColor = platformActiveSelectionForegroundColor(options);
- return m_activeSelectionForegroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.activeSelectionForegroundColor.isValid() && supportsSelectionForegroundColors(options))
+ cache.activeSelectionForegroundColor = platformActiveSelectionForegroundColor(options);
+ return cache.activeSelectionForegroundColor;
}
Color RenderTheme::inactiveSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_inactiveSelectionForegroundColor.isValid() && supportsSelectionForegroundColors())
- m_inactiveSelectionForegroundColor = platformInactiveSelectionForegroundColor(options);
- return m_inactiveSelectionForegroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.inactiveSelectionForegroundColor.isValid() && supportsSelectionForegroundColors(options))
+ cache.inactiveSelectionForegroundColor = platformInactiveSelectionForegroundColor(options);
+ return cache.inactiveSelectionForegroundColor;
}
Color RenderTheme::activeListBoxSelectionBackgroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_activeListBoxSelectionBackgroundColor.isValid())
- m_activeListBoxSelectionBackgroundColor = platformActiveListBoxSelectionBackgroundColor(options);
- return m_activeListBoxSelectionBackgroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.activeListBoxSelectionBackgroundColor.isValid())
+ cache.activeListBoxSelectionBackgroundColor = platformActiveListBoxSelectionBackgroundColor(options);
+ return cache.activeListBoxSelectionBackgroundColor;
}
Color RenderTheme::inactiveListBoxSelectionBackgroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_inactiveListBoxSelectionBackgroundColor.isValid())
- m_inactiveListBoxSelectionBackgroundColor = platformInactiveListBoxSelectionBackgroundColor(options);
- return m_inactiveListBoxSelectionBackgroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.inactiveListBoxSelectionBackgroundColor.isValid())
+ cache.inactiveListBoxSelectionBackgroundColor = platformInactiveListBoxSelectionBackgroundColor(options);
+ return cache.inactiveListBoxSelectionBackgroundColor;
}
Color RenderTheme::activeListBoxSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_activeListBoxSelectionForegroundColor.isValid() && supportsListBoxSelectionForegroundColors())
- m_activeListBoxSelectionForegroundColor = platformActiveListBoxSelectionForegroundColor(options);
- return m_activeListBoxSelectionForegroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.activeListBoxSelectionForegroundColor.isValid() && supportsListBoxSelectionForegroundColors(options))
+ cache.activeListBoxSelectionForegroundColor = platformActiveListBoxSelectionForegroundColor(options);
+ return cache.activeListBoxSelectionForegroundColor;
}
Color RenderTheme::inactiveListBoxSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_inactiveListBoxSelectionForegroundColor.isValid() && supportsListBoxSelectionForegroundColors())
- m_inactiveListBoxSelectionForegroundColor = platformInactiveListBoxSelectionForegroundColor(options);
- return m_inactiveListBoxSelectionForegroundColor;
+ auto& cache = colorCache(options);
+ if (!cache.inactiveListBoxSelectionForegroundColor.isValid() && supportsListBoxSelectionForegroundColors(options))
+ cache.inactiveListBoxSelectionForegroundColor = platformInactiveListBoxSelectionForegroundColor(options);
+ return cache.inactiveListBoxSelectionForegroundColor;
}
Color RenderTheme::platformActiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const
@@ -1146,21 +1154,15 @@
{
}
+void RenderTheme::purgeCaches()
+{
+ m_colorCache = ColorCache();
+}
+
void RenderTheme::platformColorsDidChange()
{
- m_activeSelectionForegroundColor = Color();
- m_inactiveSelectionForegroundColor = Color();
- m_activeSelectionBackgroundColor = Color();
- m_inactiveSelectionBackgroundColor = Color();
+ m_colorCache = ColorCache();
- m_activeListBoxSelectionForegroundColor = Color();
- m_inactiveListBoxSelectionForegroundColor = Color();
- m_activeListBoxSelectionBackgroundColor = Color();
- m_inactiveListBoxSelectionForegroundColor = Color();
-
- m_activeTextSearchHighlightColor = Color();
- m_inactiveTextSearchHighlightColor = Color();
-
Page::updateStyleForAllPagesAfterGlobalChangeInEnvironment();
}
@@ -1288,16 +1290,18 @@
Color RenderTheme::activeTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_activeTextSearchHighlightColor.isValid())
- m_activeTextSearchHighlightColor = platformActiveTextSearchHighlightColor(options);
- return m_activeTextSearchHighlightColor;
+ auto& cache = colorCache(options);
+ if (!cache.activeTextSearchHighlightColor.isValid())
+ cache.activeTextSearchHighlightColor = platformActiveTextSearchHighlightColor(options);
+ return cache.activeTextSearchHighlightColor;
}
Color RenderTheme::inactiveTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
{
- if (!m_inactiveTextSearchHighlightColor.isValid())
- m_inactiveTextSearchHighlightColor = platformInactiveTextSearchHighlightColor(options);
- return m_inactiveTextSearchHighlightColor;
+ auto& cache = colorCache(options);
+ if (!cache.inactiveTextSearchHighlightColor.isValid())
+ cache.inactiveTextSearchHighlightColor = platformInactiveTextSearchHighlightColor(options);
+ return cache.inactiveTextSearchHighlightColor;
}
Color RenderTheme::platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options>) const
Modified: trunk/Source/WebCore/rendering/RenderTheme.h (233531 => 233532)
--- trunk/Source/WebCore/rendering/RenderTheme.h 2018-07-05 18:36:25 UTC (rev 233531)
+++ trunk/Source/WebCore/rendering/RenderTheme.h 2018-07-05 19:11:39 UTC (rev 233532)
@@ -20,6 +20,7 @@
#pragma once
+#include "ColorHash.h"
#include "ControlStates.h"
#include "PaintInfo.h"
#include "PopupMenuStyle.h"
@@ -26,6 +27,7 @@
#include "ScrollTypes.h"
#include "StyleColor.h"
#include "ThemeTypes.h"
+#include <wtf/HashMap.h>
namespace WebCore {
@@ -54,7 +56,7 @@
// appropriate platform theme.
WEBCORE_EXPORT static RenderTheme& singleton();
- virtual void purgeCaches() { }
+ virtual void purgeCaches();
// This method is called whenever style has been computed for an element and the appearance
// property has been set to a value other than "none". The theme should map in all of the appropriate
@@ -269,8 +271,8 @@
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; }
+ virtual bool supportsSelectionForegroundColors(OptionSet<StyleColor::Options>) const { return true; }
+ virtual bool supportsListBoxSelectionForegroundColors(OptionSet<StyleColor::Options>) const { return true; }
#if !USE(NEW_THEME)
// Methods for each appearance value.
@@ -398,19 +400,30 @@
bool isReadOnlyControl(const RenderObject&) const;
bool isDefault(const RenderObject&) const;
-private:
- mutable Color m_activeSelectionBackgroundColor;
- mutable Color m_inactiveSelectionBackgroundColor;
- mutable Color m_activeSelectionForegroundColor;
- mutable Color m_inactiveSelectionForegroundColor;
+protected:
+ struct ColorCache {
+ HashMap<int, Color> systemStyleColors;
- mutable Color m_activeListBoxSelectionBackgroundColor;
- mutable Color m_inactiveListBoxSelectionBackgroundColor;
- mutable Color m_activeListBoxSelectionForegroundColor;
- mutable Color m_inactiveListBoxSelectionForegroundColor;
+ Color systemVisitedLinkColor;
- mutable Color m_activeTextSearchHighlightColor;
- mutable Color m_inactiveTextSearchHighlightColor;
+ Color activeSelectionBackgroundColor;
+ Color inactiveSelectionBackgroundColor;
+ Color activeSelectionForegroundColor;
+ Color inactiveSelectionForegroundColor;
+
+ Color activeListBoxSelectionBackgroundColor;
+ Color inactiveListBoxSelectionBackgroundColor;
+ Color activeListBoxSelectionForegroundColor;
+ Color inactiveListBoxSelectionForegroundColor;
+
+ Color activeTextSearchHighlightColor;
+ Color inactiveTextSearchHighlightColor;
+ };
+
+ virtual ColorCache& colorCache(OptionSet<StyleColor::Options>) const { return m_colorCache; }
+
+private:
+ mutable ColorCache m_colorCache;
};
} // namespace WebCore
Modified: trunk/Source/WebCore/rendering/RenderThemeMac.h (233531 => 233532)
--- trunk/Source/WebCore/rendering/RenderThemeMac.h 2018-07-05 18:36:25 UTC (rev 233531)
+++ trunk/Source/WebCore/rendering/RenderThemeMac.h 2018-07-05 19:11:39 UTC (rev 233532)
@@ -23,8 +23,6 @@
#if PLATFORM(MAC)
#import "RenderThemeCocoa.h"
-#import <wtf/RetainPtr.h>
-#import <wtf/HashMap.h>
#if ENABLE(SERVICE_CONTROLS)
OBJC_CLASS NSServicesRolloverButtonCell;
@@ -53,6 +51,8 @@
bool isControlStyled(const RenderStyle&, const BorderData&, const FillLayer&, const Color& backgroundColor) const final;
+ bool supportsSelectionForegroundColors(OptionSet<StyleColor::Options>) const final;
+
Color platformActiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;
Color platformActiveSelectionForegroundColor(OptionSet<StyleColor::Options>) const final;
Color platformInactiveSelectionBackgroundColor(OptionSet<StyleColor::Options>) const final;
@@ -173,6 +173,8 @@
Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const final;
+ ColorCache& colorCache(OptionSet<StyleColor::Options>) const final;
+
void purgeCaches() final;
// Get the control size based off the font. Used by some of the controls (like buttons).
@@ -244,10 +246,7 @@
bool m_isSliderThumbHorizontalPressed { false };
bool m_isSliderThumbVerticalPressed { false };
- mutable HashMap<int, Color> m_lightSystemColorCache;
- mutable HashMap<int, Color> m_darkSystemColorCache;
- mutable Color m_lightSystemVisitedLinkColor;
- mutable Color m_darkSystemVisitedLinkColor;
+ mutable ColorCache m_darkColorCache;
RetainPtr<WebCoreRenderThemeNotificationObserver> m_notificationObserver;
Modified: trunk/Source/WebCore/rendering/RenderThemeMac.mm (233531 => 233532)
--- trunk/Source/WebCore/rendering/RenderThemeMac.mm 2018-07-05 18:36:25 UTC (rev 233531)
+++ trunk/Source/WebCore/rendering/RenderThemeMac.mm 2018-07-05 19:11:39 UTC (rev 233532)
@@ -259,6 +259,9 @@
m_mediaControlsScript.clearImplIfNotShared();
m_legacyMediaControlsStyleSheet.clearImplIfNotShared();
m_mediaControlsStyleSheet.clearImplIfNotShared();
+ m_darkColorCache = ColorCache();
+
+ RenderTheme::purgeCaches();
}
String RenderThemeMac::mediaControlsScript()
@@ -328,10 +331,18 @@
#endif
}
+bool RenderThemeMac::supportsSelectionForegroundColors(OptionSet<StyleColor::Options> options) const
+{
+ LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDefaultAppearance));
+ return localAppearance.usingDarkAppearance();
+}
+
Color RenderThemeMac::platformActiveSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
{
LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDefaultAppearance));
- return colorFromNSColor([NSColor selectedTextColor]);
+ if (localAppearance.usingDarkAppearance())
+ return colorFromNSColor([NSColor selectedTextColor]);
+ return { };
}
Color RenderThemeMac::platformInactiveSelectionForegroundColor(OptionSet<StyleColor::Options> options) const
@@ -338,10 +349,12 @@
{
#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseSystemAppearance), options.contains(StyleColor::Options::UseDefaultAppearance));
- return colorFromNSColor([NSColor unemphasizedSelectedTextColor]);
+ if (localAppearance.usingDarkAppearance())
+ return colorFromNSColor([NSColor unemphasizedSelectedTextColor]);
+ return { };
#else
UNUSED_PARAM(options);
- return colorFromNSColor([NSColor textColor]);
+ return { };
#endif
}
@@ -504,15 +517,21 @@
void RenderThemeMac::platformColorsDidChange()
{
- m_lightSystemColorCache.clear();
- m_darkSystemColorCache.clear();
+ m_darkColorCache = ColorCache();
- m_lightSystemVisitedLinkColor = Color();
- m_darkSystemVisitedLinkColor = Color();
-
RenderTheme::platformColorsDidChange();
}
+auto RenderThemeMac::colorCache(OptionSet<StyleColor::Options> options) const -> ColorCache&
+{
+ const bool useSystemAppearance = options.contains(StyleColor::Options::UseSystemAppearance);
+ const bool useDefaultAppearance = options.contains(StyleColor::Options::UseDefaultAppearance);
+ LocalDefaultSystemAppearance localAppearance(useSystemAppearance, useDefaultAppearance);
+ if (localAppearance.usingDarkAppearance())
+ return m_darkColorCache;
+ return RenderTheme::colorCache(options);
+}
+
Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::Options> options) const
{
const bool useSystemAppearance = options.contains(StyleColor::Options::UseSystemAppearance);
@@ -521,20 +540,16 @@
LocalDefaultSystemAppearance localAppearance(useSystemAppearance, useDefaultAppearance);
+ auto& cache = colorCache(options);
+
// The system color cache below can't handle visited links. The only color value
// that cares about visited links is CSSValueWebkitLink, so handle it here.
if (forVisitedLink && cssValueID == CSSValueWebkitLink) {
// Only use NSColor when the system appearance is desired, otherwise use RenderTheme's default.
if (useSystemAppearance) {
- 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;
+ if (!cache.systemVisitedLinkColor.isValid())
+ cache.systemVisitedLinkColor = semanticColorFromNSColor([NSColor systemPurpleColor]);
+ return cache.systemVisitedLinkColor;
}
return RenderTheme::systemColor(cssValueID, options);
@@ -542,8 +557,7 @@
ASSERT(!forVisitedLink);
- auto& systemColorCache = localAppearance.usingDarkAppearance() ? m_darkSystemColorCache : m_lightSystemColorCache;
- return systemColorCache.ensure(cssValueID, [this, cssValueID, useSystemAppearance, options] () -> Color {
+ return cache.systemStyleColors.ensure(cssValueID, [this, cssValueID, useSystemAppearance, options] () -> Color {
auto selectCocoaColor = [cssValueID, useSystemAppearance] () -> SEL {
switch (cssValueID) {
case CSSValueWebkitLink:
