Bug 214370 – [css-grid] Vertical alignment is wrong with box-sizing: border-box

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

https://codepen.io/argyleink/pen/pogZxaZ

Looks different in WebKit vs other browsers.

[Simon Fraser (smfr)]

Maybe a grid issue?

[Manuel Rego Casasnovas]

Created attachment 404385 [details]
Testcase to reproduce the issue

[Manuel Rego Casasnovas]

(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]

<rdar://problem/65948925>

[Oriol Brufau]

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]

Created attachment 405201 [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

[Oriol Brufau]

It seems Chromium did the same change in https://chromium.googlesource.com/chromium/src/+/acc04ddbdc31a7c5bda78f88719df43367630fed%5E%21/#F2

[Manuel Rego Casasnovas]

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]

(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]

Created attachment 405412 [details]
Patch

[Oriol Brufau]

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]

Committed r265020: <https://trac.webkit.org/changeset/265020>

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