Created attachment 393799 [details] Test case to reproduce the issue Margin should not be affected when other elements' content are modified in the grid. It seems that we are not recalculating correctly the auto margins when the grid's size changes. Apparently, the root case is that the grid item's margins are not cleared when their container is laid out again, due to a size (width in this case) change.
Created attachment 393800 [details] Patch
Comment on attachment 393800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393800&action=review > Source/WebCore/rendering/RenderGrid.cpp:1178 > + LayoutUnit marginLogicalWidth; How does this patch work? We compute this value, but never use it. > Source/WebCore/rendering/RenderGrid.cpp:1209 > + LayoutUnit marginLogicalHeight; Same here.
Comment on attachment 393800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393800&action=review >> Source/WebCore/rendering/RenderGrid.cpp:1178 >> + LayoutUnit marginLogicalWidth; > > How does this patch work? We compute this value, but never use it. There must be something wrong with the test, too, since it’s passing without any behavior change.
Created attachment 393969 [details] Patch
Comment on attachment 393800 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=393800&action=review >>> Source/WebCore/rendering/RenderGrid.cpp:1178 >>> + LayoutUnit marginLogicalWidth; >> >> How does this patch work? We compute this value, but never use it. > > There must be something wrong with the test, too, since it’s passing without any behavior change. Wow, I'm so sorry about that. I must have submitted an old version of the patch, which even had test failures in the expected.txt files. As a matter of fact, I applied some changes in the tests to ensure the test-harness checks are done after the Ahem web-font is loaded. I have submitted a PR [1] for the WPT github repo as well with these changes. [1] https://github.com/web-platform-tests/wpt/pull/22332 >> Source/WebCore/rendering/RenderGrid.cpp:1209 >> + LayoutUnit marginLogicalHeight; > > Same here. yeah, already submitted a patch to fix this.
Committed r258735: <https://trac.webkit.org/changeset/258735> All reviewed patches have been landed. Closing bug and clearing flags on attachment 393969 [details].
<rdar://problem/60655776>