Attachment 343816 Details for Bug 187144 – Patch

[patch] Patch

bug-187144-20180628104233.patch (text/plain), 10.67 KB, created by Timothy Hatcher on 2018-06-28 10:42:36 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Timothy Hatcher

Created: 2018-06-28 10:42:36 PDT

Size: 10.67 KB

patch

obsolete

>Subversion Revision: 233297
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 0c42e42b7abb6512f831cc96a94b1fa04498f4aa..d639f67f4dcc7f6b11c24a2b6bb79f5fd159501b 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-27  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Align inFlowNonReplacedHeightAndMargin() style with the rest of the compute functions.
>diff --git a/Source/WebCore/platform/graphics/GraphicsContext.h b/Source/WebCore/platform/graphics/GraphicsContext.h
>index 40a9691fb641764792bd4cb803e77b230ab9fa47..d6ac81d4d1be81fac0ff51f9bca5c393f1de9de7 100644
>--- a/Source/WebCore/platform/graphics/GraphicsContext.h
>+++ b/Source/WebCore/platform/graphics/GraphicsContext.h
>@@ -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);
>diff --git a/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm b/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm
>index 5a8eb34cc9afa5761fbd465f8008fc8026408a15..3fe2081d5ab6fa00552fb9d3a87adb25c57a5b14 100644
>--- a/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm
>+++ b/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm
>@@ -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);
>diff --git a/Source/WebCore/platform/mac/ThemeMac.mm b/Source/WebCore/platform/mac/ThemeMac.mm
>index cf7d6fb41fe6ac94003e5732c669d37733f393ed..b190c735ec7052a636ccaa2dc509f56dea571e72 100644
>--- a/Source/WebCore/platform/mac/ThemeMac.mm
>+++ b/Source/WebCore/platform/mac/ThemeMac.mm
>@@ -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);
>diff --git a/Source/WebCore/rendering/RenderElement.cpp b/Source/WebCore/rendering/RenderElement.cpp
>index 5983b0dad93a68172c8203ef56df440a2715615a..75a4f758b8cee7ddace04d89b8dd67204a9e636a 100644
>--- a/Source/WebCore/rendering/RenderElement.cpp
>+++ b/Source/WebCore/rendering/RenderElement.cpp
>@@ -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
>diff --git a/Source/WebCore/rendering/RenderImage.cpp b/Source/WebCore/rendering/RenderImage.cpp
>index 04aa7b80e4bc2e99b5657a3a23650506b7940b0e..467c3de2c022ac3b659745c64341ba0ee0710a71 100644
>--- a/Source/WebCore/rendering/RenderImage.cpp
>+++ b/Source/WebCore/rendering/RenderImage.cpp
>@@ -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

Actions: View | Formatted Diff | Diff

Attachments on bug 187144: 343816