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-sizing-4] Only apply contain-intrinsic-size: auto with content-visibility: auto #6308

Closed
cbiesinger opened this issue May 24, 2021 · 14 comments
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-sizing-4

Comments

@cbiesinger
Copy link

https://drafts.csswg.org/css-sizing-4/#intrinsic-size-override

If auto applies to everything, you can into the situation where an element is visible on screen but rendered with outdated dimensions. For example:

<div style="contain-intrinsic-size: auto 100px; float: left; width: 200px;"></div>

Now, if JS adds contain: size; width: auto;, the element will be rendered at 200px width, even though this size is neither in the current CSS style not is it related to the content size.

This is an unfortunate situation. To fix this, we would like to propose only applying auto if the element also has content-visibility: auto. This would ensure that the remembered size is only used if the element is offscreen, where an outdated size will only affect the scrollbar size.

Thoughts?

@dbaron @chrishtr @vmpstr @tabatkins

@chrishtr
Copy link
Contributor

I think this is a good change, because offscreen placeholder sizing is the only use case we've defined for this feature. And it's very important to provide a way to utilize content-visibility, and this use case, this without script, so that pages can have excellent progressive rendering, and improve performance & accessibility.

Also, for developers who want to have the same behavior without content-visibility: auto, there is always the option to use a RezizeObserver.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed [css-sizing-4] Only apply contain-intrinsic-size: auto with content-visibility: auto, and agreed to the following:

  • RESOLVED: only apply contain-intrinsic-size: auto with content-visibility: auto
The full IRC log of that discussion <dael> Topic: [css-sizing-4] Only apply contain-intrinsic-size: auto with content-visibility: auto
<dael> github: https://github.com//issues/6308
<dael> cbiesinger: With contain-intrinsic-size: auto. you can get into situation where current size of element is based on css prop that's no longer set on element.
<dael> cbiesinger: Set explicit width, remove it, explicit width could still be rememebred and applied.
<dael> cbiesinger: Weird situation. Main use case for contain-intrinsic-size: auto is when in combo with contain-vsicility: auto
<dael> cbiesinger: Suggestion is to make it only apply with contain-visibility: auto which means this will not be visable for user but address main use case
<dael> cbiesinger: What does WG think. Is this a problem we care about and, if is, is this a good solution
<Rossen_> q?
<dael> TabAtkins: Makes sense to me. Happy to do this.
<dael> emilio: Can you remind me how we made apply contain-intrinsic-size: auto work? Does it only change behavior on the screen?
<dael> emilio: Can you remind me of how we made content-visibility: auto work? Does it change just the used value?
<dael> cbiesinger: Not sure offhand
<dael> emilio: Depending on how that works this doesn't solve the problem or solves it.
<dael> emilio: If the computed value remains auto proposed solution...needs to be more subtle
<dael> chrishtr: Saying script looks at bounding client rect of the offscreen element it would notice old width?
<dael> emilio: Yes, but not complaining. When have content-visbility: auto and becomes visible do we change content-visbility style?
<dael> chrishtr: No. Contain-size is removed
<dael> cbiesinger: Spec only changes used value of contain
<dael> emilio: Then this doesn't solve issue, does it? Need to be only for content-visibility: auto that are offscreen
<dael> cbiesinger: If it's on screen cotnain size won't apply so c-i-s doesn't apply
<TabAtkins> To be clear: it has no effect on the used value of 'contain'. It simply applies containments, *independent* of the 'contain' property.
<dael> emilio: Makes sense. Wanted that clarified
<dael> Rossen_: That satisfies your concern emilio?
<dael> emilio: Yeah. Seems reasonable
<dael> Rossen_: Other opinions or concerns?
<dael> emilio: Another question- if main use case...why do we want to make this special case? I understand it can cause weird behavior. If authors not expected to use c-i-s:auto with things that don't have c-v:auto...do we need to special case?
<dael> chrishtr: Arguement for is to avoid poss of element on screen that dev is confused about. Only use case we know of is placeholder sizing when element is offscreen.
<dael> chrishtr: Had to say if would be a big problem in practice
<dael> emilio: I guess somebody could come with creative use cases. I don't know. I would prefer to not special case if we don't have evidence this is really confusing. not a hill I want to die on
<dael> cbiesinger: I don't really care so much myself. TAG and someone else had concerns and prefered the change. I'm happy not change if WG thinks we don't need to
<dael> emilio: TAG is generally smarter then me. If they think would be confusing I'm okay
<dael> Rossen_: cbiesinger what was the context of the review?
<dael> cbiesinger: Review auto value of c-i-s. I can find a link to the review
<dael> Rossen_: We can link in the issue later
<cbiesinger> https://github.com/w3ctag/design-reviews/issues/624
<dael> Rossen_: There is some thumbs up and lots of not really caring about how this goes one way or the other. I want to call for objections to resolving on this
<dael> Rossen_: We can always come back, but not hearing strong pushback. Proposed behavior does make sense and will improve the behavior
<dael> Rossen_: Objections to only apply contain-intrinsic-size: auto with content-visibility: auto
<dael> RESOLVED: only apply contain-intrinsic-size: auto with content-visibility: auto
<chrishtr> Thanks all!

@tabatkins
Copy link
Member

Whoops, didn't link the edits for this issue to this thread: 22a5e0e

@cbiesinger
Copy link
Author

Reopening as discussed, that edit does not affect c-v: auto

@cbiesinger cbiesinger reopened this Dec 2, 2021
@chrishtr
Copy link
Contributor

chrishtr commented Dec 2, 2021

Didn't commit 22a5e0e clarify that c-v: auto is the only feature affected by the auto behavior?

tabatkins added a commit that referenced this issue Dec 2, 2021
…element is skipping its contents, so you don't have the chance of a phantom size.
@tabatkins
Copy link
Member

No, my mistake, that was an irrelevant commit, it was just about the fact that an element can gain and lose a last remembered size several times. It has nothing to do with the "phantom size visibly sticking around" problem that Christian is talking about here.

The edit I just committed (6eb6faa) is directly handling that, tho.

@chrishtr
Copy link
Contributor

chrishtr commented Dec 2, 2021

I left a comment on that commit..

aarongable pushed a commit to chromium/chromium that referenced this issue Dec 2, 2021
As per w3c/csswg-drafts#6308, we should
only use the last remembered size when we have `content-visibility: auto`
and are currently hidden.

R=vmpstr@chromium.org

Bug: 1199460
Change-Id: I35e884d0b0ffab538fee20311da8dcb79d580da5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3311271
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#947670}
tabatkins added a commit that referenced this issue Dec 2, 2021
…n on-screen c-v:hidden element using a phantom size would also still be bad.
rwlbuis added a commit to rwlbuis/WebKit that referenced this issue Jun 8, 2022
https://bugs.webkit.org/show_bug.cgi?id=238867

Reviewed by NOBODY (OOPS!).

This patch adds support for contain-intrinsic-size value "none | <length>". The value "auto && <length>"
is not supported, because per [1] it depends on "content-visibility:auto" which is not supported yet.
Per [2], the properties allow elements with size containment to specify an explicit intrinsic inner size,
which is determined in RenderBox. These values will affect the intrinsic width calculation in computeIntrinsicLogicalWidths.
As to intrinsic height, the value will be overridden at the point after layout children and before logical height calculation.
For grid layout, we need to use explicit intrinsic inner size to calculate auto repetition, if width/height is auto.

[1] w3c/csswg-drafts#6308
[2] https://www.w3.org/TR/css-sizing-4/#intrinsic-size-override

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-003-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-004-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-005-expected.txt:
* Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::computeGridContainerIntrinsicSizes): Set explicitIntrinsicInnerLogicalSize to m_minContentSize and m_maxContentSize.
(WebCore::GridTrackSizingAlgorithm::run): Don't skip stretching tracks for size containment if there is explicitIntrinsicInnerLogicalSize.
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeIntrinsicLogicalWidths const): Ditto
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::computeIntrinsicLogicalWidths const): Ditto
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::updateLogicalHeight): Set explicitIntrinsicHeight to logicalHeight if it exists.
(WebCore::RenderBox::computeLogicalHeightWithoutLayout const): We should take account of explicitIntrinsicInnerLogicalHeight here.
(WebCore::RenderBox::hasExplicitIntrinsicInnerWidth const):
(WebCore::RenderBox::explicitIntrinsicInnerWidth const):
(WebCore::RenderBox::hasExplicitIntrinsicInnerHeight const):
(WebCore::RenderBox::explicitIntrinsicInnerHeight const):
* Source/WebCore/rendering/RenderBox.h:
(WebCore::RenderBox::explicitIntrinsicInnerLogicalWidth const):
(WebCore::RenderBox::explicitIntrinsicInnerLogicalHeight const):
* Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::computeIntrinsicLogicalWidths const): Ditto
* Source/WebCore/rendering/RenderFileUploadControl.cpp:
(WebCore::RenderFileUploadControl::computeIntrinsicLogicalWidths const): Ditto
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeIntrinsicLogicalWidths const): Ditto
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Should use explicitIntrinsicInnerHeigth as the definite size if exists.
(WebCore::RenderGrid::computeIntrinsicLogicalWidths const): Don't add gutterSize if it's explicitIntrinsicInnerLogicalSize.
(WebCore::RenderGrid::computeTrackSizesForIndefiniteSize const): Ditto.
(WebCore::RenderGrid::explicitIntrinsicInnerLogicalSize const): Ditto.
(WebCore::RenderGrid::computeAutoRepeatTracksCount const): Use explicitIntrinsicInnerLogicalSize as availableSize if the size of grid container is not specified
(WebCore::RenderGrid::computeEmptyTracksForAutoRepeat const): The tracks of grid container with size containment are not empty if hasExplicitIntrinsicInnerLogicalSize.
* Source/WebCore/rendering/RenderGrid.h:
* Source/WebCore/rendering/RenderListBox.cpp:
(WebCore::RenderListBox::computeIntrinsicLogicalWidths const): Ditto.
(WebCore::RenderListBox::computeLogicalHeight const): Use explicitIntrinsicInnerLogicalHeight as estimated height not the sum of item height.
* Source/WebCore/rendering/RenderMenuList.cpp:
(RenderMenuList::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const): Don't let size containment effect aspect ratio.
* Source/WebCore/rendering/RenderReplaced.h: Return explicitIntrinsicInnerSize if exits.
* Source/WebCore/rendering/RenderSlider.cpp:
(WebCore::RenderSlider::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderTextControl.cpp:
(WebCore::RenderTextControl::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderVideo.cpp:
(WebCore::RenderVideo::calculateIntrinsicSize): Ditto.
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::rareNonInheritedDataChangeRequiresLayout): The changes of contain-intrinsic-size should require layout.
cathiechen added a commit to cathiechen/WebKit that referenced this issue Jun 25, 2022
        https://bugs.webkit.org/show_bug.cgi?id=238867

        Reviewed by NOBODY (OOPS!).

        This patch adds support for contain-intrinsic-size value "none | <length>". The value "auto && <length>"
        is not supported, because per [1] it depends on "content-visibility:auto" which is not supported yet.
        Per [2], the properties allow elements with size containment to specify an explicit intrinsic inner size,
        which is determined in RenderBox. These values will affect the intrinsic width calculation in computeIntrinsicLogicalWidths.
        As to intrinsic height, the value will be overridden at the point after layout children and before logical height calculation.
        For grid layout, we need to use explicit intrinsic inner size to calculate auto repetition, if width/height is auto.

        [1] w3c/csswg-drafts#6308
        [2] https://www.w3.org/TR/css-sizing-4/#intrinsic-size-override

        * rendering/GridTrackSizingAlgorithm.cpp:
        (WebCore::GridTrackSizingAlgorithm::computeGridContainerIntrinsicSizes): Set explicitIntrinsicInnerLogicalSize to m_minContentSize and m_maxContentSize.
        (WebCore::GridTrackSizingAlgorithm::run): Don't skip stretching tracks for size containment if there is explicitIntrinsicInnerLogicalSize.
        * rendering/RenderBlock.cpp:
        (WebCore::RenderBlock::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderBlockFlow.cpp:
        (WebCore::RenderBlockFlow::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderBox.cpp:
        (WebCore::RenderBox::updateLogicalHeight): Set explicitIntrinsicHeight to logicalHeight if it exists.
        (WebCore::RenderBox::computeLogicalHeightWithoutLayout const): We should take account of explicitIntrinsicInnerLogicalHeight here.
        (WebCore::RenderBox::hasExplicitIntrinsicInnerWidth const):
        (WebCore::RenderBox::explicitIntrinsicInnerWidth const):
        (WebCore::RenderBox::hasExplicitIntrinsicInnerHeight const):
        (WebCore::RenderBox::explicitIntrinsicInnerHeight const):
        * rendering/RenderBox.h:
        (WebCore::RenderBox::explicitIntrinsicInnerLogicalWidth const):
        (WebCore::RenderBox::explicitIntrinsicInnerLogicalHeight const):
        * rendering/RenderDeprecatedFlexibleBox.cpp:
        (WebCore::RenderDeprecatedFlexibleBox::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderFileUploadControl.cpp:
        (WebCore::RenderFileUploadControl::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderFlexibleBox.cpp:
        (WebCore::RenderFlexibleBox::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderGrid.cpp:
        (WebCore::RenderGrid::layoutBlock): Should use explicitIntrinsicInnerHeigth as the definite size if exists.
        (WebCore::RenderGrid::computeIntrinsicLogicalWidths const): Don't add gutterSize if it's explicitIntrinsicInnerLogicalSize.
        (WebCore::RenderGrid::computeTrackSizesForIndefiniteSize const): Ditto.
        (WebCore::RenderGrid::explicitIntrinsicInnerLogicalSize const): Ditto.
        (WebCore::RenderGrid::computeAutoRepeatTracksCount const): Use explicitIntrinsicInnerLogicalSize as availableSize if the size of grid container is not specified
        (WebCore::RenderGrid::computeEmptyTracksForAutoRepeat const): The tracks of grid container with size containment are not empty if hasExplicitIntrinsicInnerLogicalSize.
        * rendering/RenderGrid.h:
        * rendering/RenderListBox.cpp:
        (WebCore::RenderListBox::computeIntrinsicLogicalWidths const): Ditto.
        (WebCore::RenderListBox::computeLogicalHeight const): Use explicitIntrinsicInnerLogicalHeight as estimated height not the sum of item height.
        * rendering/RenderMenuList.cpp:
        (RenderMenuList::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderReplaced.cpp:
        (WebCore::RenderReplaced::computeIntrinsicRatioInformation const): Don't let size containment effect aspect ratio.
        * rendering/RenderReplaced.h: Return explicitIntrinsicInnerSize if exits.
        * rendering/RenderSlider.cpp:
        (WebCore::RenderSlider::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderTextControl.cpp:
        (WebCore::RenderTextControl::computeIntrinsicLogicalWidths const): Ditto.
        * rendering/RenderVideo.cpp:
        (WebCore::RenderVideo::calculateIntrinsicSize): Ditto.
        * rendering/style/RenderStyle.cpp:
        (WebCore::rareNonInheritedDataChangeRequiresLayout): The changes of contain-intrinsic-size should require layout.
@Loirooriol
Copy link
Contributor

I was confused because from the spec it seems that contain-intrinsic-size: auto can be used with content-visibility: hidden, but some WPT and this resolution say it's only with content-visibility: auto.

According to 6eb6faa#r61269377

No, the resolution was worded confusingly to refer to the property value, when it meant the effect; if I did just gate it on the property value, Christian's issue would still apply exactly as stated, with a visible element having a phantom size. The intention was that it needed to only apply while the element was being skipped.

Christian and I discussed whether it made sense to only apply it while being skipped as a result of c-v:auto or in any skip, and we couldn't come up with a good reason to be restrictive.

So in https://software.hixie.ch/utilities/js/live-dom-viewer/saved/10484 I expect both elements to keep being 50px tall when gaining content-visibility: auto or hidden. But in Chromium content-visibility: hidden results in 1px, ignoring the auto in contain-intrinsic-size.

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 20, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should apply
to all elements that skip their contents, not just content-visibility: auto
elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jul 20, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777917
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030610}
aarongable pushed a commit to chromium/chromium that referenced this issue Aug 2, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777917
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030610}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 2, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777917
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030610}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 4, 2022
…to content-visibility: hidden as well, a=testonly

Automatic update from web-platform-tests
CIS: Apply contain-intrinsic-size: auto to content-visibility: hidden as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777917
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030610}

--

wpt-commits: d8fcd31ebcb01dcf78530bab3aa173c18666b602
wpt-pr: 34915
@tabatkins tabatkins added Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. labels Aug 24, 2022
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
As per w3c/csswg-drafts#6308, we should
only use the last remembered size when we have `content-visibility: auto`
and are currently hidden.

R=vmpstr@chromium.org

Bug: 1199460
Change-Id: I35e884d0b0ffab538fee20311da8dcb79d580da5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3311271
Commit-Queue: Christian Biesinger <cbiesinger@chromium.org>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Auto-Submit: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#947670}
NOKEYCHECK=True
GitOrigin-RevId: 5509c164b9858a843e4d054b76a87c724a84e3e1
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
… as well

Based on discussion on
w3c/csswg-drafts#6308 (comment)

and based on the spec as written, contain-intrinsic-size auto should
apply to all elements that skip their contents, not just
content-visibility: auto elements.

This patch does this.

R=chrishtr@chromium.org

Change-Id: Ibf38be014ae25ff64af027f6c1608caa7ded3514
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3777917
Reviewed-by: Oriol Brufau <obrufau@igalia.com>
Commit-Queue: Vladimir Levin <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1030610}
NOKEYCHECK=True
GitOrigin-RevId: 694e3bf075c754e2156cc5ec68b83408dc855f0b
webkit-early-warning-system pushed a commit to cathiechen/WebKit that referenced this issue Oct 25, 2022
https://bugs.webkit.org/show_bug.cgi?id=238867

Reviewed by Alan Bujtas.

This patch adds support for contain-intrinsic-size value "none | <length>". The value "auto && <length>"
is not supported, because per [1] it depends on "content-visibility:auto" which is not supported yet.
Per [2], the properties allow elements with size containment to specify an explicit intrinsic inner size,
which is determined in RenderBox. These values will affect the intrinsic width calculation in computeIntrinsicLogicalWidths.
As to intrinsic height, the value will be overridden at the point after layout children and before logical height calculation.
For grid layout, we need to use explicit intrinsic inner size to calculate auto repetition, if width/height is auto.

[1] w3c/csswg-drafts#6308
[2] https://www.w3.org/TR/css-sizing-4/#intrinsic-size-override

* LayoutTests/TestExpectations:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-003-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-004-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/auto-005-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-009-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-029-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-030-expected.txt: The three failed cases are related to css scrollbar-gutter.
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-031-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-032-expected.txt:
* LayoutTests/imported/w3c/web-platform-tests/css/css-sizing/contain-intrinsic-size/contain-intrinsic-size-logical-003-expected.txt:
* Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:
(WebCore::GridTrackSizingAlgorithm::computeGridContainerIntrinsicSizes): Set explicitIntrinsicInnerLogicalSize to m_minContentSize and m_maxContentSize.
(WebCore::GridTrackSizingAlgorithm::run): Don't skip stretching tracks for size containment if there is explicitIntrinsicInnerLogicalSize.
* Source/WebCore/rendering/RenderBlock.cpp:
(WebCore::RenderBlock::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderBox.cpp:
(WebCore::RenderBox::updateLogicalHeight): Set explicitIntrinsicHeight to logicalHeight if it exists.
(WebCore::RenderBox::computeLogicalHeightWithoutLayout const): We should take account of explicitIntrinsicInnerLogicalHeight here.
(WebCore::RenderBox::explicitIntrinsicInnerWidth const):
(WebCore::RenderBox::explicitIntrinsicInnerHeight const):
* Source/WebCore/rendering/RenderBox.h:
(WebCore::RenderBox::explicitIntrinsicInnerLogicalWidth const):
(WebCore::RenderBox::explicitIntrinsicInnerLogicalHeight const):
* Source/WebCore/rendering/RenderDeprecatedFlexibleBox.cpp:
(WebCore::RenderDeprecatedFlexibleBox::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderFileUploadControl.cpp:
(WebCore::RenderFileUploadControl::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderFlexibleBox.cpp:
(WebCore::RenderFlexibleBox::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderGrid.cpp:
(WebCore::RenderGrid::layoutBlock): Should use explicitIntrinsicInnerHeigth as the definite size if exists.
(WebCore::RenderGrid::computeIntrinsicLogicalWidths const): Don't add gutterSize if it's explicitIntrinsicInnerLogicalSize.
(WebCore::RenderGrid::computeTrackSizesForIndefiniteSize const): Ditto.
(WebCore::RenderGrid::explicitIntrinsicInnerLogicalSize const): Ditto.
(WebCore::RenderGrid::computeAutoRepeatTracksCount const): Use explicitIntrinsicInnerLogicalSize as availableSize if the size of grid container is not specified.
(WebCore::RenderGrid::computeEmptyTracksForAutoRepeat const): The tracks of grid container with size containment are not empty if hasExplicitIntrinsicInnerLogicalSize.
* Source/WebCore/rendering/RenderGrid.h:
* Source/WebCore/rendering/RenderListBox.cpp:
(WebCore::RenderListBox::computeIntrinsicLogicalWidths const): Ditto.
(WebCore::RenderListBox::computeLogicalHeight const): Use explicitIntrinsicInnerLogicalHeight as estimated height not the sum of item height.
* Source/WebCore/rendering/RenderMenuList.cpp:
(RenderMenuList::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderReplaced.cpp:
(WebCore::RenderReplaced::computeIntrinsicRatioInformation const): Don't let size containment effect aspect ratio.
* Source/WebCore/rendering/RenderReplaced.h: Return explicitIntrinsicInnerSize if exits.
* Source/WebCore/rendering/RenderSlider.cpp:
(WebCore::RenderSlider::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderTextControl.cpp:
(WebCore::RenderTextControl::computeIntrinsicLogicalWidths const): Ditto.
* Source/WebCore/rendering/RenderVideo.cpp:
(WebCore::RenderVideo::calculateIntrinsicSize):
* Source/WebCore/rendering/style/RenderStyle.cpp:
(WebCore::rareNonInheritedDataChangeRequiresLayout): The changes of contain-intrinsic-size should require layout.

Canonical link: https://commits.webkit.org/255971@main
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed Accepted by CSSWG Resolution Commenter Satisfied Commenter has indicated satisfaction with the resolution / edits. css-sizing-4
Projects
None yet
Development

No branches or pull requests

6 participants