Use the system's link color when system appearance is desired for a WebView.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Apr 2018 08:35:00 +0000 (08:35 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 7 Apr 2018 08:35:00 +0000 (08:35 +0000)
https://bugs.webkit.org/show_bug.cgi?id=184353
rdar://problem/9420053

Reviewed by Wenson Hsieh.

Source/WebCore:

Have Document consult RenderTheme via StyleColor for the various link colors.
This allows the system to have different colors than the standard hardcoded ones.
This adds StyleColor::Options, to avoid multiple booleans being passed around,
since the "for visited link" state is now needed in RenderTheme.

* WebCore.xcodeproj/project.pbxproj: Made StyleColor.h private, since RenderTheme.h includes it.
* css/StyleColor.cpp:
(WebCore::StyleColor::colorFromKeyword): Use options instead of a bool.
(WebCore::StyleColor::isSystemColor): Consider CSSValueWebkitLink the start of system colors.
* css/StyleColor.h:
* css/StyleResolver.cpp:
(WebCore::StyleResolver::colorFromPrimitiveValue const): Use StyleColor::Options.
* css/parser/CSSParser.cpp:
(WebCore::CSSParser::parseSystemColor): Use StyleColor::Options.
* dom/Document.cpp:
(WebCore::Document::resetLinkColor): Ask StyleColor for the link color instead of hardcoding it.
(WebCore::Document::resetVisitedLinkColor): Ditto.
(WebCore::Document::resetActiveLinkColor): Ditto.
(WebCore::Document::styleColorOptions const): Added. Helper to get the options used.
* dom/Document.h:
* html/canvas/CanvasRenderingContext2D.cpp:
(WebCore::CanvasRenderingContext2D::drawFocusIfNeededInternal): Use StyleColor::Options.
* rendering/RenderTheme.cpp:
(WebCore::RenderTheme::systemColor const): Add default values here, moved from Document.
(WebCore::RenderTheme::focusRingColor): Use StyleColor::Options.
* rendering/RenderTheme.h:
(WebCore::RenderTheme::platformFocusRingColor const): Use StyleColor::Options.
* rendering/RenderThemeGtk.cpp:
(WebCore::RenderThemeGtk::systemColor const): Use StyleColor::Options.
* rendering/RenderThemeGtk.h:
* rendering/RenderThemeIOS.h:
* rendering/RenderThemeIOS.mm:
(WebCore::RenderThemeIOS::systemColor const): Use StyleColor::Options.
* rendering/RenderThemeMac.h:
* rendering/RenderThemeMac.mm:
(WebCore::RenderThemeMac::platformFocusRingColor const): Use StyleColor::Options.
(WebCore::RenderThemeMac::platformColorsDidChange): Clear m_systemVisitedLinkColor.
(WebCore::RenderThemeMac::systemColor const): Use StyleColor::Options.
(WebCore::RenderThemeMac::adjustMenuListStyle const): Ditto.
* rendering/RenderThemeWin.cpp:
(WebCore::RenderThemeWin::systemColor const): Use StyleColor::Options.
* rendering/RenderThemeWin.h:
* rendering/TextPaintStyle.cpp:
(WebCore::computeTextPaintStyle): Use StyleColor::Options.

Source/WebCore/PAL:

* pal/spi/cocoa/NSColorSPI.h: Added linkColor.

Tools:

* TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* TestWebKitAPI/Tests/WebKitCocoa/SystemColors.mm: Added.
(TestWebKitAPI::WebKit::LinkColor):
(TestWebKitAPI::WebKit::LinkColorWithSystemAppearance):

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

25 files changed:
Source/WebCore/ChangeLog
Source/WebCore/PAL/ChangeLog
Source/WebCore/PAL/pal/spi/cocoa/NSColorSPI.h
Source/WebCore/WebCore.xcodeproj/project.pbxproj
Source/WebCore/css/StyleColor.cpp
Source/WebCore/css/StyleColor.h
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/parser/CSSParser.cpp
Source/WebCore/dom/Document.cpp
Source/WebCore/dom/Document.h
Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp
Source/WebCore/rendering/RenderTheme.cpp
Source/WebCore/rendering/RenderTheme.h
Source/WebCore/rendering/RenderThemeGtk.cpp
Source/WebCore/rendering/RenderThemeGtk.h
Source/WebCore/rendering/RenderThemeIOS.h
Source/WebCore/rendering/RenderThemeIOS.mm
Source/WebCore/rendering/RenderThemeMac.h
Source/WebCore/rendering/RenderThemeMac.mm
Source/WebCore/rendering/RenderThemeWin.cpp
Source/WebCore/rendering/RenderThemeWin.h
Source/WebCore/rendering/TextPaintStyle.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj
Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemColors.mm [new file with mode: 0644]

index a6497d7..a15f7a9 100644 (file)
@@ -1,3 +1,57 @@
+2018-04-07  Timothy Hatcher  <timothy@apple.com>
+
+        Use the system's link color when system appearance is desired for a WebView.
+
+        https://bugs.webkit.org/show_bug.cgi?id=184353
+        rdar://problem/9420053
+
+        Reviewed by Wenson Hsieh.
+
+        Have Document consult RenderTheme via StyleColor for the various link colors.
+        This allows the system to have different colors than the standard hardcoded ones.
+        This adds StyleColor::Options, to avoid multiple booleans being passed around,
+        since the "for visited link" state is now needed in RenderTheme.
+
+        * WebCore.xcodeproj/project.pbxproj: Made StyleColor.h private, since RenderTheme.h includes it.
+        * css/StyleColor.cpp:
+        (WebCore::StyleColor::colorFromKeyword): Use options instead of a bool.
+        (WebCore::StyleColor::isSystemColor): Consider CSSValueWebkitLink the start of system colors.
+        * css/StyleColor.h:
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::colorFromPrimitiveValue const): Use StyleColor::Options.
+        * css/parser/CSSParser.cpp:
+        (WebCore::CSSParser::parseSystemColor): Use StyleColor::Options.
+        * dom/Document.cpp:
+        (WebCore::Document::resetLinkColor): Ask StyleColor for the link color instead of hardcoding it.
+        (WebCore::Document::resetVisitedLinkColor): Ditto.
+        (WebCore::Document::resetActiveLinkColor): Ditto.
+        (WebCore::Document::styleColorOptions const): Added. Helper to get the options used.
+        * dom/Document.h:
+        * html/canvas/CanvasRenderingContext2D.cpp:
+        (WebCore::CanvasRenderingContext2D::drawFocusIfNeededInternal): Use StyleColor::Options.
+        * rendering/RenderTheme.cpp:
+        (WebCore::RenderTheme::systemColor const): Add default values here, moved from Document.
+        (WebCore::RenderTheme::focusRingColor): Use StyleColor::Options.
+        * rendering/RenderTheme.h:
+        (WebCore::RenderTheme::platformFocusRingColor const): Use StyleColor::Options.
+        * rendering/RenderThemeGtk.cpp:
+        (WebCore::RenderThemeGtk::systemColor const): Use StyleColor::Options.
+        * rendering/RenderThemeGtk.h:
+        * rendering/RenderThemeIOS.h:
+        * rendering/RenderThemeIOS.mm:
+        (WebCore::RenderThemeIOS::systemColor const): Use StyleColor::Options.
+        * rendering/RenderThemeMac.h:
+        * rendering/RenderThemeMac.mm:
+        (WebCore::RenderThemeMac::platformFocusRingColor const): Use StyleColor::Options.
+        (WebCore::RenderThemeMac::platformColorsDidChange): Clear m_systemVisitedLinkColor.
+        (WebCore::RenderThemeMac::systemColor const): Use StyleColor::Options.
+        (WebCore::RenderThemeMac::adjustMenuListStyle const): Ditto.
+        * rendering/RenderThemeWin.cpp:
+        (WebCore::RenderThemeWin::systemColor const): Use StyleColor::Options.
+        * rendering/RenderThemeWin.h:
+        * rendering/TextPaintStyle.cpp:
+        (WebCore::computeTextPaintStyle): Use StyleColor::Options.
+
 2018-04-06  Youenn Fablet  <youenn@apple.com>
 
         Response headers should be filtered when sent from NetworkProcess to WebProcess
index 458529a..ce3479d 100644 (file)
@@ -1,3 +1,14 @@
+2018-04-07  Timothy Hatcher  <timothy@apple.com>
+
+        Use the system's link color when system appearance is desired for a WebView.
+
+        https://bugs.webkit.org/show_bug.cgi?id=184353
+        rdar://problem/9420053
+
+        Reviewed by Wenson Hsieh.
+
+        * pal/spi/cocoa/NSColorSPI.h: Added linkColor.
+
 2018-04-05  John Wilander  <wilander@apple.com>
 
         Add necessary colon to CFNetwork selector
index 99ed496..c9719a8 100644 (file)
@@ -41,6 +41,7 @@
 + (NSColor *)systemPinkColor;
 + (NSColor *)systemPurpleColor;
 + (NSColor *)systemGrayColor;
++ (NSColor *)linkColor;
 @end
 
 #endif
index 690b9e9..2ae0c8a 100644 (file)
                93F9B6E10BA0FB7200854064 /* JSComment.h in Headers */ = {isa = PBXBuildFile; fileRef = 93F9B6DF0BA0FB7200854064 /* JSComment.h */; };
                93F9B7A10BA6032600854064 /* JSCDATASection.h in Headers */ = {isa = PBXBuildFile; fileRef = 93F9B79F0BA6032600854064 /* JSCDATASection.h */; };
                93FDAFCA0B11307400E2746F /* EditorInsertAction.h in Headers */ = {isa = PBXBuildFile; fileRef = 93FDAFC90B11307400E2746F /* EditorInsertAction.h */; settings = {ATTRIBUTES = (Private, ); }; };
-               9418278B1D8B244000492764 /* StyleColor.h in Headers */ = {isa = PBXBuildFile; fileRef = 941827891D8B242200492764 /* StyleColor.h */; };
+               9418278B1D8B244000492764 /* StyleColor.h in Headers */ = {isa = PBXBuildFile; fileRef = 941827891D8B242200492764 /* StyleColor.h */; settings = {ATTRIBUTES = (Private, ); }; };
                9418278F1D8CAF9200492764 /* CSSPendingSubstitutionValue.h in Headers */ = {isa = PBXBuildFile; fileRef = 9418278D1D8CAE9500492764 /* CSSPendingSubstitutionValue.h */; };
                9444CBD41D860C8B0073A074 /* SizesCalcParser.h in Headers */ = {isa = PBXBuildFile; fileRef = 9444CBCF1D860C740073A074 /* SizesCalcParser.h */; };
                9444CBD61D860C8B0073A074 /* SizesAttributeParser.h in Headers */ = {isa = PBXBuildFile; fileRef = 9444CBD11D860C740073A074 /* SizesAttributeParser.h */; };
index 447a1b0..29bca19 100644 (file)
 
 namespace WebCore {
 
-Color StyleColor::colorFromKeyword(CSSValueID keyword, bool useSystemAppearance)
+Color StyleColor::colorFromKeyword(CSSValueID keyword, OptionSet<Options> options)
 {
     if (const char* valueName = getValueName(keyword)) {
         if (const NamedColor* namedColor = findColor(valueName, strlen(valueName)))
             return Color(namedColor->ARGBValue);
     }
-    return RenderTheme::singleton().systemColor(keyword, useSystemAppearance);
+
+    return RenderTheme::singleton().systemColor(keyword, options);
 }
 
 bool StyleColor::isColorKeyword(CSSValueID id)
@@ -53,7 +54,7 @@ bool StyleColor::isColorKeyword(CSSValueID id)
 
 bool StyleColor::isSystemColor(CSSValueID id)
 {
-    return (id >= CSSValueActiveborder && id <= CSSValueWebkitFocusRingColor) || id == CSSValueMenu || id == CSSValueText;
+    return (id >= CSSValueWebkitLink && id <= CSSValueWebkitFocusRingColor) || id == CSSValueMenu || id == CSSValueText;
 }
 
 } // namespace WebCore
index fb59b61..decb2aa 100644 (file)
@@ -33,6 +33,7 @@
 
 #include "CSSValueKeywords.h"
 #include "Color.h"
+#include <wtf/OptionSet.h>
 
 namespace WebCore {
 
@@ -50,7 +51,12 @@ public:
 
     const Color& resolve(const Color& currentColor) const { return m_currentColor ? currentColor : m_color; }
 
-    static Color colorFromKeyword(CSSValueID, bool useSystemAppearance);
+    enum class Options : uint8_t {
+        ForVisitedLink = 1 << 0,
+        UseSystemAppearance = 1 << 1
+    };
+
+    static Color colorFromKeyword(CSSValueID, OptionSet<Options>);
     static bool isColorKeyword(CSSValueID);
     static bool isSystemColor(CSSValueID);
 
index 051ed38..1318d65 100644 (file)
@@ -1829,14 +1829,14 @@ Color StyleResolver::colorFromPrimitiveValue(const CSSPrimitiveValue& value, boo
     case CSSValueWebkitActivelink:
         return document().activeLinkColor();
     case CSSValueWebkitFocusRingColor:
-        return RenderTheme::focusRingColor(document().useSystemAppearance());
+        return RenderTheme::focusRingColor(document().styleColorOptions());
     case CSSValueCurrentcolor:
         // Color is an inherited property so depending on it effectively makes the property inherited.
         // FIXME: Setting the flag as a side effect of calling this function is a bit oblique. Can we do better?
         m_state.style()->setHasExplicitlyInheritedProperties();
         return m_state.style()->color();
     default:
-        return StyleColor::colorFromKeyword(identifier, document().useSystemAppearance());
+        return StyleColor::colorFromKeyword(identifier, document().styleColorOptions());
     }
 }
 
index fda88fb..a87c671 100644 (file)
@@ -181,9 +181,11 @@ Color CSSParser::parseSystemColor(const String& string, std::optional<const CSSP
     CSSValueID id = cssValueKeywordID(string);
     if (!StyleColor::isSystemColor(id))
         return Color();
-    if (context)
-        return RenderTheme::singleton().systemColor(id, context.value().useSystemAppearance);
-    return RenderTheme::singleton().systemColor(id, false);
+
+    OptionSet<StyleColor::Options> options;
+    if (context && context.value().useSystemAppearance)
+        options |= StyleColor::Options::UseSystemAppearance;
+    return RenderTheme::singleton().systemColor(id, options);
 }
 
 RefPtr<CSSValue> CSSParser::parseSingleValue(CSSPropertyID propertyID, const String& string, const CSSParserContext& context)
index c7a182b..69b6ca6 100644 (file)
 #include "SocketProvider.h"
 #include "StorageEvent.h"
 #include "StringCallback.h"
+#include "StyleColor.h"
 #include "StyleProperties.h"
 #include "StyleResolveForDocument.h"
 #include "StyleResolver.h"
@@ -801,17 +802,17 @@ String Document::compatMode() const
 
 void Document::resetLinkColor()
 {
-    m_linkColor = Color(0, 0, 238);
+    m_linkColor = StyleColor::colorFromKeyword(CSSValueWebkitLink, styleColorOptions());
 }
 
 void Document::resetVisitedLinkColor()
 {
-    m_visitedLinkColor = Color(85, 26, 139);    
+    m_visitedLinkColor = StyleColor::colorFromKeyword(CSSValueWebkitLink, styleColorOptions() | StyleColor::Options::ForVisitedLink);
 }
 
 void Document::resetActiveLinkColor()
 {
-    m_activeLinkColor = Color(255, 0, 0);
+    m_activeLinkColor = StyleColor::colorFromKeyword(CSSValueWebkitActivelink, styleColorOptions());
 }
 
 DOMImplementation& Document::implementation()
@@ -7027,7 +7028,7 @@ float Document::deviceScaleFactor() const
         deviceScaleFactor = documentPage->deviceScaleFactor();
     return deviceScaleFactor;
 }
-    
+
 bool Document::useSystemAppearance() const
 {
     bool useSystemAppearance = false;
@@ -7035,7 +7036,15 @@ bool Document::useSystemAppearance() const
         useSystemAppearance = documentPage->useSystemAppearance();
     return useSystemAppearance;
 }
-    
+
+OptionSet<StyleColor::Options> Document::styleColorOptions() const
+{
+    OptionSet<StyleColor::Options> options;
+    if (useSystemAppearance())
+        options |= StyleColor::Options::UseSystemAppearance;
+    return options;
+}
+
 void Document::didAssociateFormControl(Element* element)
 {
     if (!frame() || !frame()->page() || !frame()->page()->chrome().client().shouldNotifyOnFormChanges())
index bae9f2f..bd72e58 100644 (file)
@@ -45,6 +45,7 @@
 #include "RenderPtr.h"
 #include "ScriptExecutionContext.h"
 #include "StringWithDirection.h"
+#include "StyleColor.h"
 #include "Supplementable.h"
 #include "TextResourceDecoder.h"
 #include "Timer.h"
@@ -543,8 +544,9 @@ public:
     Settings& mutableSettings() { return m_settings.get(); }
 
     float deviceScaleFactor() const;
-        
+
     bool useSystemAppearance() const;
+    OptionSet<StyleColor::Options> styleColorOptions() const;
 
     WEBCORE_EXPORT Ref<Range> createRange();
 
index ca3d0d4..c27af23 100644 (file)
@@ -84,7 +84,7 @@ void CanvasRenderingContext2D::drawFocusIfNeededInternal(const Path& path, Eleme
     auto* context = drawingContext();
     if (!element.focused() || !state().hasInvertibleTransform || path.isEmpty() || !element.isDescendantOf(canvas()) || !context)
         return;
-    context->drawFocusRing(path, 1, 1, RenderTheme::focusRingColor(element.document().useSystemAppearance()));
+    context->drawFocusRing(path, 1, 1, RenderTheme::focusRingColor(element.document().styleColorOptions()));
 }
 
 String CanvasRenderingContext2D::font() const
index 07d7004..6a4e5b5 100644 (file)
@@ -1208,11 +1208,13 @@ void RenderTheme::systemFont(CSSValueID systemFontID, FontCascadeDescription& fo
     updateCachedSystemFontDescription(systemFontID, fontDescription);
 }
 
-Color RenderTheme::systemColor(CSSValueID cssValueId, bool useSystemAppearance) const
+Color RenderTheme::systemColor(CSSValueID cssValueId, OptionSet<StyleColor::Options> options) const
 {
-    UNUSED_PARAM(useSystemAppearance);
-    
     switch (cssValueId) {
+    case CSSValueWebkitLink:
+        return options.contains(StyleColor::Options::ForVisitedLink) ? 0xFF551A8B : 0xFF0000EE;
+    case CSSValueWebkitActivelink:
+        return 0xFFFF0000;
     case CSSValueActiveborder:
         return 0xFFFFFFFF;
     case CSSValueActivebuttontext:
@@ -1327,9 +1329,9 @@ void RenderTheme::setCustomFocusRingColor(const Color& color)
     customFocusRingColor() = color;
 }
 
-Color RenderTheme::focusRingColor(bool useSystemAppearance)
+Color RenderTheme::focusRingColor(OptionSet<StyleColor::Options> options)
 {
-    return customFocusRingColor().isValid() ? customFocusRingColor() : RenderTheme::singleton().platformFocusRingColor(useSystemAppearance);
+    return customFocusRingColor().isValid() ? customFocusRingColor() : RenderTheme::singleton().platformFocusRingColor(options);
 }
 
 String RenderTheme::fileListDefaultLabel(bool multipleFilesAllowed) const
index 0c655cf..1503eef 100644 (file)
@@ -24,6 +24,7 @@
 #include "PaintInfo.h"
 #include "PopupMenuStyle.h"
 #include "ScrollTypes.h"
+#include "StyleColor.h"
 #include "ThemeTypes.h"
 
 namespace WebCore {
@@ -149,8 +150,8 @@ public:
 
     virtual Color disabledTextColor(const Color& textColor, const Color& backgroundColor) const;
 
-    static Color focusRingColor(bool useSystemAppearance);
-    virtual Color platformFocusRingColor(bool) const { return Color(0, 0, 0); }
+    static Color focusRingColor(OptionSet<StyleColor::Options>);
+    virtual Color platformFocusRingColor(OptionSet<StyleColor::Options>) const { return Color(0, 0, 0); }
     static void setCustomFocusRingColor(const Color&);
     static float platformFocusRingWidth() { return 3; }
     static float platformFocusRingOffset(float outlineWidth) { return std::max<float>(outlineWidth - platformFocusRingWidth(), 0); }
@@ -164,7 +165,7 @@ public:
 
     // System fonts and colors for CSS.
     void systemFont(CSSValueID, FontCascadeDescription&) const;
-    virtual Color systemColor(CSSValueID, bool useSystemAppearance) const;
+    virtual Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const;
 
     virtual int minimumMenuListSize(const RenderStyle&) const { return 0; }
 
index 2b1de58..0785910 100644 (file)
@@ -1756,7 +1756,7 @@ Color RenderThemeGtk::platformInactiveListBoxSelectionForegroundColor() const
     return styleColor(ListBox, GTK_STATE_FLAG_SELECTED, StyleColorForeground);
 }
 
-Color RenderThemeGtk::systemColor(CSSValueID cssValueId, bool) const
+Color RenderThemeGtk::systemColor(CSSValueID cssValueId, OptionSet<StyleColor::Options> options) const
 {
     switch (cssValueId) {
     case CSSValueButtontext:
@@ -1764,7 +1764,7 @@ Color RenderThemeGtk::systemColor(CSSValueID cssValueId, bool) const
     case CSSValueCaptiontext:
         return styleColor(Entry, GTK_STATE_FLAG_ACTIVE, StyleColorForeground);
     default:
-        return RenderTheme::systemColor(cssValueId, false);
+        return RenderTheme::systemColor(cssValueId, options);
     }
 }
 
index ab5b3d2..00a19e1 100644 (file)
@@ -86,7 +86,7 @@ public:
     void platformColorsDidChange() override;
 
     // System colors.
-    Color systemColor(CSSValueID, bool) const override;
+    Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const override;
 
     bool popsMenuBySpaceOrReturn() const override { return true; }
 
index 071700a..9b600c7 100644 (file)
@@ -135,7 +135,7 @@ private:
     const Color& shadowColor() const;
     FloatRect addRoundedBorderClip(const RenderObject& box, GraphicsContext&, const IntRect&);
 
-    Color systemColor(CSSValueID, bool) const override;
+    Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const override;
 
     String m_legacyMediaControlsScript;
     String m_mediaControlsScript;
index 33b5181..4fe83e0 100644 (file)
@@ -1397,8 +1397,18 @@ String RenderThemeIOS::mediaControlsBase64StringForIconNameAndType(const String&
 
 #endif // ENABLE(VIDEO)
 
-Color RenderThemeIOS::systemColor(CSSValueID cssValueID, bool) const
+Color RenderThemeIOS::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::Options> options) const
 {
+    const bool forVisitedLink = options.contains(StyleColor::Options::ForVisitedLink);
+
+    // 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 by
+    // calling through to RenderTheme's base implementation.
+    if (forVisitedLink && cssValueID == CSSValueWebkitLink)
+        return RenderTheme::systemColor(cssValueID, options);
+
+    ASSERT(!forVisitedLink);
+
     auto addResult = m_systemColorCache.add(cssValueID, Color());
     if (!addResult.isNewEntry)
         return addResult.iterator->value;
@@ -1434,7 +1444,7 @@ Color RenderThemeIOS::systemColor(CSSValueID cssValueID, bool) const
     }
 
     if (!color.isValid())
-        color = RenderTheme::systemColor(cssValueID, false);
+        color = RenderTheme::systemColor(cssValueID, options);
 
     addResult.iterator->value = color;
 
index 4b6de26..3a86311 100644 (file)
@@ -59,7 +59,7 @@ public:
     Color platformActiveListBoxSelectionForegroundColor() const final;
     Color platformInactiveListBoxSelectionBackgroundColor(bool) const final;
     Color platformInactiveListBoxSelectionForegroundColor() const final;
-    Color platformFocusRingColor(bool useSystemAppearance) const final;
+    Color platformFocusRingColor(OptionSet<StyleColor::Options>) const final;
 
     ScrollbarControlSize scrollbarControlSizeForPart(ControlPart) final { return SmallScrollbar; }
 
@@ -169,7 +169,7 @@ private:
 private:
     String fileListNameForWidth(const FileList*, const FontCascade&, int width, bool multipleFilesAllowed) const final;
 
-    Color systemColor(CSSValueID, bool useSystemAppearance) const final;
+    Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const final;
 
     void purgeCaches() final;
 
@@ -243,6 +243,7 @@ private:
     bool m_isSliderThumbVerticalPressed { false };
 
     mutable HashMap<int, Color> m_systemColorCache;
+    mutable Color m_systemVisitedLinkColor;
 
     RetainPtr<WebCoreRenderThemeNotificationObserver> m_notificationObserver;
 
index 66215a5..c8ba2cb 100644 (file)
@@ -315,12 +315,11 @@ Color RenderThemeMac::platformInactiveListBoxSelectionForegroundColor() const
     return Color::black;
 }
 
-Color RenderThemeMac::platformFocusRingColor(bool useSystemAppearance) const
+Color RenderThemeMac::platformFocusRingColor(OptionSet<StyleColor::Options> options) const
 {
     if (usesTestModeFocusRingColor())
         return oldAquaFocusRingColor();
-
-    return systemColor(CSSValueWebkitFocusRingColor, useSystemAppearance);
+    return systemColor(CSSValueWebkitFocusRingColor, options);
 }
 
 Color RenderThemeMac::platformInactiveListBoxSelectionBackgroundColor(bool useSystemAppearance) const
@@ -425,15 +424,42 @@ static RGBA32 menuBackgroundColor()
 void RenderThemeMac::platformColorsDidChange()
 {
     m_systemColorCache.clear();
+    m_systemVisitedLinkColor = Color();
     RenderTheme::platformColorsDidChange();
 }
 
-Color RenderThemeMac::systemColor(CSSValueID cssValueID, bool useSystemAppearance) const
+Color RenderThemeMac::systemColor(CSSValueID cssValueID, OptionSet<StyleColor::Options> options) const
 {
+    const bool useSystemAppearance = options.contains(StyleColor::Options::UseSystemAppearance);
+    const bool forVisitedLink = options.contains(StyleColor::Options::ForVisitedLink);
+
     LocalDefaultSystemAppearance localAppearance(useSystemAppearance);
-    return m_systemColorCache.ensure(cssValueID, [this, cssValueID, useSystemAppearance] () -> Color {
-        auto selectCocoaColor = [cssValueID] () -> SEL {
+
+    // 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 (!m_systemVisitedLinkColor.isValid())
+                m_systemVisitedLinkColor = colorFromNSColor([NSColor systemPurpleColor]);
+            return m_systemVisitedLinkColor;
+        }
+
+        return RenderTheme::systemColor(cssValueID, options);
+    }
+
+    ASSERT(!forVisitedLink);
+
+    return m_systemColorCache.ensure(cssValueID, [this, cssValueID, useSystemAppearance, options] () -> Color {
+        auto selectCocoaColor = [cssValueID, useSystemAppearance] () -> SEL {
             switch (cssValueID) {
+            case CSSValueWebkitLink:
+                // Only use NSColor when the system appearance is desired, otherwise use RenderTheme's default.
+                return useSystemAppearance ? @selector(linkColor) : nullptr;
+            case CSSValueWebkitActivelink:
+                // Only use NSColor when the system appearance is desired, otherwise use RenderTheme's default.
+                // FIXME: Use a semantic system color for this, instead of systemRedColor. <rdar://problem/39256684>
+                return useSystemAppearance ? @selector(systemRedColor) : nullptr;
             case CSSValueActiveborder:
                 return @selector(keyboardFocusIndicatorColor);
             case CSSValueActivecaption:
@@ -524,10 +550,12 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, bool useSystemAppearanc
                 return nullptr;
             }
         };
+
         if (auto selector = selectCocoaColor()) {
             if (auto color = wtfObjcMsgSend<NSColor *>([NSColor class], selector))
                 return colorFromNSColor(color);
         }
+
         switch (cssValueID) {
         case CSSValueActivebuttontext:
             // No corresponding NSColor for this so we use a hard coded value.
@@ -546,7 +574,7 @@ Color RenderThemeMac::systemColor(CSSValueID cssValueID, bool useSystemAppearanc
             // Use platform-independent value returned by base class.
             FALLTHROUGH;
         default:
-            return RenderTheme::systemColor(cssValueID, useSystemAppearance);
+            return RenderTheme::systemColor(cssValueID, options);
         }
     }).iterator->value;
 }
@@ -1297,8 +1325,8 @@ void RenderThemeMac::adjustMenuListStyle(StyleResolver& styleResolver, RenderSty
     // Set the foreground color to black or gray when we have the aqua look.
     Color c = Color::darkGray;
     if (e) {
-        bool useSystemAppearance = e->document().page()->useSystemAppearance();
-        c = !e->isDisabledFormControl() ? systemColor(CSSValueButtontext, useSystemAppearance) : systemColor(CSSValueGraytext, useSystemAppearance);
+        OptionSet<StyleColor::Options> options = e->document().styleColorOptions();
+        c = !e->isDisabledFormControl() ? systemColor(CSSValueButtontext, options) : systemColor(CSSValueGraytext, options);
     }
     style.setColor(c);
 
index fa7e668..deea1a9 100644 (file)
@@ -1012,11 +1012,11 @@ static int cssValueIdToSysColorIndex(CSSValueID cssValueId)
     }
 }
 
-Color RenderThemeWin::systemColor(CSSValueID cssValueId, bool) const
+Color RenderThemeWin::systemColor(CSSValueID cssValueId, OptionSet<StyleColor::Options> options) const
 {
     int sysColorIndex = cssValueIdToSysColorIndex(cssValueId);
     if (sysColorIndex == -1)
-        return RenderTheme::systemColor(cssValueId, false);
+        return RenderTheme::systemColor(cssValueId, options);
 
     COLORREF color = GetSysColor(sysColorIndex);
     return Color(GetRValue(color), GetGValue(color), GetBValue(color));
index 8bcdd60..d3f1be1 100644 (file)
@@ -59,7 +59,7 @@ public:
     Color platformActiveSelectionForegroundColor() const override;
     Color platformInactiveSelectionForegroundColor() const override;
 
-    Color systemColor(CSSValueID, bool) const override;
+    Color systemColor(CSSValueID, OptionSet<StyleColor::Options>) const override;
 
     bool paintCheckbox(const RenderObject& o, const PaintInfo& i, const IntRect& r) override
     { return paintButton(o, i, r); }
index 06f9165..ebfdec2 100644 (file)
@@ -98,7 +98,10 @@ TextPaintStyle computeTextPaintStyle(const Frame& frame, const RenderStyle& line
     if (lineStyle.insideDefaultButton()) {
         Page* page = frame.page();
         if (page && page->focusController().isActive()) {
-            paintStyle.fillColor = RenderTheme::singleton().systemColor(CSSValueActivebuttontext, page->useSystemAppearance());
+            OptionSet<StyleColor::Options> options;
+            if (page->useSystemAppearance())
+                options |= StyleColor::Options::UseSystemAppearance;
+            paintStyle.fillColor = RenderTheme::singleton().systemColor(CSSValueActivebuttontext, options);
             return paintStyle;
         }
     }
index fa88968..5a8eb3a 100644 (file)
@@ -1,3 +1,17 @@
+2018-04-07  Timothy Hatcher  <timothy@apple.com>
+
+        Use the system's link color when system appearance is desired for a WebView.
+
+        https://bugs.webkit.org/show_bug.cgi?id=184353
+        rdar://problem/9420053
+
+        Reviewed by Wenson Hsieh.
+
+        * TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
+        * TestWebKitAPI/Tests/WebKitCocoa/SystemColors.mm: Added.
+        (TestWebKitAPI::WebKit::LinkColor):
+        (TestWebKitAPI::WebKit::LinkColorWithSystemAppearance):
+
 2018-04-06  Zalan Bujtas  <zalan@apple.com>
 
         Rebaseline LayoutReloaded patch file.
index 296bc5c..c2ed2be 100644 (file)
@@ -62,6 +62,7 @@
                1C2B81831C891F0900A5529F /* CancelFontSubresourcePlugIn.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1C2B81811C891EFA00A5529F /* CancelFontSubresourcePlugIn.mm */; };
                1C2B81861C89259D00A5529F /* webfont.html in Copy Resources */ = {isa = PBXBuildFile; fileRef = 1C2B81841C8924A200A5529F /* webfont.html */; };
                1C2B81871C8925A000A5529F /* Ahem.ttf in Copy Resources */ = {isa = PBXBuildFile; fileRef = 1C2B81851C89252300A5529F /* Ahem.ttf */; };
+               1C734B5320788C4800F430EA /* SystemColors.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1C734B5220788C4800F430EA /* SystemColors.mm */; };
                1C9EB8411E380DA1005C6442 /* ComplexTextController.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1C9EB8401E380DA1005C6442 /* ComplexTextController.cpp */; };
                1CAD1F861E5CE7DA00AF2C2C /* FontCache.cpp in Sources */ = {isa = PBXBuildFile; fileRef = 1CAD1F851E5CE7DA00AF2C2C /* FontCache.cpp */; };
                1F83571B1D3FFB2300E3967B /* WKBackForwardList.mm in Sources */ = {isa = PBXBuildFile; fileRef = 1F83571A1D3FFB0E00E3967B /* WKBackForwardList.mm */; };
                1C2B81811C891EFA00A5529F /* CancelFontSubresourcePlugIn.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = CancelFontSubresourcePlugIn.mm; sourceTree = "<group>"; };
                1C2B81841C8924A200A5529F /* webfont.html */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.html; path = webfont.html; sourceTree = "<group>"; };
                1C2B81851C89252300A5529F /* Ahem.ttf */ = {isa = PBXFileReference; lastKnownFileType = file; path = Ahem.ttf; sourceTree = "<group>"; };
+               1C734B5220788C4800F430EA /* SystemColors.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = SystemColors.mm; sourceTree = "<group>"; };
                1C9EB8401E380DA1005C6442 /* ComplexTextController.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = ComplexTextController.cpp; sourceTree = "<group>"; };
                1CAD1F851E5CE7DA00AF2C2C /* FontCache.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = FontCache.cpp; sourceTree = "<group>"; };
                1CB9BC371A67482300FE5678 /* WeakPtr.cpp */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; path = WeakPtr.cpp; sourceTree = "<group>"; };
                                2D9A53AE1B31FA8D0074D5AA /* ShrinkToFit.mm */,
                                2DFF7B6C1DA487AF00814614 /* SnapshotStore.mm */,
                                515BE1701D428BD100DD7C68 /* StoreBlobThenDelete.mm */,
+                               1C734B5220788C4800F430EA /* SystemColors.mm */,
                                5CB40B4D1F4B98BE007DC7B9 /* UIDelegate.mm */,
                                7CC3E1FA197E234100BE6252 /* UserContentController.mm */,
                                7C882E031C80C624006BF731 /* UserContentWorld.mm */,
                                CE4D5DE71F6743BA0072CFC6 /* StringWithDirection.cpp in Sources */,
                                7CCE7ED21A411A7E00447C4C /* SubresourceErrorCrash.mm in Sources */,
                                7CCE7EA81A411A1900447C4C /* SyntheticBackingScaleFactorWindow.m in Sources */,
+                               1C734B5320788C4800F430EA /* SystemColors.mm in Sources */,
                                7CCE7F161A411AE600447C4C /* TerminateTwice.cpp in Sources */,
                                7CCE7EA91A411A1D00447C4C /* TestBrowsingContextLoadDelegate.mm in Sources */,
                                2D1C04A71D76298B000A6816 /* TestNavigationDelegate.mm in Sources */,
diff --git a/Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemColors.mm b/Tools/TestWebKitAPI/Tests/WebKitCocoa/SystemColors.mm
new file mode 100644 (file)
index 0000000..ff914d7
--- /dev/null
@@ -0,0 +1,62 @@
+/*
+ * Copyright (C) 2018 Apple Inc. All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, this list of conditions and the following disclaimer.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE INC. AND ITS CONTRIBUTORS ``AS IS''
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
+ * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
+ * PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL APPLE INC. OR ITS CONTRIBUTORS
+ * BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF
+ * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS
+ * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN
+ * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
+ * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF
+ * THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include "config.h"
+
+#if WK_API_ENABLED
+
+#import "PlatformUtilities.h"
+#import "TestWKWebView.h"
+#import <WebKit/WKWebViewPrivate.h>
+#import <wtf/RetainPtr.h>
+
+namespace TestWebKitAPI {
+
+TEST(WebKit, LinkColor)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+
+    [webView synchronouslyLoadHTMLString:@"<a href>Test</a>"];
+
+    NSString *linkColor = [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.links[0]).color"];
+    EXPECT_WK_STREQ("rgb(0, 0, 238)", linkColor);
+}
+
+#if PLATFORM(MAC)
+TEST(WebKit, LinkColorWithSystemAppearance)
+{
+    auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600)]);
+    [webView _setUseSystemAppearance:YES];
+
+    [webView synchronouslyLoadHTMLString:@"<a href>Test</a>"];
+
+    NSString *linkColor = [webView stringByEvaluatingJavaScript:@"getComputedStyle(document.links[0]).color"];
+    EXPECT_WK_STREQ("rgb(0, 105, 217)", linkColor);
+}
+#endif
+
+} // namespace TestWebKitAPI
+
+#endif