Branch: refs/heads/main Home: https://github.com/WebKit/WebKit Commit: 3d8bc4adc4366c9b57b2aa50e2c62c3056b9c2a5 https://github.com/WebKit/WebKit/commit/3d8bc4adc4366c9b57b2aa50e2c62c3056b9c2a5 Author: Aditya Keerthi <akeer...@apple.com> Date: 2023-01-25 (Wed, 25 Jan 2023)
Changed paths: M LayoutTests/accessibility/node-only-object-element-rect-expected.txt M LayoutTests/platform/mac/compositing/contents-opaque/control-layer-expected.txt M Source/WebCore/platform/graphics/mac/controls/ButtonMac.mm M Source/WebCore/platform/mac/ThemeMac.mm Log Message: ----------- REGRESSION: File input button layout looks bad at various zoom levels https://bugs.webkit.org/show_bug.cgi?id=251065 rdar://104197684 Reviewed by Tim Nguyen. On macOS, at different zoom levels, the “Choose File” text in the file input button is poorly aligned relative to the button outline. This bug is the consequence of three distinct changes, each of which are described below. However, in order to understand the individual causes, a baseline understanding of form control rendering on macOS must be established. Form controls on macOS are painted by drawing `NSCell`s vended from AppKit. This approach is essential in ensuring that WebKit form controls have a native look and feel. A major limitation of AppKit controls is that they are almost all backed by fixed-size bitmaps, whereas form controls on the web have no inherent sizing limitations. AppKit defines four fixed sizes, `NSControlSizeRegular`, `NSControlSizeSmall`, `NSControlSizeMini`, and `NSControlSizeLarge`. WebKit takes various approaches to reconcile fixed AppKit control sizes with arbitrary element sizes. In all cases, the `NSControlSize` corresponding to the fixed size closest to the element's box size (adjusted for zoom) is determined. Buttons have historically done one of two things with a given `NSControlSize`: 1. Adjust (force) the element's size to match the fixed size. 2. Paint the `NSCell` in the center of the element's box. Until 258754@main, `<input type=button>` (and file input buttons) used the first approach, while `<button>` used the second. 258754@main unified `<input type=button>` and `<button`> styling/painting, using the second approach, to resolve numerous web compatibility issues. In doing so, all bugs with `<button>` painting were also brought to file input buttons. While 258754@main regressed file input buttons, it is not the root cause of the issue. The root cause of the issue revolves around how the bitmap image vended from AppKit is centered in the element's box. Specifically, the frame (rect) used to draw an `NSCell` is not equivalent to the size of the bitmap. The actual image is inset from the frame. Consequently, in order to paint the button at a particular size/position, the frame passed to AppKit must be inflated. WebKit currently achieves this by using a hardcoded list of cell sizes and outsets. However, these values are long out-of-date, as macOS theme definitions have evolved. The result of incorrect outsets is that the rendered position of the button outline does not align with the text. Some outset values (for a given `NSControlSize`) are more accurate than others. When the zoom level changes the used `NSControlSize` can also change, resulting in an effect where it appears as though the text moves relative to the button. To fix this issue, WebKit needs to use correct values for the outsets, so that the button outline is drawn at the intended position. However, fixing the outsets revealed yet another regression in the logic responsible for determining the "inflated rect" `NSButtonCell`s. 258492@main moved button painting code from `ThemeMac` into `ButtonMac`, but made two errors: 1. The use of `auto` to store the cell size prior to scaling by the zoom factor. Since cell sizes are integer values, the scaled height ends up as an integer, which throws off the position of the renderered bitmap. Before 258492@main, the logic explicitly used `FloatSize`. 2. The use of `expand` to inflate the rect. `expand` does not adjust the position of the rect – it merely increases the size. Consequently, the position of the rect is not adjusted to center it. Before 258492@main, the logic used `setY` and `setHeight`. In summary, AppKit changes, 258492@main, and 258754@main all combined to result in file input button layout looking bad at various zoom levels. * LayoutTests/accessibility/node-only-object-element-rect-expected.txt: Rebaseline. * LayoutTests/platform/mac/compositing/contents-opaque/control-layer-expected.txt: Rebaseline. * Source/WebCore/platform/graphics/mac/controls/ButtonMac.mm: (WebCore::ButtonMac::cellSize const): Use updated values from AppKit. (WebCore::ButtonMac::cellOutsets const): Use updated values from AppKit. These should be determined programmatically once the `ControlPart` refactor is complete, and an additional refactor to use `NSControl`s (that own `NSCell`s) can be made. (WebCore::ButtonMac::rectForBounds const): Fix the two logic errors described above. Explicitly use `FloatSize` to avoid clamping to integers and use `inflateY` to ensure the position of the inflated rect is correct. * Source/WebCore/platform/mac/ThemeMac.mm: Use updated values from AppKit. This code duplication will eventually be removed, once the ongoing `ControlPart` refactor is complete, but for now, the values need to be updated to adjust the repaint rect. (WebCore::buttonSizes): (WebCore::buttonMargins): Canonical link: https://commits.webkit.org/259359@main _______________________________________________ webkit-changes mailing list webkit-changes@lists.webkit.org https://lists.webkit.org/mailman/listinfo/webkit-changes