Summary: | [css-grid] Vertical alignment is wrong with box-sizing: border-box | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Simon Fraser (smfr) <simon.fraser> | ||||||||||
Component: | Layout and Rendering | Assignee: | Oriol Brufau <obrufau> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | bfulgham, changseok, clopez, darin, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, obrufau, pdr, rego, simon.fraser, webkit-bug-importer, youennf, zalan | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | Safari Technology Preview | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
URL: | https://codepen.io/argyleink/pen/pogZxaZ | ||||||||||||
See Also: | https://github.com/web-platform-tests/wpt/pull/24748 | ||||||||||||
Attachments: |
|
Description
Simon Fraser (smfr)
2020-07-15 12:15:57 PDT
Maybe a grid issue? Created attachment 404385 [details]
Testcase to reproduce the issue
(In reply to Simon Fraser (smfr) from comment #1) > Maybe a grid issue? Yes, it looks like a bug related to vertical alignment and box-sizing: border-box. Please Oriol, could you take a look? Created attachment 405157 [details]
testcase with both box-sizing values
Actually it also happens with 'box-sizing: content-box'. See new testcase.
The problem seems that RenderBox::availableLogicalHeight calls constrainLogicalHeightByMinMax instead of constrainContentBoxLogicalHeightByMinMax.
Created attachment 405201 [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 It seems Chromium did the same change in https://chromium.googlesource.com/chromium/src/+/acc04ddbdc31a7c5bda78f88719df43367630fed%5E%21/#F2 Comment on attachment 405201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405201&action=review It looks like we could have a test without grid too (checking the Chromium change). BTW, is this a new test? If that's the case you need to export it to WPT and land it there, before merging it in WebKit. > Source/WebCore/ChangeLog:9 > + the available grid space for in the block axis. But there was a bug when Nit: You might want to reword this sentence "the available grid space for in the block axis" sounds weird to me. (In reply to Manuel Rego Casasnovas from comment #9) > BTW, is this a new test? If that's the case you need to export it to WPT and > land it there, before merging it in WebKit. Yes, a new test, this is the export: https://github.com/web-platform-tests/wpt/pull/24748 Created attachment 405412 [details]
Patch
Comment on attachment 405201 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=405201&action=review I have imported the Chromium test. >> Source/WebCore/ChangeLog:9 >> + the available grid space for in the block axis. But there was a bug when > > Nit: You might want to reword this sentence "the available grid space for in the block axis" sounds weird to me. Done. Committed r265020: <https://trac.webkit.org/changeset/265020> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405412 [details]. |