Affected WPT tests - imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-align-self-002.html imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-align-self-rtl-003.html imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-align-self-rtl-004.html imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-justify-self-002.html imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-justify-self-rtl-003.html imported/w3c/web-platform-tests/css/css-grid/abspos/grid-abspos-staticpos-justify-self-rtl-004.html
Created attachment 445853 [details] Patch
<rdar://problem/86320868>
Comment on attachment 445853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=445853&action=review Thanks for working on this Ziran ! As we discussed offline, the test cases you are fixing with this patch are not grid related, if we are strict with what the grid spec states. In that sense, that's why you are adding this alignment related logic in RenderBox. At a first sight, it looks like very similar to the one we already have for Flexbox and Grid. I believe that a proper solution to this bug would be to properly implement the CSS Box Alignment spec for block-level boxes and absolute positioned elements. I understand that implementing the whole feature would be a long-term effort, but at least we can fix this issue in a way that is more aligned to such long-term goal (eg, thinking about possible refactoring in the flex and grid alignment logic). > Source/WebCore/rendering/RenderBox.cpp:4201 > + if (isGridItem() && isStaticChildInline && GridLayoutFunctions::flowAwareDirectionForChild(downcast<RenderGrid>(*parent()), *this, ForColumns) == ForColumns) { Wouldn't be a good idea to store the RenderGrid downcast in a local variable ? it seems it's used later again.
After a couple of offline discussions with Javier and Oriol, we feel that a generic solution that would work for both css-grid and flexibox might be the way forward in long term. I'd like to leave this bug open for anyone who has time and effort to take it.