Focus ring color does not honor dark mode or system accent color.
authortimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 18:51:07 +0000 (18:51 +0000)
committertimothy@apple.com <timothy@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 28 Jun 2018 18:51:07 +0000 (18:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=187144
rdar://problem/41105081

Reviewed by Tim Horton.

Pass the focus ring color through to the GraphicsContext methods that draw it.

* platform/graphics/GraphicsContext.h:
* platform/graphics/cocoa/GraphicsContextCocoa.mm:
(WebCore::drawFocusRingAtTime):
(WebCore::drawFocusRing):
(WebCore::drawFocusRingToContext):
(WebCore::drawFocusRingToContextAtTime):
(WebCore::GraphicsContext::drawFocusRing):
(WebCore::GraphicsContext::focusRingColor): Deleted.
* platform/mac/ThemeMac.mm:
(WebCore::drawCellFocusRingWithFrameAtTime):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::paintFocusRing):
* rendering/RenderImage.cpp:
(WebCore::RenderImage::paintAreaElementFocusRing):

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

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/GraphicsContext.h
Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm
Source/WebCore/platform/mac/ThemeMac.mm
Source/WebCore/rendering/RenderElement.cpp
Source/WebCore/rendering/RenderImage.cpp

index 469e7ea..ba19e90 100644 (file)
@@ -1,3 +1,28 @@
+2018-06-28  Timothy Hatcher  <timothy@apple.com>
+
+        Focus ring color does not honor dark mode or system accent color.
+        https://bugs.webkit.org/show_bug.cgi?id=187144
+        rdar://problem/41105081
+
+        Reviewed by Tim Horton.
+
+        Pass the focus ring color through to the GraphicsContext methods that draw it.
+
+        * platform/graphics/GraphicsContext.h:
+        * platform/graphics/cocoa/GraphicsContextCocoa.mm:
+        (WebCore::drawFocusRingAtTime):
+        (WebCore::drawFocusRing):
+        (WebCore::drawFocusRingToContext):
+        (WebCore::drawFocusRingToContextAtTime):
+        (WebCore::GraphicsContext::drawFocusRing):
+        (WebCore::GraphicsContext::focusRingColor): Deleted.
+        * platform/mac/ThemeMac.mm:
+        (WebCore::drawCellFocusRingWithFrameAtTime):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::paintFocusRing):
+        * rendering/RenderImage.cpp:
+        (WebCore::RenderImage::paintAreaElementFocusRing):
+
 2018-06-28  Aditya Keerthi  <akeerthi@apple.com>
 
         REGRESSION (r232040): Cursor jumping in Safari text fields
index 40a9691..d6ac81d 100644 (file)
@@ -442,9 +442,8 @@ public:
     void drawFocusRing(const Vector<FloatRect>&, float width, float offset, const Color&);
     void drawFocusRing(const Path&, float width, float offset, const Color&);
 #if PLATFORM(MAC)
-    void drawFocusRing(const Path&, double timeOffset, bool& needsRedraw);
-    void drawFocusRing(const Vector<FloatRect>&, double timeOffset, bool& needsRedraw);
-    static CGColorRef focusRingColor();
+    void drawFocusRing(const Path&, double timeOffset, bool& needsRedraw, const Color&);
+    void drawFocusRing(const Vector<FloatRect>&, double timeOffset, bool& needsRedraw, const Color&);
 #endif
 
     void setLineCap(LineCap);
index 5a8eb34..3fe2081 100644 (file)
@@ -60,16 +60,7 @@ namespace WebCore {
 
 #if PLATFORM(MAC)
 
-CGColorRef GraphicsContext::focusRingColor()
-{
-    static CGColorRef color = [] {
-        CGFloat colorComponents[] = { 0.5, 0.75, 1.0, 1.0 };
-        return CGColorCreate(sRGBColorSpaceRef(), colorComponents);
-    }();
-    return color;
-}
-
-static bool drawFocusRingAtTime(CGContextRef context, NSTimeInterval timeOffset)
+static bool drawFocusRingAtTime(CGContextRef context, NSTimeInterval timeOffset, const Color& color)
 {
     CGFocusRingStyle focusRingStyle;
     BOOL needsRepaint = NSInitializeCGFocusRingStyleForTime(NSFocusRingOnly, &focusRingStyle, timeOffset);
@@ -79,7 +70,7 @@ static bool drawFocusRingAtTime(CGContextRef context, NSTimeInterval timeOffset)
     // -1. According to CoreGraphics, the reasoning for this behavior has been
     // lost in time.
     focusRingStyle.accumulate = -1;
-    auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, GraphicsContext::focusRingColor()));
+    auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, cachedCGColor(color)));
 
     CGContextSaveGState(context);
     CGContextSetStyle(context, style.get());
@@ -89,24 +80,24 @@ static bool drawFocusRingAtTime(CGContextRef context, NSTimeInterval timeOffset)
     return needsRepaint;
 }
 
-static void drawFocusRing(CGContextRef context)
+static void drawFocusRing(CGContextRef context, const Color& color)
 {
-    drawFocusRingAtTime(context, std::numeric_limits<double>::max());
+    drawFocusRingAtTime(context, std::numeric_limits<double>::max(), color);
 }
 
-static void drawFocusRingToContext(CGContextRef context, CGPathRef focusRingPath)
+static void drawFocusRingToContext(CGContextRef context, CGPathRef focusRingPath, const Color& color)
 {
     CGContextBeginPath(context);
     CGContextAddPath(context, focusRingPath);
-    drawFocusRing(context);
+    drawFocusRing(context, color);
 }
 
-static bool drawFocusRingToContextAtTime(CGContextRef context, CGPathRef focusRingPath, double timeOffset)
+static bool drawFocusRingToContextAtTime(CGContextRef context, CGPathRef focusRingPath, double timeOffset, const Color& color)
 {
     UNUSED_PARAM(timeOffset);
     CGContextBeginPath(context);
     CGContextAddPath(context, focusRingPath);
-    return drawFocusRingAtTime(context, std::numeric_limits<double>::max());
+    return drawFocusRingAtTime(context, std::numeric_limits<double>::max(), color);
 }
 
 #endif // PLATFORM(MAC)
@@ -122,7 +113,7 @@ void GraphicsContext::drawFocusRing(const Path& path, float width, float offset,
         return;
     }
 
-    drawFocusRingToContext(platformContext(), path.platformPath());
+    drawFocusRingToContext(platformContext(), path.platformPath(), color);
 #else
     UNUSED_PARAM(path);
     UNUSED_PARAM(width);
@@ -132,7 +123,7 @@ void GraphicsContext::drawFocusRing(const Path& path, float width, float offset,
 }
 
 #if PLATFORM(MAC)
-void GraphicsContext::drawFocusRing(const Path& path, double timeOffset, bool& needsRedraw)
+void GraphicsContext::drawFocusRing(const Path& path, double timeOffset, bool& needsRedraw, const Color& color)
 {
     if (paintingDisabled() || path.isNull())
         return;
@@ -140,10 +131,10 @@ void GraphicsContext::drawFocusRing(const Path& path, double timeOffset, bool& n
     if (m_impl) // FIXME: implement animated focus ring drawing.
         return;
 
-    needsRedraw = drawFocusRingToContextAtTime(platformContext(), path.platformPath(), timeOffset);
+    needsRedraw = drawFocusRingToContextAtTime(platformContext(), path.platformPath(), timeOffset, color);
 }
 
-void GraphicsContext::drawFocusRing(const Vector<FloatRect>& rects, double timeOffset, bool& needsRedraw)
+void GraphicsContext::drawFocusRing(const Vector<FloatRect>& rects, double timeOffset, bool& needsRedraw, const Color& color)
 {
     if (paintingDisabled())
         return;
@@ -155,7 +146,7 @@ void GraphicsContext::drawFocusRing(const Vector<FloatRect>& rects, double timeO
     for (const auto& rect : rects)
         CGPathAddRect(focusRingPath.get(), 0, CGRect(rect));
 
-    needsRedraw = drawFocusRingToContextAtTime(platformContext(), focusRingPath.get(), timeOffset);
+    needsRedraw = drawFocusRingToContextAtTime(platformContext(), focusRingPath.get(), timeOffset, color);
 }
 #endif
 
@@ -174,7 +165,7 @@ void GraphicsContext::drawFocusRing(const Vector<FloatRect>& rects, float width,
     for (auto& rect : rects)
         CGPathAddRect(focusRingPath.get(), 0, CGRectInset(rect, -offset, -offset));
 
-    drawFocusRingToContext(platformContext(), focusRingPath.get());
+    drawFocusRingToContext(platformContext(), focusRingPath.get(), color);
 #else
     UNUSED_PARAM(rects);
     UNUSED_PARAM(width);
index cf7d6fb..b190c73 100644 (file)
@@ -362,12 +362,15 @@ static bool drawCellFocusRingWithFrameAtTime(NSCell *cell, NSRect cellFrame, NSV
 
     CGFocusRingStyle focusRingStyle;
     bool needsRepaint = NSInitializeCGFocusRingStyleForTime(NSFocusRingOnly, &focusRingStyle, timeOffset);
+
     // We want to respect the CGContext clipping and also not overpaint any
     // existing focus ring. The way to do this is set accumulate to
     // -1. According to CoreGraphics, the reasoning for this behavior has been
     // lost in time.
     focusRingStyle.accumulate = -1;
-    auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, GraphicsContext::focusRingColor()));
+
+    // FIXME: This color should be shared with RenderThemeMac. For now just use the same NSColor color.
+    auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, [NSColor keyboardFocusIndicatorColor].CGColor));
     CGContextSetStyle(cgContext, style.get());
 
     CGContextBeginTransparencyLayerWithRect(cgContext, NSRectToCGRect(cellFrame), nullptr);
index 5983b0d..75a4f75 100644 (file)
@@ -1836,9 +1836,9 @@ void RenderElement::paintFocusRing(PaintInfo& paintInfo, const RenderStyle& styl
             for (auto rect : pixelSnappedFocusRingRects)
                 path.addRect(rect);
         }
-        paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint);
+        paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(document().styleColorOptions()));
     } else
-        paintInfo.context().drawFocusRing(pixelSnappedFocusRingRects, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint);
+        paintInfo.context().drawFocusRing(pixelSnappedFocusRingRects, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(document().styleColorOptions()));
     if (needsRepaint)
         page().focusController().setFocusedElementNeedsRepaint();
 #else
index 04aa7b8..467c3de 100644 (file)
@@ -567,7 +567,7 @@ void RenderImage::paintAreaElementFocusRing(PaintInfo& paintInfo, const LayoutPo
 
 #if PLATFORM(MAC)
     bool needsRepaint;
-    paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint);
+    paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(document().styleColorOptions()));
     if (needsRepaint)
         page().focusController().setFocusedElementNeedsRepaint();
 #else