The top and bottom borders, as well as the top and bottom of the focus ring, are clipped when a search field is zoomed in.
<rdar://problem/66161781>
Created attachment 406476 [details] Patch
Created attachment 406488 [details] Patch
Comment on attachment 406488 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=406488&action=review > Source/WebCore/rendering/RenderThemeMac.mm:1999 > + NSControlSize controlSize = controlSizeForFont(style); > + setFontFromControlSize(style, controlSize); I know you just moved this, but I suggest doing this in one line without a local variable. > LayoutTests/ChangeLog:15 > + * fast/forms/search/search-zoom-rendering-expected.txt: Added. > + * fast/forms/search/search-zoom-rendering.html: Added. > + * platform/ios-wk2/fast/forms/search/search-zoom-rendering-expected.txt: Added. > + * platform/mac-mojave/fast/forms/search/search-zoom-rendering-expected.txt: Added. > + * platform/win/fast/forms/search/search-zoom-rendering-expected.txt: Added. It’s really undesirable to have another render-tree-dumping test like this. Is there any way we can test this without a render tree dump? One thing to think about is that the render tree dump doesn’t actually check the clipping, only the geometry. Since that’s all we need to check, maybe we can do a reftest that explicitly sets the size? Or perhaps we can do a dumpAsText test that explicitly queries the height? We can land like this, but these are really hard to maintain over time. I’m particularly puzzled by the seemingly random set of expectation files. For example, why does legacy WebKit testing on iOS use the shared master expectation, but modern WebKit testing on iOS use its own. Maybe that file should be in platform/ios instead of in platform/ios-wk2?
Created attachment 406494 [details] Patch for landing
(In reply to Darin Adler from comment #4) > Comment on attachment 406488 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=406488&action=review > > > Source/WebCore/rendering/RenderThemeMac.mm:1999 > > + NSControlSize controlSize = controlSizeForFont(style); > > + setFontFromControlSize(style, controlSize); > > I know you just moved this, but I suggest doing this in one line without a > local variable. 👍 > > LayoutTests/ChangeLog:15 > > + * fast/forms/search/search-zoom-rendering-expected.txt: Added. > > + * fast/forms/search/search-zoom-rendering.html: Added. > > + * platform/ios-wk2/fast/forms/search/search-zoom-rendering-expected.txt: Added. > > + * platform/mac-mojave/fast/forms/search/search-zoom-rendering-expected.txt: Added. > > + * platform/win/fast/forms/search/search-zoom-rendering-expected.txt: Added. > > It’s really undesirable to have another render-tree-dumping test like this. > Is there any way we can test this without a render tree dump? > > One thing to think about is that the render tree dump doesn’t actually check > the clipping, only the geometry. Since that’s all we need to check, maybe we > can do a reftest that explicitly sets the size? Or perhaps we can do a > dumpAsText test that explicitly queries the height? > > We can land like this, but these are really hard to maintain over time. I’m > particularly puzzled by the seemingly random set of expectation files. For > example, why does legacy WebKit testing on iOS use the shared master > expectation, but modern WebKit testing on iOS use its own. Maybe that file > should be in platform/ios instead of in platform/ios-wk2? I was unable to write a reftest that explicitly sets the size, since height is overridden when rendering our native search control (same behavior as NSSearchFieldCell). However, I did replace the render-tree-dumping test with a dumpAsText test. I also moved the expectation from platform/ios-wk2 into platform/ios.
Created attachment 406514 [details] Patch for landing
Committed r265613: <https://trac.webkit.org/changeset/265613> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406514 [details].
Reopening to attach patch for test fix.
Created attachment 406542 [details] Rebaseline test
Committed r265625: <https://trac.webkit.org/changeset/265625> All reviewed patches have been landed. Closing bug and clearing flags on attachment 406542 [details].