New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-grid] Stretching tracks fails to feed back into sizing algorithm #1150

Closed
fantasai opened this Issue Mar 30, 2017 · 6 comments

Comments

Projects
None yet
3 participants
@fantasai
Contributor

fantasai commented Mar 30, 2017

Continuing from #1117 examples 3/4/7/8...

<div style="display: grid; width: 200px;">
 <img style="width: 100%;" src="http://placehold.it/50x50">
</div>

Here to resolve the size of the auto column, we check our children.
auto is somehow equivalent to minmax(auto, max-content).
The image has width: 100%; which cannot be resolved as the column has auto size, so for the minimum track size we consider 0px. And we use 50px, the content size of the image as maximum track size. And for the row, we use 50px too (the content size of the image).
Then the column size is affected by the default justify-content: stretch; which makes the auto column to grow up to 200px. The image is sized against that size of the column, so it ends up being 200x200. However the row is still 50px.

The row here being 50px is not very helpful. It should respond to the stretching of the track.

The responsible spec prose is in https://drafts.csswg.org/css-grid/#algo-overview where stretch is processed at the end of the grid sizing algorithm. It should instead be integrated as a final step of track sizing, after fr sizing, in order to be looped into later track-sizing passes.

@mrego

This comment has been minimized.

Member

mrego commented Apr 3, 2017

I'm not sure if this will be possible without introducing other issues. For example in the case of orthogonal flows.

Let's try to illustrate that with the following example:

  <div style="display: grid; with: 500px; grid: 250px / auto auto; border: magenta solid thick; font: 100px/1 Ahem;">
    <div style="background: cyan; writing-mode: vertical-lr;">XX X X</div>
    <div style="background: yellow; writing-mode: vertical-lr;">X</div>
  </div>

With the current spec stretch is done at the end of the grid track sizing algorithm. So this case works like:

  1. On the 1st pass for the columns, the width is calculated as 100px for both columns (the items are vertical).
  2. The pass for rows it's straight forward, as the row has a fixed size of 250px.
  3. Then the min-content contribution of the 1st item has changed due to the row size, so we need to do a 2nd pass. On the 2nd pass for the columns, the width is calculated as 300px for the 1st column and 100px for the 2nd one.
  4. Again the rows pass is simple as it's a fixed size.
  5. Then we do stretch so the 1st column becomes 350px and the 2nd column 150px.

Current behavior with stretch after tracks sizing

However, with the new proposal I believe this will end up in a worse result:

  1. The 1st pass for columns is the same. 100px width for both.
  2. But then we do stretch, so each column becomes 250px.
  3. Then the pass for rows is again simple as it has a fixed size of 250px.
  4. As the min-content contribution of the first item has changed again we need a new pass. On the 2nd pass for columns the 1st column has a width of 300px, and the 2nd one keeps its 250px width (the item was already sized using the 250px of that column after the stretch. And here's the problem we'll have overflow on the grid container, because of the stretch done in step 2.

New behavior with stretch inside track sizing

As you can see in the picture, in the last case we'll be overflowing the grid container because of stretch, which feels really wrong as stretch should use the available space not more.

Last, I'm not sure if the use case is important enough to introduce changes on the track sizing algorithm at this stage. If you're using percentages on items against indefinite track sizes, I guess that having weird results in some cases can be somehow expected.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Aug 1, 2017

Fixed max track sizes in the row axis are assumed at the beginning of the first column sizing pass, so the second interpretation can't happen in any case. Note the second paragraph:

First, the track sizing algorithm is used to resolve the sizes of the grid columns.

If calculating the layout of a grid item in this step depends on the available space in the block axis, assume the available space that it would have if any row with a definite max track sizing function had that size and all other rows were infinite.

A case where we could get this result would be if the grid container itself had a fixed height.

A second comment is, if we're recalculating the column sizes, shouldn't the extra space from stretching be re-calculated? In which case we should still get the first result.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Aug 4, 2017

The CSS Working Group just discussed Stretching tracks fails to feed back into sizing algorithm, and agreed to the following resolutions:

  • RESOLVED: integrate stretching into track sizing algo
The full IRC log of that discussion <dauwhe> Topic: Stretching tracks fails to feed back into sizing algorithm
<astearns> github: https://github.com//issues/1150
<dauwhe> fantasai: the issue here is that the stretch alignment property takes effect
<dauwhe> ... at the end of the algo
<dauwhe> ... you size the cols, then use size of cols to set height of rows
<dauwhe> ... at the end you do alignment
<dauwhe> ... that's generally fine
<dauwhe> ... cause alignment don't change track sizes
<dauwhe> ... but stretch does
<dauwhe> ... if you start with this example, you have image with intrinsic size, col sizes to intrinsic size, then try to size the rows
<dauwhe> ... and we use the size of the columns because there's an aspect ratio
<dauwhe> ... and now the item that's 100% wide fills the track in that axis, but then overflows the row
<dauwhe> ... the suggestion is that the stretching phase should be accounted for in the sizing algo
<dauwhe> TabAtkins: makes sense
<dauwhe> fantasai: we'll try to incorporate this into the algo, then will bring to group for review
<dauwhe> TabAtkins: is auto track and stretch distro the only thing that does this?
<dauwhe> fantasai: yes
<dauwhe> TabAtkins: then this makes total sense
<dauwhe> fantasai: the proposal is to handle the streching at each phase of stretch sizing
<dauwhe> astearns: would it make sense to have a note in align?
<dauwhe> fantasai: flex already has this
<fantasai> s/this/this kind of integration/
<dauwhe> Rossen: stretching and last baseline tend to have feedback back into the layout algo
<fantasai> astearns: Yeah, so should have a note pointing to the appropriate section of grid/flex
<dauwhe> RESOLVED: integrate stretching into track sizing algo
<dauwhe> fantasai: ONE MORE THING
@fantasai

This comment has been minimized.

Contributor

fantasai commented Aug 9, 2017

Edits checked in. @mrego, please let us know if anything doesn't seem right to you?

@mrego

This comment has been minimized.

Member

mrego commented Sep 8, 2017

A case where we could get this result would be if the grid container itself had a fixed height.

Yep, you're right that I missed the part of the algorithm that explains that we'll be using the fixed max track size, so that case is not an issue.

Also if we go for the case where the row is auto but the grid container has height: 250px the issue is not present either, as due to the orthogonal item we do a 2nd pass and everything works as expected.

I've been doing some experiments and it seems that it works as expected and I didn't find any case that breaks due to this. Of course this need more testing but it seems a good approach.
It's always scary to touch the track sizing algorithm, but let's hope everything goes right. 😄

Regarding the prose, it seems there are 2 verbs together in:

This step sizes expands tracks

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 8, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this changes integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
WPT-Export-Revision: f3cc94d820cfd2c3f0ce4ea285cfc40decf3bb46

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
WPT-Export-Revision: d3cb26941eeaf7cd4f49172b1894407c767bc988

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 9, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
WPT-Export-Revision: 68c1a9c059d864a9e590063dc3a0c058039fa5be

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 11, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
WPT-Export-Revision: 1ed13fd94f88428b49d461959ab67fedfe46fdf1

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 12, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
WPT-Export-Revision: edd8b0c249be5489c35f3a6d06b4cd70d8281ba0

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 12, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
WPT-Export-Revision: 143a4ac8c9d97a67714e34920cbb9c92a00ac6f5

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 12, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#501249}

chromium-wpt-export-bot added a commit to web-platform-tests/wpt that referenced this issue Sep 12, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#501249}

hubot pushed a commit to WebKit/webkit that referenced this issue Sep 12, 2017

rego@igalia.com
[css-grid] Stretching auto tracks should be done as part of the track…
… sizing algorithm

https://bugs.webkit.org/show_bug.cgi?id=176783

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the test related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:

Source/WebCore:

CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
RenderGrid::applyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::stretchAutoTracks().
And then modifies the grid track sizing algorithm to execute
the new step.

This patch uses the WPT test updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::initializeTrackSizes): Initialize
the list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::stretchFlexibleTracks): Add
early return if there are not flexible tracks.
(WebCore::GridTrackSizingAlgorithm::stretchAutoTracks): Code moved from
RenderGrid::applyStretchAlignmentToTracksIfNeeded().
(WebCore::GridTrackSizingAlgorithm::setup): Reset list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::run): Add new step
stretchAutoTracks().
(WebCore::GridTrackSizingAlgorithm::reset): Reset auto tracks.
* rendering/GridTrackSizingAlgorithm.h: Add m_autoSizedTracksIndex.
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Avoid applying stretch here.
(WebCore::RenderGrid::contentAlignmentNormalBehaviorGrid): Make public.
(WebCore::RenderGrid::applyStretchAlignmentToTracksIfNeeded): Moved to
GridTrackSizingAlgorithm::stretchAutoTracks().
* rendering/RenderGrid.h:


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@221931 268f45cc-cd09-0410-ab3c-d52691b4dbfc

MXEBot pushed a commit to mirror/chromium that referenced this issue Sep 13, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#501249}
@mrego

This comment has been minimized.

Member

mrego commented Oct 13, 2017

JFTR, this seems to have caused a new issue #1866.

bertogg pushed a commit to Igalia/webkit that referenced this issue Oct 31, 2017

carlosgc@webkit.org
Merge r221931 - [css-grid] Stretching auto tracks should be done as p…
…art of the track sizing algorithm

https://bugs.webkit.org/show_bug.cgi?id=176783

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the test related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:

Source/WebCore:

CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
RenderGrid::applyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::stretchAutoTracks().
And then modifies the grid track sizing algorithm to execute
the new step.

This patch uses the WPT test updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::initializeTrackSizes): Initialize
the list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::stretchFlexibleTracks): Add
early return if there are not flexible tracks.
(WebCore::GridTrackSizingAlgorithm::stretchAutoTracks): Code moved from
RenderGrid::applyStretchAlignmentToTracksIfNeeded().
(WebCore::GridTrackSizingAlgorithm::setup): Reset list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::run): Add new step
stretchAutoTracks().
(WebCore::GridTrackSizingAlgorithm::reset): Reset auto tracks.
* rendering/GridTrackSizingAlgorithm.h: Add m_autoSizedTracksIndex.
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Avoid applying stretch here.
(WebCore::RenderGrid::contentAlignmentNormalBehaviorGrid): Make public.
(WebCore::RenderGrid::applyStretchAlignmentToTracksIfNeeded): Moved to
GridTrackSizingAlgorithm::stretchAutoTracks().
* rendering/RenderGrid.h:

git-svn-id: http://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.18@223364 268f45cc-cd09-0410-ab3c-d52691b4dbfc

rachelandrew added a commit to rachelandrew/web-platform-tests that referenced this issue Nov 8, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#501249}

jakearchibald added a commit to jakearchibald/web-platform-tests that referenced this issue Nov 16, 2017

[css-grid] Move stretch of "auto" tracks into the track sizing algorithm
CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
LayoutGrid::ApplyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::StretchAutoTracksIfNeeded().
And then modifies the grid track sizing algorithm to execute
the new step.

This also modifies the test to check the new expected behavior.

BUG=763386
TEST=external/wpt/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html

Change-Id: Ieb31e822989ae4655d0e632885215e4a032dd445
Reviewed-on: https://chromium-review.googlesource.com/657578
Commit-Queue: Manuel Rego Casasnovas <rego@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Cr-Commit-Position: refs/heads/master@{#501249}

ashkulz pushed a commit to ashkulz/webkit that referenced this issue Apr 13, 2018

carlosgc@webkit.org
Merge r221931 - [css-grid] Stretching auto tracks should be done as p…
…art of the track sizing algorithm

https://bugs.webkit.org/show_bug.cgi?id=176783

Reviewed by Sergio Villar Senin.

LayoutTests/imported/w3c:

Import changes on the test related to the new behavior.

* web-platform-tests/css/css-grid-1/grid-items/grid-minimum-size-grid-items-021.html:

Source/WebCore:

CSS WG has agreed to modify the track sizing algorithm to include
a new step: https://drafts.csswg.org/css-grid/#algo-stretch
We used to do the stretch of the "auto" tracks at the end of
the track sizing algorithm, however this change integrates it
into the algorithm itself as the last step.
See: w3c/csswg-drafts#1150

The patch moves the method
RenderGrid::applyStretchAlignmentToTracksIfNeeded() to
GridTrackSizingAlgorithm::stretchAutoTracks().
And then modifies the grid track sizing algorithm to execute
the new step.

This patch uses the WPT test updated to check the new behavior.

* rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::initializeTrackSizes): Initialize
the list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::stretchFlexibleTracks): Add
early return if there are not flexible tracks.
(WebCore::GridTrackSizingAlgorithm::stretchAutoTracks): Code moved from
RenderGrid::applyStretchAlignmentToTracksIfNeeded().
(WebCore::GridTrackSizingAlgorithm::setup): Reset list of auto tracks.
(WebCore::GridTrackSizingAlgorithm::run): Add new step
stretchAutoTracks().
(WebCore::GridTrackSizingAlgorithm::reset): Reset auto tracks.
* rendering/GridTrackSizingAlgorithm.h: Add m_autoSizedTracksIndex.
* rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Avoid applying stretch here.
(WebCore::RenderGrid::contentAlignmentNormalBehaviorGrid): Make public.
(WebCore::RenderGrid::applyStretchAlignmentToTracksIfNeeded): Moved to
GridTrackSizingAlgorithm::stretchAutoTracks().
* rendering/RenderGrid.h:

git-svn-id: http://svn.webkit.org/repository/webkit/releases/WebKitGTK/webkit-2.18@223364 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment