Bug 233812

Summary:

[css-grid] Add support for static pos of abspos grid children with end/center alignment

Product:

WebKit

Reporter:

zsun

Component:

CSS

Assignee:

Nobody <webkit-unassigned>

Status:

NEW ---

Severity:

Normal

CC:

ahmad.saleem792, changseok, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, sgill26, svillar, webkit-bug-importer, zalan

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none

Description zsun 2021-12-03 07:21:39 PST

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

Comment 1 zsun 2021-12-03 07:48:10 PST

Created attachment 445853 [details]
Patch

Comment 2 Radar WebKit Bug Importer 2021-12-10 07:22:17 PST

<rdar://problem/86320868>

Comment 3 Javier Fernandez 2021-12-13 09:00:42 PST

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.

Comment 4 zsun 2022-05-13 02:43:42 PDT

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.