Title: [277452] trunk
- Revision
- 277452
- Author
- akeer...@apple.com
- Date
- 2021-05-13 14:53:00 -0700 (Thu, 13 May 2021)
Log Message
REGRESSION (r276945): [iOS] Focus rings are too large
https://bugs.webkit.org/show_bug.cgi?id=225778
<rdar://problem/77858341>
Reviewed by Tim Horton.
Source/WebCore:
r276945 updated scaling logic to ensure that the scale of the base CTM
matches the device scale factor. The change itself makes our base CTM
more correct, but exposed a longstanding bug with our focus ring
implementation.
Focus rings are drawn using CoreGraphics, using CGFocusRingStyle. On
macOS, the style is initialized using NSInitializeCGFocusRingStyleForTime.
However, on iOS, we initialize the style ourselves, using UIKit constants.
Currently, the focus ring's style's radius is set to
+[UIFocusRingStyle cornerRadius], a constant of 8. This is the longstanding
issue. CGFocusRingStyle's radius is not a corner radius, but is the
width of the focus ring. In UIKit, this width is represented by
+[UIFocusRingStyle borderThickness], a constant of 3. WebKit has always
intended to match this width, as evidenced by the default outline-width
of 3px in the UA style sheet.
Considering the large disparity between the existing and expected radius
value, it may be surprising that this mistake has gone unnoticed since
2019, when focus rings were first introduced on iOS. This is where r276945
comes into play. Prior to r276945, the scale of the base CTM did not match
the device scale factor. This meant that CG scaled down the constant of 8
to 4 CSS pixels (1 (base CTM) / 2 (device scale factor)). After r276945,
the two scales are equal and the focus ring is drawn as 8 CSS pixels, much
larger than the expected 3px.
Test: fast/forms/ios/focus-ring-size.html
* platform/graphics/cocoa/GraphicsContextCocoa.mm:
(WebCore::drawFocusRingAtTime):
To fix, use the correct SPI to get the focus ring width,
+[UIFocusRingStyle borderThickness]. This ensures that focus rings have
the right width, following r276945.
The change also fixes focus ring repaint issues that arose from the
fact that the outline-width was 3px, but the painted width was 4px.
Source/WebCore/PAL:
* pal/spi/ios/UIKitSPI.h:
LayoutTests:
Added a regression test to verify the size of the focus ring on iOS.
The test works by drawing an overlay on top of an input element, that
is just large enough to obscure the focus ring. If the focus ring is
too large, the ring will not obscured, leading to a mismatch failure.
* fast/forms/ios/focus-ring-size-expected.html: Added.
* fast/forms/ios/focus-ring-size.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (277451 => 277452)
--- trunk/LayoutTests/ChangeLog 2021-05-13 21:28:34 UTC (rev 277451)
+++ trunk/LayoutTests/ChangeLog 2021-05-13 21:53:00 UTC (rev 277452)
@@ -1,3 +1,19 @@
+2021-05-13 Aditya Keerthi <akeer...@apple.com>
+
+ REGRESSION (r276945): [iOS] Focus rings are too large
+ https://bugs.webkit.org/show_bug.cgi?id=225778
+ <rdar://problem/77858341>
+
+ Reviewed by Tim Horton.
+
+ Added a regression test to verify the size of the focus ring on iOS.
+ The test works by drawing an overlay on top of an input element, that
+ is just large enough to obscure the focus ring. If the focus ring is
+ too large, the ring will not obscured, leading to a mismatch failure.
+
+ * fast/forms/ios/focus-ring-size-expected.html: Added.
+ * fast/forms/ios/focus-ring-size.html: Added.
+
2021-05-13 Amir Mark Jr <amir_m...@apple.com>
[MacOS Wk1] media/video-playback-quality.html is a flaky failure
Added: trunk/LayoutTests/fast/forms/ios/focus-ring-size-expected.html (0 => 277452)
--- trunk/LayoutTests/fast/forms/ios/focus-ring-size-expected.html (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/focus-ring-size-expected.html 2021-05-13 21:53:00 UTC (rev 277452)
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+
+:root {
+ --focus-ring-size: 3px;
+ --input-width: 160px;
+ --input-height: 20px;
+ --border-width: 1px;
+}
+
+#overlay {
+ position: absolute;
+ top: var(--focus-ring-size);
+ left: var(--focus-ring-size);
+ background-color: red;
+ border: var(--border-width) solid red;
+ width: calc(var(--input-width) + 2 * var(--focus-ring-size));
+ height: calc(var(--input-height) + 2 * var(--focus-ring-size));
+}
+
+</style>
+</head>
+<body>
+<div id="overlay"></div>
+</body>
+</html>
Added: trunk/LayoutTests/fast/forms/ios/focus-ring-size.html (0 => 277452)
--- trunk/LayoutTests/fast/forms/ios/focus-ring-size.html (rev 0)
+++ trunk/LayoutTests/fast/forms/ios/focus-ring-size.html 2021-05-13 21:53:00 UTC (rev 277452)
@@ -0,0 +1,50 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<style>
+
+:root {
+ --focus-ring-size: 3px;
+ --input-width: 160px;
+ --input-height: 20px;
+ --border-width: 1px;
+}
+
+input {
+ caret-color: transparent;
+
+ position: absolute;
+ top: calc(2 * var(--focus-ring-size));
+ left: calc(2 * var(--focus-ring-size));
+ border: var(--border-width) solid black;
+ padding: 0;
+ width: var(--input-width);
+ height: var(--input-height);
+}
+
+#overlay {
+ position: absolute;
+ top: var(--focus-ring-size);
+ left: var(--focus-ring-size);
+ background-color: red;
+ border: var(--border-width) solid red;
+ width: calc(var(--input-width) + 2 * var(--focus-ring-size));
+ height: calc(var(--input-height) + 2 * var(--focus-ring-size));
+}
+
+</style>
+</head>
+<body>
+<input id="test" type="text">
+<div id="overlay"></div>
+<script>
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+ let testElement = document.getElementById("test");
+ testElement.addEventListener("focus", () => testRunner.notifyDone(), { once: true });
+ UIHelper.keyDown("\t", ["altKey"]);
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (277451 => 277452)
--- trunk/Source/WebCore/ChangeLog 2021-05-13 21:28:34 UTC (rev 277451)
+++ trunk/Source/WebCore/ChangeLog 2021-05-13 21:53:00 UTC (rev 277452)
@@ -1,3 +1,48 @@
+2021-05-13 Aditya Keerthi <akeer...@apple.com>
+
+ REGRESSION (r276945): [iOS] Focus rings are too large
+ https://bugs.webkit.org/show_bug.cgi?id=225778
+ <rdar://problem/77858341>
+
+ Reviewed by Tim Horton.
+
+ r276945 updated scaling logic to ensure that the scale of the base CTM
+ matches the device scale factor. The change itself makes our base CTM
+ more correct, but exposed a longstanding bug with our focus ring
+ implementation.
+
+ Focus rings are drawn using CoreGraphics, using CGFocusRingStyle. On
+ macOS, the style is initialized using NSInitializeCGFocusRingStyleForTime.
+ However, on iOS, we initialize the style ourselves, using UIKit constants.
+ Currently, the focus ring's style's radius is set to
+ +[UIFocusRingStyle cornerRadius], a constant of 8. This is the longstanding
+ issue. CGFocusRingStyle's radius is not a corner radius, but is the
+ width of the focus ring. In UIKit, this width is represented by
+ +[UIFocusRingStyle borderThickness], a constant of 3. WebKit has always
+ intended to match this width, as evidenced by the default outline-width
+ of 3px in the UA style sheet.
+
+ Considering the large disparity between the existing and expected radius
+ value, it may be surprising that this mistake has gone unnoticed since
+ 2019, when focus rings were first introduced on iOS. This is where r276945
+ comes into play. Prior to r276945, the scale of the base CTM did not match
+ the device scale factor. This meant that CG scaled down the constant of 8
+ to 4 CSS pixels (1 (base CTM) / 2 (device scale factor)). After r276945,
+ the two scales are equal and the focus ring is drawn as 8 CSS pixels, much
+ larger than the expected 3px.
+
+ Test: fast/forms/ios/focus-ring-size.html
+
+ * platform/graphics/cocoa/GraphicsContextCocoa.mm:
+ (WebCore::drawFocusRingAtTime):
+
+ To fix, use the correct SPI to get the focus ring width,
+ +[UIFocusRingStyle borderThickness]. This ensures that focus rings have
+ the right width, following r276945.
+
+ The change also fixes focus ring repaint issues that arose from the
+ fact that the outline-width was 3px, but the painted width was 4px.
+
2021-05-13 Sam Weinig <wei...@apple.com>
Split pixel buffer format data out into a new PixelBufferFormat struct
Modified: trunk/Source/WebCore/PAL/ChangeLog (277451 => 277452)
--- trunk/Source/WebCore/PAL/ChangeLog 2021-05-13 21:28:34 UTC (rev 277451)
+++ trunk/Source/WebCore/PAL/ChangeLog 2021-05-13 21:53:00 UTC (rev 277452)
@@ -1,3 +1,13 @@
+2021-05-13 Aditya Keerthi <akeer...@apple.com>
+
+ REGRESSION (r276945): [iOS] Focus rings are too large
+ https://bugs.webkit.org/show_bug.cgi?id=225778
+ <rdar://problem/77858341>
+
+ Reviewed by Tim Horton.
+
+ * pal/spi/ios/UIKitSPI.h:
+
2021-05-12 Jean-Yves Avenard <j...@apple.com>
Adopt CoreMedia SPI to identify audio-only playback for MSE clients
Modified: trunk/Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h (277451 => 277452)
--- trunk/Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h 2021-05-13 21:28:34 UTC (rev 277451)
+++ trunk/Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h 2021-05-13 21:53:00 UTC (rev 277452)
@@ -168,7 +168,7 @@
@end
@interface UIFocusRingStyle : NSObject
-+ (CGFloat)cornerRadius;
++ (CGFloat)borderThickness;
+ (CGFloat)maxAlpha;
+ (CGFloat)alphaThreshold;
@end
Modified: trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm (277451 => 277452)
--- trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm 2021-05-13 21:28:34 UTC (rev 277451)
+++ trunk/Source/WebCore/platform/graphics/cocoa/GraphicsContextCocoa.mm 2021-05-13 21:53:00 UTC (rev 277452)
@@ -74,7 +74,7 @@
focusRingStyle.tint = kCGFocusRingTintBlue;
focusRingStyle.ordering = kCGFocusRingOrderingNone;
focusRingStyle.alpha = [PAL::getUIFocusRingStyleClass() maxAlpha];
- focusRingStyle.radius = [PAL::getUIFocusRingStyleClass() cornerRadius];
+ focusRingStyle.radius = [PAL::getUIFocusRingStyleClass() borderThickness];
focusRingStyle.threshold = [PAL::getUIFocusRingStyleClass() alphaThreshold];
focusRingStyle.bounds = CGRectZero;
#endif
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
