Bug 226981 - [css-grid] Overflow should be computed with the actual logical bottom
Summary: [css-grid] Overflow should be computed with the actual logical bottom
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: zsun
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-06-14 11:43 PDT by zsun
Modified: 2021-09-15 11:06 PDT (History)
12 users (show)

See Also:


Attachments
Patch (26.69 KB, patch)
2021-06-14 11:58 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (7.91 KB, patch)
2021-08-12 06:24 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (16.31 KB, patch)
2021-09-06 04:01 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (16.63 KB, patch)
2021-09-08 05:08 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (16.64 KB, patch)
2021-09-13 11:46 PDT, zsun
no flags Details | Formatted Diff | Diff
Patch (16.64 KB, patch)
2021-09-14 01:30 PDT, zsun
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description zsun 2021-06-14 11:43:32 PDT
In order to compute the overflow area in the grid container we should
use the result of the ClientLogicalBottom function.
Comment 1 zsun 2021-06-14 11:58:25 PDT
Created attachment 431349 [details]
Patch
Comment 2 EWS Watchlist 2021-06-14 11:59:24 PDT
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 3 Sergio Villar Senin 2021-06-17 03:38:23 PDT
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?
Comment 4 zsun 2021-06-18 03:43:02 PDT
(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.
Comment 5 Radar WebKit Bug Importer 2021-06-21 11:44:47 PDT
<rdar://problem/79572860>
Comment 6 zsun 2021-08-12 06:24:50 PDT
Created attachment 435419 [details]
Patch
Comment 7 Sergio Villar Senin 2021-08-13 02:51:39 PDT
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.
Comment 8 zsun 2021-08-13 03:51:46 PDT
(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.
Comment 9 zsun 2021-09-06 04:01:24 PDT
Created attachment 437403 [details]
Patch
Comment 10 Javier Fernandez 2021-09-06 05:00:25 PDT
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 ?
Comment 11 zsun 2021-09-08 05:08:39 PDT
Created attachment 437620 [details]
Patch
Comment 12 Javier Fernandez 2021-09-13 07:18:25 PDT
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 ?
Comment 13 zsun 2021-09-13 11:46:55 PDT
Created attachment 438062 [details]
Patch
Comment 14 zsun 2021-09-14 01:30:04 PDT
Created attachment 438113 [details]
Patch
Comment 15 EWS 2021-09-15 11:06:54 PDT
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].