| Summary: | [css-grid] Overflow should be computed with the actual logical bottom | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | zsun | ||||||||||||||
| Component: | CSS | Assignee: | zsun | ||||||||||||||
| Status: | RESOLVED FIXED | ||||||||||||||||
| Severity: | Normal | CC: | changseok, clopez, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, svillar, webkit-bug-importer, youennf | ||||||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||||||
| Version: | WebKit Nightly Build | ||||||||||||||||
| Hardware: | Unspecified | ||||||||||||||||
| OS: | Unspecified | ||||||||||||||||
| Attachments: |
|
||||||||||||||||
|
Description
zsun
2021-06-14 11:43:32 PDT
Created attachment 431349 [details]
Patch
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 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. Created attachment 435419 [details]
Patch
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
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
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
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]. |