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

subtree-visibility CSS property #4843

Closed
chrishtr opened this issue Mar 6, 2020 · 16 comments
Closed

subtree-visibility CSS property #4843

chrishtr opened this issue Mar 6, 2020 · 16 comments
Labels
css-contain-2 Current Work

Comments

@chrishtr
Copy link
Contributor

chrishtr commented Mar 6, 2020

I would like CSSWG feedback on the proposed subtree-visibility CSS property.

subtree-visibility solves the same use cases as the previous rendersubtree element attribute that was presented to the CSSWG at TPAC 2019. [1] The changes are:

  • Now a CSS property with only 3 values (visible, auto, hidden).
  • hidden subtrees, and auto subtrees that are offscreen, are invisible; otherwise visible
  • "invisible" is semantically very similar to what visibility:hidden does today
  • auto subtrees are also exposed to accessibility and all UA features such as find-in-page
  • There is no activation event or updateRendering method
  • UA is encouraged but not required to skip rendering work for invisible subtrees

I'd appreciate feedback on all of the feature of course, but in particular:

  • Naming
  • Semantics related to boxes and flat trees

Chromium has implemented this CSS property fully, and if all goes well hoping to ship it. There is an explainer here. It can be turned on for testing via chrome://flags/#enable-experimental-web-platform-features.

Related issues include #4531 and #4229.

[1] Unofficial Meeting notes from Dael Jackson below:

rendersubtree

  • chrishtr introduced the proposal for a rendersubtree attribute.
    Proposal text:
    https://github.com/WICG/display-locking/blob/master/README.md
    Proposed PR: Rendersubtree attribute whatwg/html#4862
    Github issue: Add a rendersubtree attribute to elements whatwg/html#4861
  • The 'rendersubtree' attribute controls whether the subtree is
    laid out and rendered or not, in order to allow browsers to be
    more performant and authors to reserve space for an element that
    is not yet loaded. This also introduces an "activation" concept
    which allows the browser to choose to render.
  • Several members expressed concern that this felt more like a CSS
    property. The reason it was not written to be is because the UA
    is allowed to overwrite it.
  • The use case for 'invisible' and not 'activatable' is so the author
    can prevent not-yet-rendered content from being shown. It was
    suggested the default should be 'activatable' since authors might
    not think through all cases of 'invisible' if it's the default.
  • It was suggested to add callbacks for the activation actions so
    authors could listen for the changes.
  • The difference between this and 'contain: all' is that this would
    prevent animations from loading. Meanwhile, the difference to
    'display: none' is that the layout work is still done in this
    case.
  • More info at Add a rendersubtree attribute to elements whatwg/html#4861
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Feature introduction, subtree-visibility CSS property.

The full IRC log of that discussion <dael> Topic: Feature introduction, subtree-visibility CSS property
<dael> github: https://github.com//issues/4843
<dael> chrishtr: At TPAC 2019 I presented a dom attribute with purpose to allow browser to avoid rendering of stuff not drawn to screen. Based on feeback there and from other vendors since proposal has changed a lot. Semantics are about same but API shape changes to improve dev semantics and now fits well within css It's a css property called subtree-visibility. Semantics are about same as visibility:hidden. One difference is when content is skipped, aka not visible
<dael> chrishtr: element is contain:size. This property is meant to be paired with contain:intrinisic-size so when content is visible you can have placeholder size
<dael> chrishtr: 3 properties. Invisible controlled by the browser so when content is on screen or focused to it's visible or not skipped. If it's not visible on screen to user and not part of a focus or selection it's auto marked as skipped so browser is free to skip style layout. Not required but allowed
<dael> chrishtr: Can view this as another addition to contain spec where it's a way to augmenet contain so it's easier for browser to optimize work off screen
<dael> chrishtr: hidden is dev controlled and meant for vitual scrollers and single page app
<dael> chrishtr: Would like feedback. Is naming good? I think it's better b/c clear it's a hint. Semenatics are about boxes and block trees.
<dael> chrishtr: We've impl and you can try it on demos by enabling it in chromium
<dael> Rossen_: Thank you chrishtr. I'd encourage people to engage on the issue and refresh yourselves on the property and how it fits. If/When needed we'll bring back to WG call to make decisions

@atanassov atanassov removed the Agenda+ label Mar 11, 2020
@fantasai
Copy link
Collaborator

fantasai commented Mar 11, 2020

Chromium has implemented this CSS property fully, and if all goes well hoping to ship it.

It's not clear to me whether you're asking CSSWG to adopt the feature for standardization, or just to make some review comments quickly before some imminent shipping deadline... Is Chrome planning to write a consensus-backed spec and publish it through the CSSWG as an official feature before shipping it, assuming CSSWG approves your proposal, or is this issue just about getting a cursory review prior to to implementing and shipping off Chrome's explainer in the WICG?

@chrishtr
Copy link
Contributor Author

It's not clear to me whether you're asking CSSWG to adopt the feature for standardization, or just to make some review comments quickly before some imminent shipping deadline... Is Chrome planning to write a consensus-backed spec and publish it through the CSSWG as an official feature before shipping it, assuming CSSWG approves your proposal, or is this issue just about getting a cursory review prior to to implementing and shipping off Chrome's explainer in the WICG?

Yes I would like the CSSWG to adopt this spec soon, though I didn't ask for that in today's call. (The same goes for contain-intrinsic-size BTW.)

I don't just want a cursory review, I would like substantial feedback on this API. Further, there is an actual draft spec, not just an explainer.

While it's true that Chromium has implemented this feature and hopes to ship it soon, there has been a lot of public discussion and feedback from many parties on it and previous iterations, including the CSSWG. Further review and feedback is not just a formality, I really do want more feedback to identify any concerns or ways it could be improved.

@chrishtr
Copy link
Contributor Author

Hi all, any feedback on this CSS property?

Any issue with the name? Semantics?

Agenda+ to request final review of this feature's shape.

@chrishtr
Copy link
Contributor Author

Here are some potential issues to resolve:

WICG/display-locking#126 (is "auto" a good name for a non-default state?)

WICG/display-locking#127 (is "subtree" a good word to use?)

There are also some other open issues on the spec:
https://github.com/WICG/display-locking/issues

@fantasai
Copy link
Collaborator

fantasai commented Apr 8, 2020

@chrishtr IMHO auto is acceptable for a non-default state, its primary meaning is "this does some special non-straightforward calculation and/or is UA-defined". Wrt subtree, I agree with dbaron, it's not a term we use in CSS. Would suggest content instead, which we do use.

@FremyCompany
Copy link
Contributor

FWIW, I think this is a good proposal. Thanks @chrishtr et al. for working on this!

@tabatkins
Copy link
Member

Hmm, content-visibility isn't bad, yeah. It does indeed match the meaning of, say, display:contents, and still clearly indicates that it doesn't affect the visibility of the element itself. 👍

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed subtree-visibility CSS property, and agreed to the following:

  • RESOLVED: Move subtree-visibility into CSSWG
The full IRC log of that discussion <dael> Topic: subtree-visibility CSS property
<dael> github: https://github.com//issues/4843
<dael> chrishtr: Like to know if there's any other things to discuss on this property before settling on it
<dael> chrishtr: Agree syntax is okay and adopt it
<dael> AmeliaBR: Requesting approval of current spe text?
<dael> chrishtr: Yeah, linked in issue
<AmeliaBR> https://wicg.github.io/display-locking/index.html
<dael> fantasai: You have a draft. We can agree this is largely what we want. There's a pile of open issue before sign off but I don't think that's what you're asking for.
<dael> chrishtr: Like to know about general agreement on draft spec
<dael> AmeliaBR: This is WICG spec so officially adopt as CSS?
<dael> chrishtr: Yes or we handle like contain-intrinisic-size
<dael> cbiesinger: It's in sizing 4 isn't it?
<dael> fantasai: It's always been there.
<dael> chrishtr: I see. My bad. Okay.
<dael> Rossen_: Do we have at least 2 impl interested in moving this to CSS out of WICG?
<dael> Rossen_: My udnerstanding is one requirement to move out of WICG is it should be fairly stable which I think you're signaling. Other is that there are at least 2 implementor commitments to impl.
<dael> chrishtr: Confused since various other css have been written without that commitment.
<dael> Rossen_: It's WICG rules, not CSS. I'm fine to move this to CSS which is where work belongs.
<dael> Rossen_: Is that something group wants to do?
<dael> florian: WICG charter does not have strict rule on 2 implementors. They like it, but it's not a strict requirement.
<dael> fantasai: My position is if we're going to work on this in CSS we should move it in and do FPWD. If there are other browser vendors that believe this is bad instead of it's not a priority those concerns need to be raised.
<florian> +1 to fantasai
<dael> Rossen_: I think we can try and do that here and now. I believe we have 4 majro browsers represented.
<dael> Rossen_: Any objections to move the work into CSS as an official ED?
<dael> smfr: Mozilla has a position that says it's under review and notes a problem where it allows you to detect a11y enabled. Our internal review reveals we have concerns on the API but do not have a decision on it.
<smfr> https://github.com/mozilla/standards-positions/issues/135
<dael> smfr: I would encourage chrishtr to look at MOzilla feedback and add security/privacy section.
<dael> chrishtr: Those comments are out of date. Have conversations with Mozilla. There is security and privacy now and concerns are resolved.
<smfr> https://wicg.github.io/display-locking/index.html#priv-sec
<dael> smfr: Section is empty as far as I can tell
<dael> chrishtr: In explainer. Sorry. I agree needs to be added.
<dael> Rossen_: With that addition smfr are you okay with moving it to CSSWG?
<dael> smfr: I think so. We don't have intent to impl soon but don't object to the problem being solved.
<dael> Rossen_: You don't object to work being don't
<dael> smfr: Yeah, I think so
<dael> Rossen_: Objections to move subtree-visibility out of WICG?
<dael> dbaron: A bunch of other people from MOz have been looking and working with chrishtr so not sure current state. Not sure our position on interest to impl. But I think it's reasonable for the discussions to be in CSS WG
<dael> Rossen_: I'm hearing comments in favor with asterisk there's more work to be done. Given this is a CSS feature I think authors will get value of CSSWG being engaged.
<dael> Rossen_: Is Vlad a group member? To retain we need to move him over
<dael> TabAtkins: I'll help chrishtr with that
<dael> chrishtr: I'll work on that and security and privacy section next.
<dael> fantasai: Comment about renaming to subtree-visibility. I don't think it's nec for spec to match property name. Property might get renamed. It should be about what is concept of spec. Not property name. Just for shortname consideration.
<dael> dbaron: Display locking made sense for waht used to be there, not what's currently in.
<dael> fantasai: The shortname which is the file name.
<dael> AmeliaBR: It goes in the URL
<dael> chrishtr: I see. URL should agree with name of property?
<dael> fantasai: It doesn't have to
<dael> Rossen_: Can name spec display locking if that makes sense to explain feature, or something else, but it doesn't have to be property name.
<dael> florian: And there's an open issue on property name but doesn't block naming spec whatever we want.
<dael> chrishtr: Should I just pick a name and we rename later? We like URLs to be consistent.
<dael> fantasai: Talk with TabAtkins and others, look for a good short name, bring it to the call. Nice to start with a good name but we can set up redirects if we change.
<dael> Rossen_: Let's see if we can resolve. I've called for objections and didn't hear any. I think conversation reflects feedback about naming, editors, and privacy section. Once more, objections to move subtree-visibility into CSSWG?
<dael> RESOLVED: Move subtree-visibility into CSSWG
<dael> chrishtr: Great. Please take a read through and see if there's any way to improve spec or handle unhandled cases.
<dael> chrishtr: Example inkoverflow issue raised by Mozilla we were able to resolve.
<dael> Rossen_: Yep. I think being under CSSWG you'll get more engagement.
<dael> chrishtr: Excellent

@dholbert
Copy link
Member

dholbert commented Apr 8, 2020

One bit of feedback from reading the spec:

The concept of "skipped" is a bit fuzzy right now. The spec defines it in a descriptive way right now, as if it's a classification that some elements naturally fall into:

skipped: an element is considered skipped if its subtree is not painted or hit-tested.

... but then later it uses the term as if it's an classification that can be applied by the UA, which has prescriptive requirements (e.g. subtree-visibility:hidden is defined saying "The element is skipped.").

These usages need to be harmonized, probably by rewriting the definition of "skipped" such that it's prescriptive rather than descriptive. e.g. perhaps rephrasing as "if an element is skipped, it must not be painted or hit-tested [...]" or similar.

@dholbert
Copy link
Member

dholbert commented Apr 8, 2020

Another bit of feedback on a sentence in the green note below the property-definition:

The fact that off-screen elements with subtree-visibility: auto are skipped is a rendering performance optimization.

This wants s/that/that some/

(Otherwise, it can be misread as implying that all off-screen elements with subtree-visibility: auto will be "skipped". And that's not true, per the definition of "auto" - there are more requirements besides simply being off-screen.)

@fantasai
Copy link
Collaborator

fantasai commented Apr 19, 2020

We agreed to move subtree-visibility into the CSSWG. Propose to merge it into css-contain-2, as they are pretty tightly related.

@frivoal
Copy link
Collaborator

frivoal commented Apr 21, 2020

Moving it to contain-2 makes sense to me. Should Tab or I be taking an action to do this, as Editors of contain-2, or should we be getting a new co-editor?

@chrishtr
Copy link
Contributor Author

Vladimir Levin is joining the CSSWG, and would like to be a new co-editor of this spec. @vmpstr

@svgeesus svgeesus added the css-contain-2 Current Work label Apr 22, 2020
@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed subtree-visibility CSS property, and agreed to the following:

  • RESOLVED: add vmpstr as co-editor
  • RESOLVED: Move what is known as subtree-visibility into CSS Contain L2
  • RESOLVED: Make vm a co-editor of CSS Contain L2
  • RESOLVED: Rename to content-visibility
The full IRC log of that discussion <dael> Topic: subtree-visibility CSS property
<dael> github: https://github.com//issues/4843#issuecomment-611131666
<dael> Rossen___: Before we jump into issue we have vmpstr who is new to the group and going to co-edit. Welcome
<dael> Rossen___: Obj to add vmpstr as co-editor?
<dael> RESOLVED: add vmpstr as co-editor
<dael> TabAtkins: Asking to put property into contain L2 which than vmpstr is the co-editor of
<fantasai> topic of conversation is https://github.com//issues/4843#issuecomment-616206403
<dael> Rossen___: Last was moving it on its own. So this is going to go in to..
<dael> fantasai: Last resolution didn't say where. I flagged and suggested move into CSS Contain L2
<dael> vmpstr: Can we change the name to content visibility?
<dael> fantasai: Let's do sep.
<dael> Rossen___: Add it first and then rename. And than name again during LC
<dael> Rossen___: Objections to moving the spec into CSS Contain L2?
<dael> RESOLVED: Move what is known as subtree-visibility into CSS Contain L2
<astearns> q-
<dael> Rossen___: We'll make vmpstr a co-editor. Objections?
<astearns> vmpstr: renaming things at last call (close to the end of the specification process) is a running joke around here. Please don't plan on it :)
<dael> RESOLVED: Make vm a co-editor of CSS Contain L2
<dael> Rossen___: Last request was rename into content visibility?
<dael> vmpstr: Yes
<vmpstr> astearns: :)
<fantasai> s/content visibility/content-visibility/
<fantasai> +1
<dael> Rossen___: Opinions? Concerns? Obj?
<dael> RESOLVED: Rename to content-visibility
<dael> Rossen___: Is that it on the issue?

@chrishtr
Copy link
Contributor Author

Done.

rwlbuis added a commit to web-platform-tests/wpt that referenced this issue Feb 3, 2022
Remove subtree-visibility usage, this likely should be content-visibility:
w3c/csswg-drafts#4843
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

10 participants