Bug 226981 – [css-grid] Overflow should be computed with the actual logical bottom

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

In order to compute the overflow area in the grid container we should
use the result of the ClientLogicalBottom function.

Created attachment 431349 [details]
Patch

[EWS Watchlist]

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

[Sergio Villar Senin]

Comment on attachment 431349 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=431349&action=review

> LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-content-alignment-overflow-002-expected.txt:119
> +

Looks like we're failing 7/14 subtests. Question is what was the status before the patch?

(In reply to Sergio Villar Senin from comment #3)
> Comment on attachment 431349 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=431349&action=review
> 
> > LayoutTests/imported/w3c/web-platform-tests/css/css-grid/alignment/grid-content-alignment-overflow-002-expected.txt:119
> > +
> 
> Looks like we're failing 7/14 subtests. Question is what was the status
> before the patch?

I have just noticed that in chromium  with NG enabled all tests have now passed. I need to look into this and update the patch.

[Radar WebKit Bug Importer]

<rdar://problem/79572860>

Created attachment 435419 [details]
Patch

[Sergio Villar Senin]

Comment on attachment 435419 [details]
Patch

I am not sure the fix is correct. Furthermore the patch seems to fix some cases but regresses some others, so I am not sure we're improving here.

(In reply to Sergio Villar Senin from comment #7)
> Comment on attachment 435419 [details]
> Patch
> 
> I am not sure the fix is correct. Furthermore the patch seems to fix some
> cases but regresses some others, so I am not sure we're improving here.

The fix is not complete. Still I need to address the difference caused by paddings.

Created attachment 437403 [details]
Patch

[Javier Fernandez]

Comment on attachment 437403 [details]
Patch

I like the idea, but perhaps we could do some refactoring between the clientLogicalBottomAfterRepositioning and the new logic you've added into the computeOverflow function. After all, it's the same logic for both inline and block axes, isn't it ? We could do both (when needed) in the same loop. What do you think ?

Created attachment 437620 [details]
Patch

[Javier Fernandez]

Comment on attachment 437620 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=437620&action=review

r=me

> Source/WebCore/rendering/RenderBlock.h:235
> +    LayoutSize clientLogicalRightAndBottomAfterRepositioning();

This function can be 'const', right ?

Created attachment 438062 [details]
Patch

Created attachment 438113 [details]
Patch

[EWS]

Committed r282463 (241711@main): <https://commits.webkit.org/241711@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 438113 [details].