Bug 199648 - CSS Grid `align-items: baseline;` with scaled images results in excessive top whitespace
Summary: CSS Grid `align-items: baseline;` with scaled images results in excessive top...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: Safari Technology Preview
Hardware: All All
: P2 Blocker
Assignee: Oriol Brufau
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-07-09 16:56 PDT by Dan Richman
Modified: 2020-08-26 10:40 PDT (History)
16 users (show)

See Also:


Attachments
Reduction of CSS Grid issue (1.49 KB, text/html)
2019-07-09 16:56 PDT, Dan Richman
no flags Details
Simpler testcase (447 bytes, text/html)
2020-08-25 13:38 PDT, Oriol Brufau
no flags Details
Patch (7.40 KB, patch)
2020-08-26 08:46 PDT, Oriol Brufau
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Dan Richman 2019-07-09 16:56:03 PDT
Created attachment 373795 [details]
Reduction of CSS Grid issue

Summary:
A horizontal CSS Grid-based shelf with (%) percent-based scaled-width images and `align-items: baseline;` will result in excessive container size.

Tested in Safari Technology Preview, Release 86 (Safari 13.0, WebKit 14608.1.30.2)

Steps To Reproduce:
1. Open attached `grid-image-align-reduction.html`
2. Observe excessive space above images.

Results:
There should be no space between the top of the tallest image and the top of the window. The tallest image should align to the top of the window.

Regression:
When this was reduced, it became clear that giving the images a `%` width value (to scale to the width of their grid containers) is what triggered the `align-items: baseline;` bug. If you define a pixel-based width on the images, the bug is not triggered.
Comment 1 Radar WebKit Bug Importer 2019-07-10 09:23:19 PDT
<rdar://problem/52895538>
Comment 2 Manuel Rego Casasnovas 2020-08-04 12:36:57 PDT
Oriol, could you please take a look?
Comment 3 Oriol Brufau 2020-08-25 13:38:32 PDT
Created attachment 407225 [details]
Simpler testcase

OK, so the problem is that the track sizing algorithm takes baseline alignment into account, so when we calculate that baseline alignment we still don't know the final grid area sizes.

In RenderGrid::performGridItemsPreLayout, there is

```cpp
if (isBaselineAlignmentForChild(*child)) {
    updateGridAreaLogicalSize(*child, algorithm.estimatedGridAreaBreadthForChild(*child));
```

The estimated area is 200px wide and undefined tall for the 1st item.
But the 2nd item has a 50px wide X, so the estimated area is 250px wide.
Then we think that the image will be 250px wide, and therefore 250px tall via the aspect ratio.
So we add a 50px shim, which increases the row size by 50px, thinking that this will provide proper baseline alignment.
However, the 2ns column ends up being 200px, not 250px, so the above was wrong.

The problem is in GridTrackSizingAlgorithm::estimatedGridAreaBreadthForChild.
Since `isRelativeGridLengthAsAuto(maxTrackSize, direction)` is true, it returns the maxPreferredLogicalWidth.
However, while the grid track sizing function for the column is a percentage, it is actually definite due to `width: 400px`.
So isRelativeGridLengthAsAuto should return false!
But it returns true because the availableSpace has not been set yet, so it thinks that the grid is sized under a min/max-content constraint.

Chromium is also affected.
Comment 4 Oriol Brufau 2020-08-26 08:46:39 PDT
Created attachment 407299 [details]
Patch
Comment 5 Javier Fernandez 2020-08-26 09:22:11 PDT
Comment on attachment 407299 [details]
Patch

r=me
Comment 6 EWS 2020-08-26 10:40:41 PDT
Committed r266173: <https://trac.webkit.org/changeset/266173>

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