Bug 214370

Summary: [css-grid] Vertical alignment is wrong with box-sizing: border-box
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: Layout and RenderingAssignee: 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 Flags
Testcase to reproduce the issue
none
testcase with both box-sizing values
none
Patch
none
Patch none

Simon Fraser (smfr)
Reported 2020-07-15 12:15:57 PDT
https://codepen.io/argyleink/pen/pogZxaZ Looks different in WebKit vs other browsers.
Attachments
Testcase to reproduce the issue (232 bytes, text/html)
2020-07-15 13:29 PDT, Manuel Rego Casasnovas 		no flags
Details
testcase with both box-sizing values (548 bytes, text/html)
2020-07-24 09:34 PDT, Oriol Brufau 		no flags
Details
Patch (8.23 KB, patch)
2020-07-24 16:37 PDT, Oriol Brufau 		no flags
DetailsFormatted DiffDiff
Patch (11.84 KB, patch)
2020-07-28 14:09 PDT, Oriol Brufau 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1 2020-07-15 12:16:34 PDT
Maybe a grid issue?
Manuel Rego Casasnovas
Comment 2 2020-07-15 13:29:16 PDT
Created attachment 404385 [details] Testcase to reproduce the issue
Manuel Rego Casasnovas
Comment 3 2020-07-15 13:30:06 PDT
(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?
Radar WebKit Bug Importer
Comment 4 2020-07-22 12:16:12 PDT
<rdar://problem/65948925>
Oriol Brufau
Comment 5 2020-07-24 09:34:13 PDT
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.
Oriol Brufau
Comment 6 2020-07-24 16:37:47 PDT
Created attachment 405201 [details] Patch
EWS Watchlist
Comment 7 2020-07-24 16:38:44 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
Oriol Brufau
Comment 8 2020-07-24 17:25:36 PDT
It seems Chromium did the same change in https://chromium.googlesource.com/chromium/src/+/acc04ddbdc31a7c5bda78f88719df43367630fed%5E%21/#F2
Manuel Rego Casasnovas
Comment 9 2020-07-27 01:22:30 PDT
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.
Oriol Brufau
Comment 10 2020-07-27 03:40:30 PDT
(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
Oriol Brufau
Comment 11 2020-07-28 14:09:19 PDT
Created attachment 405412 [details] Patch
Oriol Brufau
Comment 12 2020-07-28 14:11:18 PDT
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.
EWS
Comment 13 2020-07-28 16:41:10 PDT
Committed r265020: <https://trac.webkit.org/changeset/265020> All reviewed patches have been landed. Closing bug and clearing flags on attachment 405412 [details].
Note You need to log in before you can comment on or make changes to this bug.