Created attachment 460514 [details] Smallest viable reproduction attachment Elements that have the CSS properties: aspect-ratio, padding and height (specifically to 100%) set seem to inadvertently scale upwards whenever a repaint is triggered. These three properties were the least amount of properties I could find to set that would trigger the problem I've included an attachment of the HTML I used, but the problems can easily be previewed on this code sandbox: https://codesandbox.io/s/broken-safari-forked-1n9o4k?file=/index.html. It's important to trigger the hover effect to see that the button elements grow. In practice I've found the hover effect is unimportant, but it helps to demonstrate the problem. Toggling styles in the developer tools (such as cursor) can cause the elements to scale upward.
In case anybody else is looking, this seems specific to grid, I tried display:flex and display:block for example and the issue is not there.
<rdar://problem/96419984>
Created attachment 460712 [details] Patch
Created attachment 461040 [details] Patch
Created attachment 461041 [details] Patch
Created attachment 461135 [details] Patch
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment on attachment 461135 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461135&action=review > COMMIT_MESSAGE:1 > +Element with aspect-ratio, padding and height: 100% grow in size when repainted "when repainted" seems wrong. This is per layout, right?
Created attachment 461147 [details] Patch
Comment on attachment 461135 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461135&action=review >> COMMIT_MESSAGE:1 >> +Element with aspect-ratio, padding and height: 100% grow in size when repainted > > "when repainted" seems wrong. This is per layout, right? Right, layout is wrong so as a consequence the (re)paint problem is visible. I stuck to the original bug title since I had a hard time making a terse first line, but I did another attempt.
Comment on attachment 461147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461147&action=review > Source/WebCore/rendering/RenderBox.cpp:5489 > + if (style().logicalWidth().isPercentOrCalculated() && parent()->style().logicalWidth().isFixed()) Wait, the rest of this function does not dereference parent(). Is this guaranteed to be non-null?
(In reply to Darin Adler from comment #11) > Comment on attachment 461147 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=461147&action=review > > > Source/WebCore/rendering/RenderBox.cpp:5489 > > + if (style().logicalWidth().isPercentOrCalculated() && parent()->style().logicalWidth().isFixed()) > > Wait, the rest of this function does not dereference parent(). Is this > guaranteed to be non-null? Also in block layout, the canonical way to get to "parent" is by calling containingBlock() (i.e parent in the renderer tree is not necessarily the "parent" in CSS terms.) It's rarely the case when you specifically want "tree parent" and not containing block in block layout (the confusion comes from the fact that containing block translates to "tree parent" in most cases, so it's easy to just call parent() instead of containingBlock()).
Committed 252819@main (39aa8ef8099b): <https://commits.webkit.org/252819@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 461147 [details].
Comment on attachment 461147 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=461147&action=review >>> Source/WebCore/rendering/RenderBox.cpp:5489 >>> + if (style().logicalWidth().isPercentOrCalculated() && parent()->style().logicalWidth().isFixed()) >> >> Wait, the rest of this function does not dereference parent(). Is this guaranteed to be non-null? > > Also in block layout, the canonical way to get to "parent" is by calling containingBlock() (i.e parent in the renderer tree is not necessarily the "parent" in CSS terms.) It's rarely the case when you specifically want "tree parent" and not containing block in block layout (the confusion comes from the fact that containing block translates to "tree parent" in most cases, so it's easy to just call parent() instead of containingBlock()). In this case I rely on isGridItem, which established parent() exists and is a RenderGrid. That is the container I want to test the percentage against and see if the percentage turns out to be definite.