Diff
Modified: trunk/Source/WebCore/ChangeLog (239172 => 239173)
--- trunk/Source/WebCore/ChangeLog 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/ChangeLog 2018-12-13 19:56:44 UTC (rev 239173)
@@ -1,3 +1,36 @@
+2018-12-13 Timothy Hatcher <[email protected]>
+
+ REGRESSION (r230064): Focus rings on webpages are fainter than in native UI.
+ https://bugs.webkit.org/show_bug.cgi?id=192639
+ rdar://problem/42669297
+
+ Reviewed by Tim Horton.
+
+ The focus ring color passed to CoreGraphics is expected to be opaque, since they
+ will apply opacity when drawing (because opacity is normally animated).
+ We were getting this by accident before when the old `RenderThemeMac::systemColor()`
+ used the old `convertNSColorToColor()`, which ignored alpha on NSColor.
+ Existing tests use fixed test focus ring color.
+
+ * css/StyleResolver.cpp:
+ (WebCore::StyleResolver::colorFromPrimitiveValue const): Use RenderTheme singleton for `focusRingColor()`.
+ * html/canvas/CanvasRenderingContext2D.cpp:
+ (WebCore::CanvasRenderingContext2D::drawFocusIfNeededInternal): Ditto.
+ * platform/graphics/cocoa/GraphicsContextCocoa.mm:
+ (WebCore::drawFocusRingAtTime): Use `CGContextStateSaver`.
+ * platform/mac/ThemeMac.mm:
+ (WebCore::drawCellFocusRingWithFrameAtTime): Force alpha to 1 on the focus ring color. Use `CGContextStateSaver`.
+ * rendering/RenderElement.cpp:
+ (WebCore::RenderElement::paintFocusRing): Use RenderTheme singleton for `focusRingColor()`.
+ * rendering/RenderImage.cpp:
+ (WebCore::RenderImage::paintAreaElementFocusRing): Ditto.
+ * rendering/RenderTheme.cpp:
+ (WebCore::RenderTheme::focusRingColor const): Made const. Cache the result of `platformFocusRingColor()`.
+ * rendering/RenderTheme.h: Made `focusRingColor()` a member function instead of static.
+ * rendering/RenderThemeMac.mm:
+ (WebCore::RenderThemeMac::platformFocusRingColor const): Force alpha to 1 on the focus ring color.
+ (WebCore::RenderThemeMac::systemColor const): Use `focusRingColor()`, instead of caching color here.
+
2018-12-13 Eric Carlson <[email protected]>
[MediaStream] Calculate width or height when constraints contain only the other
Modified: trunk/Source/WebCore/css/StyleResolver.cpp (239172 => 239173)
--- trunk/Source/WebCore/css/StyleResolver.cpp 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/css/StyleResolver.cpp 2018-12-13 19:56:44 UTC (rev 239173)
@@ -1865,7 +1865,7 @@
case CSSValueWebkitActivelink:
return document().activeLinkColor();
case CSSValueWebkitFocusRingColor:
- return RenderTheme::focusRingColor(document().styleColorOptions(m_state.style()));
+ return RenderTheme::singleton().focusRingColor(document().styleColorOptions(m_state.style()));
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?
Modified: trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp (239172 => 239173)
--- trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp 2018-12-13 19:56:44 UTC (rev 239173)
@@ -86,7 +86,7 @@
auto* context = drawingContext();
if (!element.focused() || !state().hasInvertibleTransform || path.isEmpty() || !element.isDescendantOf(canvas()) || !context)
return;
- context->drawFocusRing(path, 1, 1, RenderTheme::focusRingColor(element.document().styleColorOptions(canvas().computedStyle())));
+ context->drawFocusRing(path, 1, 1, RenderTheme::singleton().focusRingColor(element.document().styleColorOptions(canvas().computedStyle())));
}
String CanvasRenderingContext2D::font() const
Modified: trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm (239172 => 239173)
--- trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm 2018-12-13 19:56:44 UTC (rev 239173)
@@ -73,10 +73,10 @@
focusRingStyle.accumulate = -1;
auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, cachedCGColor(color)));
- CGContextSaveGState(context);
+ CGContextStateSaver stateSaver(context);
+
CGContextSetStyle(context, style.get());
CGContextFillPath(context);
- CGContextRestoreGState(context);
return needsRepaint;
}
Modified: trunk/Source/WebCore/platform/mac/ThemeMac.mm (239172 => 239173)
--- trunk/Source/WebCore/platform/mac/ThemeMac.mm 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/platform/mac/ThemeMac.mm 2018-12-13 19:56:44 UTC (rev 239173)
@@ -29,8 +29,10 @@
#if PLATFORM(MAC)
#import "AXObjectCache.h"
+#import "ColorMac.h"
#import "ControlStates.h"
#import "GraphicsContext.h"
+#import "GraphicsContextCG.h"
#import "ImageBuffer.h"
#import "LengthSize.h"
#import "LocalCurrentGraphicsContext.h"
@@ -361,8 +363,9 @@
ALLOW_DEPRECATED_DECLARATIONS_BEGIN
CGContextRef cgContext = (CGContextRef)[[NSGraphicsContext currentContext] graphicsPort];
ALLOW_DEPRECATED_DECLARATIONS_END
- CGContextSaveGState(cgContext);
+ CGContextStateSaver stateSaver(cgContext);
+
CGFocusRingStyle focusRingStyle;
bool needsRepaint = NSInitializeCGFocusRingStyleForTime(NSFocusRingOnly, &focusRingStyle, timeOffset);
@@ -373,13 +376,14 @@
focusRingStyle.accumulate = -1;
// FIXME: This color should be shared with RenderThemeMac. For now just use the same NSColor color.
- auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, [NSColor keyboardFocusIndicatorColor].CGColor));
+ // The color is expected to be opaque, since CoreGraphics will apply opacity when drawing (because opacity is normally animated).
+ auto color = colorWithOverrideAlpha(colorFromNSColor([NSColor keyboardFocusIndicatorColor]).rgb(), 1);
+ auto style = adoptCF(CGStyleCreateFocusRingWithColor(&focusRingStyle, cachedCGColor(color)));
CGContextSetStyle(cgContext, style.get());
CGContextBeginTransparencyLayerWithRect(cgContext, NSRectToCGRect(cellFrame), nullptr);
[cell drawFocusRingMaskWithFrame:cellFrame inView:controlView];
CGContextEndTransparencyLayer(cgContext);
- CGContextRestoreGState(cgContext);
return needsRepaint;
}
Modified: trunk/Source/WebCore/rendering/RenderElement.cpp (239172 => 239173)
--- trunk/Source/WebCore/rendering/RenderElement.cpp 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/rendering/RenderElement.cpp 2018-12-13 19:56:44 UTC (rev 239173)
@@ -1832,9 +1832,9 @@
for (auto rect : pixelSnappedFocusRingRects)
path.addRect(rect);
}
- paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(styleColorOptions()));
+ paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::singleton().focusRingColor(styleColorOptions()));
} else
- paintInfo.context().drawFocusRing(pixelSnappedFocusRingRects, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(styleColorOptions()));
+ paintInfo.context().drawFocusRing(pixelSnappedFocusRingRects, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::singleton().focusRingColor(styleColorOptions()));
if (needsRepaint)
page().focusController().setFocusedElementNeedsRepaint();
#else
Modified: trunk/Source/WebCore/rendering/RenderImage.cpp (239172 => 239173)
--- trunk/Source/WebCore/rendering/RenderImage.cpp 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/rendering/RenderImage.cpp 2018-12-13 19:56:44 UTC (rev 239173)
@@ -592,7 +592,7 @@
#if PLATFORM(MAC)
bool needsRepaint;
- paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::focusRingColor(styleColorOptions()));
+ paintInfo.context().drawFocusRing(path, page().focusController().timeSinceFocusWasSet().seconds(), needsRepaint, RenderTheme::singleton().focusRingColor(styleColorOptions()));
if (needsRepaint)
page().focusController().setFocusedElementNeedsRepaint();
#else
Modified: trunk/Source/WebCore/rendering/RenderTheme.cpp (239172 => 239173)
--- trunk/Source/WebCore/rendering/RenderTheme.cpp 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/rendering/RenderTheme.cpp 2018-12-13 19:56:44 UTC (rev 239173)
@@ -1411,9 +1411,15 @@
customFocusRingColor() = color;
}
-Color RenderTheme::focusRingColor(OptionSet<StyleColor::Options> options)
+Color RenderTheme::focusRingColor(OptionSet<StyleColor::Options> options) const
{
- return customFocusRingColor().isValid() ? customFocusRingColor() : RenderTheme::singleton().platformFocusRingColor(options);
+ if (customFocusRingColor().isValid())
+ return customFocusRingColor();
+
+ auto& cache = colorCache(options);
+ if (!cache.systemFocusRingColor.isValid())
+ cache.systemFocusRingColor = platformFocusRingColor(options);
+ return cache.systemFocusRingColor;
}
String RenderTheme::fileListDefaultLabel(bool multipleFilesAllowed) const
Modified: trunk/Source/WebCore/rendering/RenderTheme.h (239172 => 239173)
--- trunk/Source/WebCore/rendering/RenderTheme.h 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/rendering/RenderTheme.h 2018-12-13 19:56:44 UTC (rev 239173)
@@ -161,7 +161,7 @@
virtual Color disabledTextColor(const Color& textColor, const Color& backgroundColor) const;
- static Color focusRingColor(OptionSet<StyleColor::Options>);
+ Color focusRingColor(OptionSet<StyleColor::Options>) const;
virtual Color platformFocusRingColor(OptionSet<StyleColor::Options>) const { return Color(0, 0, 0); }
static void setCustomFocusRingColor(const Color&);
static float platformFocusRingWidth() { return 3; }
Modified: trunk/Source/WebCore/rendering/RenderThemeMac.mm (239172 => 239173)
--- trunk/Source/WebCore/rendering/RenderThemeMac.mm 2018-12-13 19:10:33 UTC (rev 239172)
+++ trunk/Source/WebCore/rendering/RenderThemeMac.mm 2018-12-13 19:56:44 UTC (rev 239173)
@@ -489,7 +489,9 @@
{
if (usesTestModeFocusRingColor())
return oldAquaFocusRingColor();
- return systemColor(CSSValueWebkitFocusRingColor, options);
+ LocalDefaultSystemAppearance localAppearance(options.contains(StyleColor::Options::UseDarkAppearance));
+ // The color is expected to be opaque, since CoreGraphics will apply opacity when drawing (because opacity is normally animated).
+ return colorWithOverrideAlpha(colorFromNSColor([NSColor keyboardFocusIndicatorColor]).rgb(), 1);
}
Color RenderThemeMac::platformActiveTextSearchHighlightColor(OptionSet<StyleColor::Options> options) const
@@ -654,7 +656,7 @@
// These should only be available when the web view is wanting the system appearance.
case CSSValueWebkitFocusRingColor:
case CSSValueActiveborder:
- return systemAppearanceColor(cache.systemFocusRingColor, @selector(keyboardFocusIndicatorColor));
+ return focusRingColor(options);
case CSSValueAppleSystemControlAccent:
#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400
@@ -841,8 +843,8 @@
case CSSValueActiveborder:
// Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
if (localAppearance.usingDarkAppearance())
- return Color(0x4C1AA9FF, Color::Semantic);
- return Color(0x3F0067F4, Color::Semantic);
+ return Color(0xFF1AA9FF, Color::Semantic);
+ return Color(0xFF0067F4, Color::Semantic);
case CSSValueAppleSystemControlAccent:
// Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
