New issue
Advanced search Search tips

Issue 816762 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug



Sign in to add a comment

[css-grid] margin isn't included in min-size contribution when min-size is definite

Project Member Reported by mpalmg...@mozilla.com, Feb 27 2018

Issue description

Chrome Version: 65.0.3311.3 (Official Build) dev (64-bit)
OS: Linux

What steps will reproduce the problem?
(1)load the attached testcase
(2)
(3)

What is the expected result?
The top four boxes renders the same as the bottom four.
The first two boxes in each row should have a 10px square area at the end.
The last two boxes in each row should have a 10px square area at the start.

What happens instead?
The boxes that has 'min-width:30px' doesn't seem to include the margin.
(the first and third boxes in the first row are wrong - see Firefox for correct results)

The Grid spec is clear that the margin should be included:
https://drafts.csswg.org/css-grid/#min-size-contribution
"The min-size contribution of an item is the outer size that would result from assuming the item’s min-width or min-height value (whichever matches the relevant axis) as its specified size if its specified size (width or height, whichever matches the relevant axis) is auto"

Where "outer size" is defined as the margin-box here:
https://drafts.csswg.org/css-sizing-3/#outer-size

 
grid-auto-min-sizing-definite-001-B.html
1.3 KB View Download

Comment 1 by e...@chromium.org, Feb 27 2018

Status: Available (was: Untriaged)

Comment 2 by r...@igalia.com, Mar 7 2018

Cc: svil...@igalia.com jfernan...@igalia.com
Owner: r...@igalia.com
Status: Started (was: Available)
Thanks for reporting.

This seems related to a TODO we still have in the code:
https://chromium.googlesource.com/chromium/src/+/d952cb7721e77d0613f884336cb468b94b9ea1b0/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.cpp#494

I'll take a look and try to fix it.
Project Member

Comment 3 by bugdroid1@chromium.org, Mar 14 2018

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

commit 70a0a9991823afda169f858dfe4aa7e5ddbef223
Author: Manuel Rego Casasnovas <rego@igalia.com>
Date: Wed Mar 14 10:49:47 2018

[css-grid] Minimum width items should include margins for indefinite grid containers

We had a TODO in IndefiniteSizeStrategy::MinLogicalWidthForChild()
that was always using 0 as margin of the item.
The patch just changes that for a call
to GridLayoutUtils::MarginLogicalWidthForChild()
(like we do in the DefiniteSizeStrategy version).

BUG= 816762 
TEST=external/wpt/css/css-grid/grid-items/grid-items-minimum-width-001.html
TEST=external/wpt/css/css-grid/grid-items/grid-items-minimum-width-002.html

Change-Id: I743584aff234625f2d00327c711c0577ffeab457
Reviewed-on: https://chromium-review.googlesource.com/955062
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#543046}
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-001.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-002.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-orthogonal-001.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-orthogonal-002.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-vertical-lr-001.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-vertical-lr-002.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-vertical-rl-001.html
[add] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/LayoutTests/external/wpt/css/css-grid/grid-items/grid-items-minimum-width-vertical-rl-002.html
[modify] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/Source/core/layout/GridLayoutUtils.cpp
[modify] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.cpp
[modify] https://crrev.com/70a0a9991823afda169f858dfe4aa7e5ddbef223/third_party/WebKit/Source/core/layout/GridTrackSizingAlgorithm.h

Comment 4 by r...@igalia.com, Mar 14 2018

Status: Fixed (was: Started)

Sign in to add a comment