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-2] Used value of contain #6246

Closed
andruud opened this issue Apr 27, 2021 · 6 comments
Closed

[css-contain-2] Used value of contain #6246

andruud opened this issue Apr 27, 2021 · 6 comments
Labels
css-contain-2 Current Work

Comments

@andruud
Copy link
Member

andruud commented Apr 27, 2021

The spec says that content-visibility can "turn on" layout/style/etc containment, but it's not clear what effect (if any) this has on the computed/used value of contain.

I think the most logical thing would be to specify that if content-visibility is such-and-such, then the used value of contain becomes this-and-that, and specify that the resolved value is the used value.

I bring this up because the Blink implementation of content-visibility modifies the computed value of contain, which seems heavy-handed. Introducing computed-value time dependencies between properties comes at a price. We should avoid doing so unless it's crucial.

cc @vmpstr

@emilio
Copy link
Collaborator

emilio commented Apr 27, 2021

I agree, fwiw.

@tabatkins
Copy link
Member

I agree it shouldn't alter the computed value, and didn't intend for it to. Do we really need to expose the active set of containments in gCS()? (I'm fine if we do need to, I just want to make sure there's a need and it's not just taken for granted.)

@tabatkins tabatkins added the css-contain-2 Current Work label Apr 27, 2021
@tabatkins
Copy link
Member

(My intention, fwiw, is that it had no effect whatsoever on the 'contain' property. An element having certain containments is just a quality it can possess, and the 'contain' property is one way of applying them, not the only way. But if we think it's useful to communicate the current set of containments, then doing so via the resolved value of 'contain' does indeed seem like the most straightforward way to do so.)

@andruud
Copy link
Member Author

andruud commented Apr 27, 2021

As far as I'm concerned, not affecting contain at any value stage is even better. You're right that I just took it for granted that we wanted to expose the active containments. I don't know if it's really needed. @vmpstr: Any thoughts on that?

My intention, fwiw, is that it had no effect whatsoever on the 'contain' property

By default I would have interpreted the spec that way as well (after all it doesn't explicitly mention messing with the contain value). But the Blink impl. made me doubt what the intent was.

@vmpstr
Copy link
Member

vmpstr commented Apr 27, 2021

As long as the effects of containment still apply when the content-visibility spec says such containment is turned on (which is true), I don't really think we need to change contain value.

There may be a chance that developers rely on this as a way to know whether content-visibility: auto element is currently being skipped or not via presence of size containment. However, I don't think they should be, and the spec never intended that.

It seems like this is a "no change needed" for the spec, and simply a bug in Blink. WDYT?

@andruud
Copy link
Member Author

andruud commented Apr 27, 2021

no change needed [...], bug in Blink

Works for me.

@andruud andruud closed this as completed Apr 27, 2021
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 9, 2021
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 10, 2021
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 10, 2021
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}
chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this issue Aug 10, 2021
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}
pull bot pushed a commit to FreddyZeng/chromium that referenced this issue Aug 10, 2021
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Aug 14, 2021
… for locked elements, a=testonly

Automatic update from web-platform-tests
Don't modify computed value of 'contain' for locked elements

DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}

--

wpt-commits: 4c5408d354ca9eb460bec40db10ae941a20ec865
wpt-pr: 29948
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Aug 20, 2021
… for locked elements, a=testonly

Automatic update from web-platform-tests
Don't modify computed value of 'contain' for locked elements

DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}

--

wpt-commits: 4c5408d354ca9eb460bec40db10ae941a20ec865
wpt-pr: 29948
mjfroman pushed a commit to mjfroman/moz-libwebrtc-third-party that referenced this issue Oct 14, 2022
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}
NOKEYCHECK=True
GitOrigin-RevId: b4ac918d3ee3276004ccba806cea0ba1b49796b8
frivoal added a commit that referenced this issue Oct 26, 2022
This is a follow up on #6246, the prior edit was incomplete.
ns-rsilva pushed a commit to ns-rsilva/chromium that referenced this issue Apr 25, 2024
DisplayLockContext::AdjustElementStyle currently modifies the computed
value of 'contain'. It should not [1].

Instead we store a bit on ComputedStyle to indicate whether or not
the element should skip its contents [2], and use this flag used-
value-time to request the appropriate containment.

[1] w3c/csswg-drafts#6246
[2] https://drafts.csswg.org/css-contain-2/#skips-its-contents

Fixed: 1203627
Change-Id: I5571b0a5fac1b89903c03a256a2b144bddcd5040
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3081725
Commit-Queue: Anders Hartvoll Ruud <andruud@chromium.org>
Reviewed-by: vmpstr <vmpstr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#910192}

Former-commit-id: b4ac918d3ee3276004ccba806cea0ba1b49796b8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-contain-2 Current Work
Projects
None yet
Development

No branches or pull requests

4 participants