Skip to content

Commit

Permalink
[macOS] drawFocusIfNeeded() should not expose the user's system accen…
Browse files Browse the repository at this point in the history
…t color

https://bugs.webkit.org/show_bug.cgi?id=260102
rdar://105554669

Reviewed by Tim Nguyen.

On macOS, `drawFocusIfNeeded()` currently exposes the user's system accent color via
`RenderTheme::focusRingColor()`. To mitigate fingerprinting risk due to this API for users that have
chosen a non-default system accent color, we make `RenderTheme::focusRingColor()` respect the given
`UseSystemAppearance` state by returning the default system focus ring color on macOS, in the case
where that option is absent. As a result, this means that quirks-mode webpages that use
`-webkit-focus-ring-color` will also no longer be able to determine the user's accent color. This
aligns with existing behavior for the "activeborder" CSS value.

Tests:  fast/canvas/do-not-expose-non-default-focus-ring-color.html
        fast/css/mac/focus-ring-color-should-not-expose-accent-color.html

* LayoutTests/TestExpectations:
* LayoutTests/fast/canvas/do-not-expose-non-default-focus-ring-color-expected.html: Added.
* LayoutTests/fast/canvas/do-not-expose-non-default-focus-ring-color.html: Added.
* LayoutTests/fast/canvas/resources/do-not-expose-non-default-focus-ring-color.js: Added.
(paintIntoSwatch):

Add a test to verify that accent colors can't be read back using canvas 2D; to test this, we render
a simple focus ring to a 2D canvas, use `getImageData` to read it back, and verify that the average
non-transparent pixel values in the resulting image data match even when the accent color is
different (customized using the new `UIScriptController` hook below).

* LayoutTests/fast/css/mac/focus-ring-color-should-not-expose-accent-color-expected-mismatch.html: Added.
* LayoutTests/fast/css/mac/focus-ring-color-should-not-expose-accent-color.html: Added.

Add another test to verify that accent colors (1) are not directly leaked through the use of the
`-webkit-focus-ring-color` CSS property, and (2) enabling system appearance is sufficient to expose
the real focus ring color again.

* LayoutTests/platform/mac-wk2/TestExpectations:
* LayoutTests/resources/ui-helper.js:
(window.UIHelper.isMac):
(window.UIHelper.setAppAccentColor):
* Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h:

Drive-by fix: remove an unnecessary UIKit SPI method declaraction.

* Source/WebCore/rendering/RenderElement.cpp:
(WebCore::RenderElement::paintFocusRing const):

Set `UseSystemAppearance` here to ensure that focus rings still paint with the correct appearance.

* Source/WebCore/rendering/RenderImage.cpp:
(WebCore::RenderImage::paintAreaElementFocusRing):

Set `UseSystemAppearance` here to ensure that focus rings still paint with the correct appearance.

* Source/WebCore/rendering/RenderThemeIOS.mm:
(WebCore::RenderThemeIOS::systemFocusRingColor):
* Source/WebCore/rendering/RenderThemeMac.mm:
(WebCore::defaultFocusRingColor):
(WebCore::RenderThemeMac::platformFocusRingColor const):

This is the main fix — pull the hard-coded value for the focus ring color out into a separate helper
function, which we use in `platformFocusRingColor` if `UseSystemAppearance` is unset.

(WebCore::RenderThemeMac::systemColor const):
* Source/WebCore/testing/Internals.cpp:
(WebCore::Internals::focusRingColor):
* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::setAppAccentColor):

Add a `UIScriptController` hook to set a custom accent color, using `-_setAccentColor:`. This is
reset to the default value (computed upon initializing the test runner and stored in
`m_defaultAppAccentColor`) between test runs.

* Tools/WebKitTestRunner/InjectedBundle/Bindings/TestRunner.idl:
* Tools/WebKitTestRunner/InjectedBundle/TestRunner.h:
(WTR::TestRunner::isMac const):
* Tools/WebKitTestRunner/TestController.h:
* Tools/WebKitTestRunner/mac/TestControllerMac.mm:
(WTR::TestController::platformInitialize):
(WTR::TestController::platformResetStateToConsistentValues):
* Tools/WebKitTestRunner/mac/UIScriptControllerMac.h:
* Tools/WebKitTestRunner/mac/UIScriptControllerMac.mm:
(WTR::UIScriptControllerMac::setAppAccentColor):

Canonical link: https://commits.webkit.org/266881@main
  • Loading branch information
whsieh committed Aug 14, 2023
1 parent afae26e commit 04c640b
Show file tree
Hide file tree
Showing 22 changed files with 303 additions and 10 deletions.
1 change: 1 addition & 0 deletions LayoutTests/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ tiled-drawing [ Skip ]
fast/attachment/cocoa [ Skip ]
fast/attachment/mac [ Skip ]
fast/css/ios [ Skip ]
fast/css/mac [ Skip ]
fast/css/watchos [ Skip ]
fast/dom/Orientation/no-orientation-change-event-when-unparenting-view.html [ Skip ]
fast/forms/date/date-editable-components [ Skip ]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
<!DOCTYPE html> <!-- webkit-test-runner [ runSingly=true ] -->
<html>
<head>
<script src="../../resources/ui-helper.js"></script>
<style>
html {
font-size: 16px;
}

canvas {
width: 20px;
height: 20px;
opacity: 0;
}

#swatch {
width: 160px;
height: 100px;
line-height: 100px;
color: white;
text-align: center;
}
</style>
</head>
<body>
<p>
Your system accent color should not be directly exposed:
<div id="swatch"></div>
</p>
<canvas width="40" height="40" style="border: 1px solid lightgray;">
<button tabindex="0"></button>
</canvas>
<script src="./resources/do-not-expose-non-default-focus-ring-color.js"></script>
<script>
if (window.testRunner)
testRunner.waitUntilDone();

(async function() {
if (window.testRunner)
await UIHelper.setAppAccentColor(0, 100, 255);

paintIntoSwatch();

if (window.testRunner)
testRunner.notifyDone();
})();
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<!DOCTYPE html> <!-- webkit-test-runner [ runSingly=true ] -->
<html>
<head>
<script src="../../resources/ui-helper.js"></script>
<style>
html {
font-size: 16px;
}

canvas {
width: 20px;
height: 20px;
opacity: 0;
}

#swatch {
width: 160px;
height: 100px;
line-height: 100px;
color: white;
text-align: center;
}
</style>
</head>
<body>
<p>
<!-- To manually run this test, set a non-default system accent color, open this test page, and
verify that the accent color isn't directly revealed to the page by checking the output in
the color swatch below -->
Your system accent color should not be directly exposed:
<div id="swatch"></div>
</p>
<canvas width="40" height="40" style="border: 1px solid lightgray;">
<button tabindex="0"></button>
</canvas>
<script src="./resources/do-not-expose-non-default-focus-ring-color.js"></script>
<script>
if (window.testRunner)
testRunner.waitUntilDone();

(async function () {
if (window.testRunner)
await UIHelper.setAppAccentColor(255, 100, 50);

paintIntoSwatch();

if (window.testRunner)
testRunner.notifyDone();
})();
</script>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
function paintIntoSwatch() {
const canvas = document.querySelector("canvas");
const context = canvas.getContext("2d");
context.save();
context.clearRect(0, 0, 40, 40);

const button = canvas.querySelector("button");
context.beginPath();
context.rect(10, 10, 20, 20);
button.focus();
context.drawFocusIfNeeded(button);
button.blur();

context.restore();

let averageRed = 0;
let averageGreen = 0;
let averageBlue = 0;
let pixelCount = 0;

const imageData = context.getImageData(0, 0, canvas.width, canvas.height).data;
for (let row = 0; row < 40; row++) {
for (let column = 0; column < 40; column++) {
const byteOffset = 4 * (row * 40 + column);
const alphaComponent = imageData.at(byteOffset + 3);
if (!alphaComponent)
continue;

const redComponent = imageData.at(byteOffset);
const greenComponent = imageData.at(byteOffset + 1);
const blueComponent = imageData.at(byteOffset + 2);

averageRed += redComponent;
averageGreen += greenComponent;
averageBlue += blueComponent;
pixelCount++;
}
}

averageRed /= pixelCount;
averageGreen /= pixelCount;
averageBlue /= pixelCount;

const colorComponents = `${Math.round(averageRed)}, ${Math.round(averageGreen)}, ${Math.round(averageBlue)}`;
const swatch = document.getElementById("swatch");
swatch.textContent = colorComponents;
swatch.style.backgroundColor = `rgb(${colorComponents})`;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<html> <!-- webkit-test-runner [ runSingly=true ] -->
<head>
<script src="../../../resources/ui-helper.js"></script>
<style>
body {
font-family: system-ui;
}

#focus-ring {
border-radius: 4px;
margin: 4px;
width: 200px;
height: 200px;
font-size: 16px;
text-align: center;
line-height: 200px;
color: white;
}
</style>
</head>
<body>
<div id="focus-ring"></div>
<script>
function runTest() {
let container = document.getElementById("focus-ring");
container.style.backgroundColor = "-webkit-focus-ring-color";
container.textContent = getComputedStyle(container).backgroundColor;
}

if (window.testRunner) {
testRunner.waitUntilDone();
(async function() {
await UIHelper.setAppAccentColor(230, 110, 2);
internals.setUseSystemAppearance(true);
runTest();
testRunner.notifyDone();
})();
} else
runTest();
</script>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
<html> <!-- webkit-test-runner [ runSingly=true ] -->
<head>
<script src="../../../resources/ui-helper.js"></script>
<style>
body {
font-family: system-ui;
}

#focus-ring {
border-radius: 4px;
margin: 4px;
width: 200px;
height: 200px;
font-size: 16px;
text-align: center;
line-height: 200px;
color: white;
}
</style>
</head>
<body>
<div id="focus-ring"></div>
<script>
function runTest() {
let container = document.getElementById("focus-ring");
container.style.backgroundColor = "-webkit-focus-ring-color";
container.textContent = getComputedStyle(container).backgroundColor;
}

if (window.testRunner) {
testRunner.waitUntilDone();
(async function() {
await UIHelper.setAppAccentColor(230, 110, 2);
internals.setUseSystemAppearance(false);
runTest();
testRunner.notifyDone();
})();
} else
runTest();
</script>
</html>
1 change: 1 addition & 0 deletions LayoutTests/platform/mac-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ editing/undo-manager [ Pass ]
editing/pasteboard/dom-paste [ Pass ]
fast/attachment/cocoa [ Pass ]
fast/attachment/mac [ Pass ]
fast/css/mac [ Pass ]
fast/events/cursors [ Pass ]
fast/forms/date/date-editable-components [ Pass ]
fast/forms/datetimelocal/datetimelocal-editable-components [ Pass ]
Expand Down
17 changes: 17 additions & 0 deletions LayoutTests/resources/ui-helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ window.UIHelper = class UIHelper {
return testRunner.isIOSFamily;
}

static isMac()
{
return testRunner.isMac;
}

static isWebKit2()
{
return testRunner.isWebKit2;
Expand Down Expand Up @@ -1960,6 +1965,18 @@ window.UIHelper = class UIHelper {
return new Promise(resolve => testRunner.runUIScript(script, resolve));
}

static setAppAccentColor(red, green, blue)
{
if (!this.isWebKit2() || !this.isMac())
return Promise.resolve();

return new Promise(resolve => {
testRunner.runUIScript(`(() => {
uiController.setAppAccentColor(${red}, ${green}, ${blue});
})()`, resolve);
});
}

static addChromeInputField()
{
return new Promise(resolve => testRunner.addChromeInputField(resolve));
Expand Down
1 change: 0 additions & 1 deletion Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ typedef NS_ENUM(NSInteger, _UIDataOwner) {
#endif // USE(APPLE_INTERNAL_SDK)

@interface UIColor (IPI)
+ (UIColor *)keyboardFocusIndicatorColor;
+ (UIColor *)tableCellDefaultSelectionTintColor;
@end

Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/rendering/RenderElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1863,7 +1863,9 @@ void RenderElement::paintFocusRing(const PaintInfo& paintInfo, const RenderStyle
rect.inflate(outlineOffset);
pixelSnappedFocusRingRects.append(snapRectToDevicePixels(rect, deviceScaleFactor));
}
Color focusRingColor = usePlatformFocusRingColorForOutlineStyleAuto() ? RenderTheme::singleton().focusRingColor(styleColorOptions()) : style.visitedDependentColorWithColorFilter(CSSPropertyOutlineColor);
auto styleOptions = styleColorOptions();
styleOptions.add(StyleColorOptions::UseSystemAppearance);
auto focusRingColor = usePlatformFocusRingColorForOutlineStyleAuto() ? RenderTheme::singleton().focusRingColor(styleOptions) : style.visitedDependentColorWithColorFilter(CSSPropertyOutlineColor);
if (useShrinkWrappedFocusRingForOutlineStyleAuto() && style.hasBorderRadius()) {
Path path = PathUtilities::pathWithShrinkWrappedRectsForOutline(pixelSnappedFocusRingRects, style.border(), outlineOffset, style.direction(), style.writingMode(),
document().deviceScaleFactor());
Expand Down
4 changes: 3 additions & 1 deletion Source/WebCore/rendering/RenderImage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -667,7 +667,9 @@ void RenderImage::paintAreaElementFocusRing(PaintInfo& paintInfo, const LayoutPo
path.translate(toFloatSize(adjustedOffset));

#if PLATFORM(MAC)
paintInfo.context().drawFocusRing(path, outlineWidth, RenderTheme::singleton().focusRingColor(styleColorOptions()));
auto styleOptions = styleColorOptions();
styleOptions.add(StyleColorOptions::UseSystemAppearance);
paintInfo.context().drawFocusRing(path, outlineWidth, RenderTheme::singleton().focusRingColor(styleOptions));
#else
paintInfo.context().drawFocusRing(path, outlineWidth, areaElementStyle->visitedDependentColorWithColorFilter(CSSPropertyOutlineColor));
#endif // PLATFORM(MAC)
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/rendering/RenderThemeIOS.mm
Original file line number Diff line number Diff line change
Expand Up @@ -1353,7 +1353,7 @@ static bool shouldUseConvexGradient(const Color& backgroundColor)
Color RenderThemeIOS::systemFocusRingColor()
{
if (!cachedFocusRingColor().has_value()) {
// FIXME: Should be using -keyboardFocusIndicatorColor. For now, work around <rdar://problem/50838886>.
// FIXME: Should be using +keyboardFocusIndicatorColor. For now, work around <rdar://problem/50838886>.
cachedFocusRingColor() = colorFromCocoaColor([PAL::getUIColorClass() systemBlueColor]);
}
return *cachedFocusRingColor();
Expand Down
18 changes: 13 additions & 5 deletions Source/WebCore/rendering/RenderThemeMac.mm
Original file line number Diff line number Diff line change
Expand Up @@ -346,10 +346,21 @@ - (void)systemColorsDidChange:(NSNotification *)notification
#endif
}

inline static Color defaultFocusRingColor(OptionSet<StyleColorOptions> options)
{
// Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
return {
options.contains(StyleColorOptions::UseDarkAppearance) ? SRGBA<uint8_t> { 26, 169, 255 } : SRGBA<uint8_t> { 0, 103, 244 },
Color::Flags::Semantic
};
}

Color RenderThemeMac::platformFocusRingColor(OptionSet<StyleColorOptions> options) const
{
if (usesTestModeFocusRingColor())
return oldAquaFocusRingColor();
if (!options.contains(StyleColorOptions::UseSystemAppearance))
return defaultFocusRingColor(options);
LocalDefaultSystemAppearance localAppearance(options.contains(StyleColorOptions::UseDarkAppearance));
// The color is expected to be opaque, since CoreGraphics will apply opacity when drawing (because opacity is normally animated).
return colorFromCocoaColor([NSColor keyboardFocusIndicatorColor]).opaqueColor();
Expand Down Expand Up @@ -460,7 +471,7 @@ static Color activeButtonTextColor()
return systemAppearanceColor(cache.systemActiveLinkColor, @selector(systemRedColor));

// The following colors would expose user appearance preferences to the web, and could be used for fingerprinting.
// These should only be available when the web view is wanting the system appearance.
// These are available only when the web view opts into the system appearance.
case CSSValueWebkitFocusRingColor:
case CSSValueActiveborder:
return focusRingColor(options);
Expand Down Expand Up @@ -666,10 +677,7 @@ static Color activeButtonTextColor()

case CSSValueWebkitFocusRingColor:
case CSSValueActiveborder:
// Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
if (localAppearance.usingDarkAppearance())
return { SRGBA<uint8_t> { 26, 169, 255 }, Color::Flags::Semantic };
return { SRGBA<uint8_t> { 0, 103, 244 }, Color::Flags::Semantic };
return defaultFocusRingColor(options);

case CSSValueAppleSystemControlAccent:
// Hardcoded to avoid exposing a user appearance preference to the web for fingerprinting.
Expand Down
2 changes: 1 addition & 1 deletion Source/WebCore/testing/Internals.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -6750,7 +6750,7 @@ bool Internals::supportsPictureInPicture()

String Internals::focusRingColor()
{
return serializationForCSS(RenderTheme::singleton().focusRingColor({ }));
return serializationForCSS(RenderTheme::singleton().focusRingColor(StyleColorOptions::UseSystemAppearance));
}

ExceptionOr<unsigned> Internals::createSleepDisabler(const String& reason, bool display)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,8 @@ interface UIScriptController {

undefined setSafeAreaInsets(double top, double right, double bottom, double left);

undefined setAppAccentColor(unsigned short red, unsigned short green, unsigned short blue);

undefined firstResponderSuppressionForWebView(boolean shouldSuppress);
undefined makeWindowContentViewFirstResponder();
readonly attribute boolean isWindowContentViewFirstResponder;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,7 @@ class UIScriptController : public JSWrappable {
virtual JSObjectRef inputViewBounds() const { notImplemented(); return nullptr; }
virtual void activateDataListSuggestion(unsigned, JSValueRef) { notImplemented(); }
virtual void setSelectedColorForColorPicker(double, double, double) { notImplemented(); }
virtual void setAppAccentColor(unsigned short, unsigned short, unsigned short) { notImplemented(); }

// Find in Page

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
interface TestRunner {
readonly attribute boolean isWebKit2;
readonly attribute boolean isIOSFamily;
readonly attribute boolean isMac;
readonly attribute boolean isKeyboardImmediatelyAvailable;

// The basics.
Expand Down
Loading

0 comments on commit 04c640b

Please sign in to comment.