Skip to content
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-contain][css-grid] Support size containment on grid containers #16854

Closed
wants to merge 1 commit into from

Conversation

@lukebjerring
Copy link
Contributor

lukebjerring commented May 15, 2019

We were always sizing grid containers with "contain: size" as 0x0.
However this is wrong as discussed on the following GitHub issue:
w3c/csswg-drafts#2804

To do this we add a new method to GridTrackSizingAlgorithmStrategy
called IsComputingSizeContainment() to determine the cases
in which we're computing the size containment dimensions.
That way we can run only the initialization step instead
of the full algorithm to get the expected track sizes
for size containment.

For widths the change is pretty simple
in LayoutGrid::ComputeIntrinsicLogicalWidths().
For heights we don't have a phase to compute the intrinsic size
so apart from the changes in LayoutGrid::UpdateBlockLayout()
we need some extra checks in LayoutBox too.

There is a minor change in test contain-size-grid-002.html,
because after this patch it was not passing yet due to some overflow
that is unrelated to the purpose of the test.

BUG=855261
TEST=external/wpt/css/css-contain/contain-size-grid-002.html
TEST=external/wpt/css/css-contain/contain-size-grid-003.html

Change-Id: I4d0fd417183068518070721afde84efdbfe1fcb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1355923
Commit-Queue: Manuel Rego rego@igalia.com
Reviewed-by: Javier Fernandez jfernandez@igalia.com
Reviewed-by: Sergio Villar svillar@igalia.com
Cr-Commit-Position: refs/heads/master@{#657678}

We were always sizing grid containers with "contain: size" as 0x0.
However this is wrong as discussed on the following GitHub issue:
w3c/csswg-drafts#2804

To do this we add a new method to GridTrackSizingAlgorithmStrategy
called IsComputingSizeContainment() to determine the cases
in which we're computing the size containment dimensions.
That way we can run only the initialization step instead
of the full algorithm to get the expected track sizes
for size containment.

For widths the change is pretty simple
in LayoutGrid::ComputeIntrinsicLogicalWidths().
For heights we don't have a phase to compute the intrinsic size
so apart from the changes in LayoutGrid::UpdateBlockLayout()
we need some extra checks in LayoutBox too.

There is a minor change in test contain-size-grid-002.html,
because after this patch it was not passing yet due to some overflow
that is unrelated to the purpose of the test.

BUG=855261
TEST=external/wpt/css/css-contain/contain-size-grid-002.html
TEST=external/wpt/css/css-contain/contain-size-grid-003.html

Change-Id: I4d0fd417183068518070721afde84efdbfe1fcb4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1355923
Commit-Queue: Manuel Rego <rego@igalia.com>
Reviewed-by: Javier Fernandez <jfernandez@igalia.com>
Reviewed-by: Sergio Villar <svillar@igalia.com>
Cr-Commit-Position: refs/heads/master@{#657678}
@lukebjerring lukebjerring requested a review from Hexcles May 15, 2019
@Hexcles Hexcles closed this May 15, 2019
@Hexcles Hexcles deleted the chromium-export-cl-1355923 branch May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.