New issue
Advanced search Search tips

Issue 786971 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[css-grid] Automatic minimum size is not clamped if min track sizing function is auto

Project Member Reported by r...@igalia.com, Nov 20 2017

Issue description


The spec is quite clear regarding this (https://drafts.csswg.org/css-grid/#min-size-auto):
 "However, if the grid item spans only grid tracks that have
  a fixed max track sizing function, its specified size and
  content size in that dimension (and the input to
  the transferred size in the other dimension) are further clamped
  to less than or equal to the stretch fit the grid area’s size
  (so as to prevent the automatic minimum size from forcing overflow
  of its fixed-size grid area)."

It says that if the max track sizing function is fixed,
it should clamp the automatic minimum size.

In the attached example we have a grid container with:
  grid-template-columns: minmax(auto, 0px);
So the automatic minimum size of the item needs to be clamped to 0px.
And the final size of the track should be 0px.

This is basically what this test wrongly checks:
http://w3c-test.org/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html

Firefox does this right.
 
automatic-minimum-size.html
168 bytes View Download
automatic-minimum-size.png
4.1 KB View Download
Project Member

Comment 1 by bugdroid1@chromium.org, Nov 23 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/93cb0356747a29c50dd85e716e7ef7894093a489

commit 93cb0356747a29c50dd85e716e7ef7894093a489
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Thu Nov 23 16:44:07 2017

[css-grid] Clamp automatic minimum size of max track sizing is fixed

We were not clamping the automatic minimum size when
the min track sizing function was intrinsic (e.g. minmax(auto, 0px)).
However the spec (https://drafts.csswg.org/css-grid/#min-size-auto)
is very clear regarding that.

This patch modifies
GridTrackSizingAlgorithm::SizeTrackToFitNonSpanningItem(),
so in the case of a fixed max track sizing function it clamps
the automatic minimum size of the item to the stretch fit
of the grid area's size.
It needs to take into account if the item has fixed size, margin, border
and/or padding as those cannot be clamped.

Added two new specific tests to verify this behavior,
and corrected a bunch of other tests that were wrong.

BUG= 786971 
TEST=external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-022.html
TEST=external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-023.html

Change-Id: I12d41fef346e81e917f443801f34db4d1eadbd90
Reviewed-on: https://chromium-review.googlesource.com/783213
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Cr-Commit-Position: refs/heads/master@{#518962}
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-definition/grid-template-columns-fit-content-001-ref.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-definition/grid-template-columns-fit-content-001.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-definition/grid-template-rows-fit-content-001-ref.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-017.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-018.html
[add] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-022.html
[add] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-minimum-size-grid-items-023.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/fast/css-grid-layout/min-height-border-box.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/fast/css-grid-layout/min-width-margin-box.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size-in-auto.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/LayoutTests/fast/css-grid-layout/percent-of-indefinite-track-size.html
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.cpp
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h
[modify] https://crrev.com/93cb0356747a29c50dd85e716e7ef7894093a489/third_party/WebKit/Source/core/style/GridTrackSize.h

Comment 2 by r...@igalia.com, Nov 23 2017

Status: Fixed (was: Started)

Comment 3 by r...@igalia.com, Nov 30 2017

 Issue 789927  has been merged into this issue.

Comment 4 by yio...@gmail.com, Dec 7 2017

So is Firefox's(59) current rendering wrong?
Firefox.png
16.1 KB View Download

Comment 5 by yio...@gmail.com, Dec 7 2017

"grid-template-columns: minmax(min-content, 0px)" This also causes two browsers to render inconsistencies.


Comment 6 by r...@igalia.com, Jan 2 2018

@yirosi, sorry I don't know why I missed your comments.

Firefox does it right for the item, as it has 0px,
the same than in Chromium after my patch (you don't see the "lime" background).

However it seems Firefox does it wrong to compute the intrinsic width of the grid container,
and it includes the size of the item.
I've just reported the bug to Firefox: https://bugzilla.mozilla.org/show_bug.cgi?id=1427608

Sign in to add a comment