-
Notifications
You must be signed in to change notification settings - Fork 658
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] specified column/row sizes on grid containers should contribute to contain:size #2804
Comments
While grid and table are similar in some respects, there is one key difference that matters with regards to css-contain: tables cannot[1] be forced to be smaller than their min-content size. That makes it impossible for This is not a problem for grid, and so we should not need to call out grid specifically here. Now, specifically on the bug you found, this is a chrome bug and a clear spec violation. The main effect of
This means that you are correct:
Based on that, I don't believe any normative change to the spec is needed. Maybe a note could help, but I am not convinced, why list this particular situation, and not all the other cases? However, we should certainly add a test case to the test suite to make sure this is caught, and file a bug on chrome. [1]: With some partial exceptions when using |
cc: @gtalbot |
@frivoal one question about this, are there other elements that can have an intrinsic size different from zero if they have no contents? Grid Layout is special, because when you define the sizes of the tracks you're somehow defining the size of the grid container too. What's the use case for this example? Why someone wants to apply I'm doing all these questions because I've been taking a look to the issue and I'm not sure if it deserves the implementation complexity it might require. For widths it's simple, you compute the intrinsic/preferred widths like if you have no grid items, so only the tracks with fixed sizes are taken into account and you get the 100px width. But for heights, browsers don't have a phase to compute the intrinsic height, so we'd need to run the track sizing algorithm for rows ignoring the contents, and then run it again for the real layout. It's true we could get this information from the style without running the algorithm, as this is only relevant when the height is indefinite, still we don't have explicit code for that and I'm wondering if it'd be really useful. If there's not a strong use case, and grids are the only ones affected by this, maybe an option for grid container, would be to add them to the replaced elements sentence, saying that their intrinsic size is zero with size containment. Of course I might be missing many things, this was just a quick look, but before going deeper I want to understand the reasons behind it. |
Here's one: div {
position: absolute;
columns: 2 50px;
}
Maybe because they have only fixed tracks, and they trust the browser to do the math and don't want to (redundantly) define the total size manually?
I can see the trade-off. It seems to me that it would be useful. If you have some kind of card UI, where each card is laid out with a fixed-size grid, you want to be able to update the content of each card without thrashing the whole layout. You could handle it manually by setting the width / height on each card, but if you have already given sufficient information to calculate that in the form of a fixed sized grid, it would be really nicer to let the browser do the math. |
So I guess that if you add Any example where the intrinisic height can be defined from CSS in an element without contents?
Mmmm, I guess that if it's only for fixed sized tracks we could try to do something special for this, reading the information from the style and avoid running the track sizing algorithm twice. |
Well, there's also the 1em column gap, so 116px (assume default font size), but otherwise yes. |
I think we've got the tradeoff figured out, but I'd like to get the feedback from the WG. Agenda+ |
The Working Group just discussed
The full IRC log of that discussion<fantasai> Topic: Contain<Rossen> github:https://github.com//issues/2804 <fantasai> florian: When you apply size containment, we first lay out the element as if it's empty <fantasai> florian: And then put content into it <fantasai> florian: Your'e expected to give it a size <fantasai> florian: Question: for a grid containe,r which has rows and columns of fixed size <fantasai> florian: Even if element was empty, it would have a non-zero size <fantasai> florian: That's not how it's implemented atm <fantasai> florian: Currently asssumed to have zero size, even if 6 100px columns <fantasai> florian: We have same similar issue with multicol <fantasai> florian: Spec currently say that empty columns and grid rows should help auto-sized size-contained element not be zero <fantasai> florian: But #1 chrome isn't doing that and #2 fixing in inline direction, but in block direction wouldn't be easy <fantasai> florian: Because no codepath to get vertical intrinsic size without doing layout <fantasai> TabAtkins: I don't care a ton, if really problematic, but in general the idea is to "lay out as if empty" and for a grid, that means we make it the height of the fixed-height rows <fantasai> florian: If we remove it nothing's destroyed, just maybe a bit inconvnient <fantasai> ericwilligers: Isn't purpose of contain that author provides the size? <fantasai> florian: Author gave us the info, though 6 100px columns plus a gap, simple math the author could do, by we could also do <fantasai> TabAtkins: min-height is handled, why not sum of row heights <fantasai> ... <fantasai> florian: On Chrome side, Igalia would be fixing it. They would do it, but want to make sure we want it. <fantasai> heycam: In the original filed issue, Morgan suggested that it should work. So I think that would be Mozilla's position <fantasai> florian: I can add a note to the spec, to make sure it's not forgotten <fantasai> RESOLVED: No change, add a note. |
"Needs edits": for adding a note |
@mrego, do you know if there's a Blink bug filed on this? (honoring hardcoded row/column sizes when computing the intrinsic size of a contained grid) I thought I remembered filing one, but couldn't find it just now. |
Tests here: web-platform-tests/wpt#12866 Review appreciated. |
Yes the bug is https://bugs.chromium.org/p/chromium/issues/detail?id=855261 |
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 GridContainerSizeWithoutItems() that returns the size based only on the grid container style. As it should has the same size than the same grid container without contents. 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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 RunMode to GridTrackSizingAlgorithm(), so we can run only the initialization step instead of the full algorithm. Using that mode we can 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
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
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
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}
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}
…tainment on grid containers, a=testonly Automatic update from web-platform-tests [css-contain][css-grid] Support size containment on grid containers 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} -- wpt-commits: 21f4c6a787a7b211f5385aa98ad8150cdafef29f wpt-pr: 14302
…tainment on grid containers, a=testonly Automatic update from web-platform-tests [css-contain][css-grid] Support size containment on grid containers 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} -- wpt-commits: 21f4c6a787a7b211f5385aa98ad8150cdafef29f wpt-pr: 14302
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}
@dholbert @MatsPalmgren Firefox doesn't seem to fix this problem yet? |
@yisibl You can see the status here: https://bugzilla.mozilla.org/show_bug.cgi?id=1488878 |
…tainment on grid containers, a=testonly Automatic update from web-platform-tests [css-contain][css-grid] Support size containment on grid containers 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 <regoigalia.com> Reviewed-by: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Sergio Villar <svillarigalia.com> Cr-Commit-Position: refs/heads/master{#657678} -- wpt-commits: 21f4c6a787a7b211f5385aa98ad8150cdafef29f wpt-pr: 14302 UltraBlame original commit: 393b6eaf4a58ab1513c919b71aa6e18efedbcf7b
…tainment on grid containers, a=testonly Automatic update from web-platform-tests [css-contain][css-grid] Support size containment on grid containers 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 <regoigalia.com> Reviewed-by: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Sergio Villar <svillarigalia.com> Cr-Commit-Position: refs/heads/master{#657678} -- wpt-commits: 21f4c6a787a7b211f5385aa98ad8150cdafef29f wpt-pr: 14302 UltraBlame original commit: 393b6eaf4a58ab1513c919b71aa6e18efedbcf7b
…tainment on grid containers, a=testonly Automatic update from web-platform-tests [css-contain][css-grid] Support size containment on grid containers 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 <regoigalia.com> Reviewed-by: Javier Fernandez <jfernandezigalia.com> Reviewed-by: Sergio Villar <svillarigalia.com> Cr-Commit-Position: refs/heads/master{#657678} -- wpt-commits: 21f4c6a787a7b211f5385aa98ad8150cdafef29f wpt-pr: 14302 UltraBlame original commit: 393b6eaf4a58ab1513c919b71aa6e18efedbcf7b
I didn't see this explicitly messaged in the spec, and I wanted to check in because grid containers are a little similar to tables, and table elements are a special case in the spec.
I think grid elements which have content-relative sizing on their columns/rows (e.g grid-template-columns: auto) should be treated as if the sizing were zero when contain:size is applied. However, if the value is specified (e.g. grid-template-row: 20px) with contain:size applied, it should be treated as being explicitly sized at whatever the specified size is (or the sum of the specified sizes).
This would mean, for example, that an element with 'grid-template-columns: 20px auto;' would have a contained width of 20px. I noticed this wasn't the behaviour in Chrome's contain implementation (https://jsfiddle.net/9qcL6frj/3/ < test case) , but I also didn't see this mentioned in the spec and thought I'd check in case, like tables, contain:size shouldn't apply to grid containers (specifically those which are relatively sized).
The text was updated successfully, but these errors were encountered: