[css-position-4] Is the overlay property ready to ship?

[astearns]

I see an intent to ship for this property has been posted:

https://groups.google.com/a/chromium.org/g/blink-dev/c/YIgIas5_Q8s/m/okhgfSdlCgAJ

I think it would be useful for the Blink process to inform us, at least as a courtesy, when these intents are proposed for CSS features.

I have concerns that there has not yet been enough review on this very-recently-updated editor’s draft, so I am filing this issue to let everyone know they might want to spend some time looking at these changes. The intent to ship gets the details of the property’s values wrong, so the Blink contacts for this intent may also want to review the draft again.

There is still an active discussion in https://github.com/w3c/csswg-drafts/issues/8189, so the assertion that “the spec is done” in the related webkit/gecko standards positions issues is likely premature.

Please open any new issues on the draft that are needed (and link them here).

[cwilso]

"I think it would be useful for the Blink process to inform us, at least as a courtesy, when these intents are proposed for CSS features."

What would be the easiest/best/clearest/least disruptive way to do that? I want to make sure that we are not requiring something that is going to turn into a firehose the WG doesn't want. How can we turn this into a well-defined gate?

(And "does the CSS WG know about this feature" obviously is too loosely interpretable. :) )

[astearns]

Good question :)

The least disruptive way (which has been used quite often) is to make it part of the discussion in the last substantive issue. Either a comment or in minuted discussion just say something like, “We think this is the last issue that needs resolution before we announce an intent to ship” gives everyone a heads-up that (if they are interested) they should make a last pass over the spec and get any concerns they have raised quickly.

Another way (that also gets used from time to time) is to open an issue like this (or https://github.com/w3c/csswg-drafts/issues/7533 as another example). If the spec itself is not in a state where the whole thing can progress, we can make a determination that the group agrees a particular feature is ready. Even if that doesn’t happen, the issue might spur someone to give the spec another review.

The intents you all post are useful inputs to our process, and notifying us of relevant ones is welcome. The whole blink-dev list is a firehose I do not want to require everyone in the group to monitor, but I do want everyone in the group to be aware of your intents for the specifications we are working on.

[cwilso]

No, of course not, I wasn't suggesting "add css WG to the blink-dev list". :)

And @fantasai pointed me at a couple of other examples: Example: https://github.com/w3c/csswg-drafts/issues/6371 Example: https://github.com/w3c/csswg-drafts/issues/6887

It seems like perhaps something like "if this is still considered an unstable feature according to CSS wg, you should file an 'is this feature shippable' issue and link here" would be a good addition to the blink intent process?

[astearns]

Yes, if an upcoming intent to ship has not already been communicated in regular issue discussion.

I think it would also be good to wait on a ship intent until we have had the editor’s draft published (which is another one of our review checkpoints that we just got to today for this feature). Blink’s participants in the group can request publication at any time (we prioritize these requests over everything else).

[nt1m]

I want to raise the concern that a bunch of spec text landed without any prior review, notably the model described in this comment: https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1447989290

No resolution was made for this particular model, and it was never discussed in any CSSWG call. Yet it ended up in the CSS positioning spec, which is fairly disappointing, and a bunch of follow up PRs in fullscreen/HTML specs were opened as well.

The change from 1 to 2 element collections is a significant one. I also asked for clarification on why the new model is linked to rendering updates without any clear answer.

[chrishtr]

Hi @nt1m,

I want to raise the concern that a bunch of spec text landed without any prior review, notably the model described in this comment: #8189 (comment)

No resolution was made for this particular model, and it was never discussed in any CSSWG call.

It was discussed in a CSSWG calll, see here.

Yet it ended up in the CSS positioning spec, which is fairly disappointing, and a bunch of follow up PRs in fullscreen/HTML specs were opened as well.

Those were added to resolve the longstanding desire to move the definition of the top layer to CSS, via css-position-4, which was also discussed in the CSSWG as linked above, and also an earlier one a year or so ago.

The change from 1 to 2 element collections is a significant one.

I don't know what you mean by this, can you clarify? Is it the pending top layer removals list? There's relevant discussion on the various PRs about how this is a concept related to the algorithms that use the top layer (fullscreen, popover, etc), I can link you to Tab for more details on why it's written that way right now if you need it.

I also asked for clarification on why the new model is linked to rendering updates without any clear answer.

Could you clarify which question didn't receive an answer? I and others responded to all of your concerns and questions in #8189, as far as I can tell.

[tabatkins]

Yes, as far as I can tell I or Chris answered all the questions you raised, @nt1m (in https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1492399214 and https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1507524156), and you didn't respond any further . If we skipped something, please restate, with our apologies.

[nt1m]

It was discussed in a CSSWG calll, see https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1479815485.

From the notes, the model wasn't really covered in detail.

Yes, as far as I can tell I or Chris answered all the questions you raised, @nt1m (in https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1492399214 and https://github.com/w3c/csswg-drafts/issues/8189#issuecomment-1507524156), and you didn't respond any further . If we skipped something, please restate, with our apologies.

Ah I missed the last comment of #8189, sorry. I did ask for a new issue to be filed, since it takes time to review specs on our side in general, and having a proper place to do it without things fall in the void is generally good. Anyway, I guess we can re-use this issue.

unless you explicitly ask for immediate removal, removal instead just adds it to the pending removal list, and it's removed for real during HTML's Update The Rendering. This ensures that transitions added at the same time will get processed and can have an effect on the 'overlay' property, possibly delaying the actual removal. (Immediate removal should only be used for security-related things, or when CSS isn't relevant, like if the element has been removed from the document.)

This doesn't quite explain how this works relative to transitions, afaik, removing from the top layer kills the transition, so this means update the rendering will kill all pending removal transitions for top layer rendering at least, so this makes me question whether this model actually works. This is the main part I wanted clarification about.

In general the spec could use more precision around the role the overlay property actually plays in top layer rendering. That would also help understanding things around transition timing as well.

From reading the spec right now, adding to the top layer will trigger right away "rendered in the top layer" style adjustments, so this makes me question what the overlay property is useful for.

User agents may, at their discretion, remove a running transition on overlay. The conditions for this are intentionally undefined. (This is to prevent potential abuse scenarios where a transition: overlay 1e9s; or similar attempts to keep an element in the top layer permanently.)

I do think this should be defined, it is quite an important part of the feature design.

[fantasai]

The least disruptive way (which has been used quite often) is to make it part of the discussion in the last substantive issue.

Actually, I think we've usually had them as their own issue, although often the discussion groups several issues including whether we think it's OK to ship. And I think that's better (as its own issue, as in the examples Chris points out), both because the discussion can be more focused and less confused; and also because it requires its own set of edits (to the snapshot).

[tabatkins]

This doesn't quite explain how this works relative to transitions, afaik, removing from the top layer kills the transition, so this means update the rendering will kill all pending removal transitions for top layer rendering at least, so this makes me question whether this model actually works. This is the main part I wanted clarification about.

Yes, actually removing an element from the top layer bypasses anything that overlay might do. That's why access to the top layer is now gated behind several algorithms that "do the right thing" - you can modify the top layer directly still, but if you "request removal" it will just remove the overlay rule (reverting it to the default none, but possibly triggering a transition that keeps it as auto for a bit) and put it in the pending removals list. Then in "process top layer removals" we check if the element is both (a) pending removal, and (b) overlay:none before actually removing it from the top layer.

In general the spec could use more precision around the role the overlay property actually plays in top layer rendering. That would also help understanding things around transition timing as well.

Hm, what else do you want detail on in there? Happy to expand on this as necessary.

From reading the spec right now, adding to the top layer will trigger right away "rendered in the top layer" style adjustments, so this makes me question what the overlay property is useful for.

Correct, as soon as it's in the top layer list, it renders as part of the top layer (and stops doing so as soon as it's not in the list).

The 'overlay' property exists to allow authors to signal they want to delay removal from the top layer for some amount of time, by adding a transition. That's all it does.

[tabatkins]

I do think this should be defined, it is quite an important part of the feature design.

We often leave anti-abuse mechanisms undefined to allow for UAs to tweak them over time as proves necessary/prudent. I wouldn't be opposed to adding a liberal limit that you UAs must at least apply (but still allowing them to do shorter, or kill a transition for any other reason). However, the question is just whether it's actually something worthwhile to make testable.

[nt1m]

Yes, actually removing an element from the top layer bypasses anything that overlay might do. That's why access to the top layer is now gated behind several algorithms that "do the right thing" - you can modify the top layer directly still, but if you "request removal" it will just remove the overlay rule (reverting it to the default none, but possibly triggering a transition that keeps it as auto for a bit) and put it in the pending removals list. Then in "process top layer removals" we check if the element is both (a) pending removal, and (b) overlay:none before actually removing it from the top layer.

This makes sense, thanks.

The 'overlay' property exists to allow authors to signal they want to delay removal from the top layer for some amount of time, by adding a transition. That's all it does.

OK that clears up a lot of things, thanks. I assumed there was also a way to delay the entry based on the name of the other issue (entry/exit transitions for top layer), but thinking about it, I guess that use case isn't terribly useful.

Thanks for the clarifications! Makes it easier to review the proposal.

[tabatkins]

I've done a bit of rewriting in the 'overlay' section to hopefully make it clearer exactly what the role of the property is, too.

[tabatkins]

I've made a minor change - "rendered in the top layer" now checks both that it's in the top layer list and has overlay:auto. The only effect of this is that entry animations are now possible, as you can delay the none->auto transition to keep the element displaying in the page normally for a little bit. I've adjusted the language in the overlay section to make the effects clearer.

This brings up a new possibility, tho. If authors can delay the element both leaving and entering the top layer, I'm not sure the "only set by the UA" is important any longer. At this point, the spec's model is essentially "the UA and the author can both signal whether they want the element in the top layer or not, and it only changes state when both agree". (The UA signals by putting something in the top layer list or the pending top layer removals list; the author signals by controlling 'overlay'.)

As currently written, tho, the "overlay is only set by the UA; author can control by using a transition" is a little awkward. The entire purpose of setting a transition in the first place is to align it with an animation, but that means you have to, well, align a transition and an animation. I don't think this restriction ends up buying us anything, tho. If we just let the author set overlay normally, then they can set it in the animation and have it change at the desired time, alongside the rest of the animation, so they only have to maintain the entry/exit animation in one place.

The model is robust enough now that I don't think we're not protecting the author from anything important by not letting them set overlay. At worst an author could set overlay: none unconditionally to prevent anything from rendering in the top layer, or set overlay: auto unconditionally to prevent anything from leaving the top layer. (And they can do either of these already, by using an extremely long transition-duration.) I think the first isn't worth protecting against (it just means that they're preventing some features from working at all for themself, which isn't a security issue), and the second can be addressed in the same way as is currently allowed - give UAs the ability to just remove something from the top layer list, for example if it stays in the pending-removals list for too long.

So do we want to remove that restriction, and make the UA-set overlay value normal, rather than !important?

[nt1m]

I think it's positive that this looking more similar to a normal CSS property but @smfr pointed out this part is actually potentially a footgun:

At worst an author could set overlay: none unconditionally to prevent anything from rendering in the top layer, or set overlay: auto unconditionally to prevent anything from leaving the top layer.

If someone sets animation-fill-mode: forwards, they might accidentally do this with a @keyframes animation without realizing.

[tabatkins]

If someone sets animation-fill-mode: forwards, they might accidentally do this with a @keyframes animation without realizing.

Hm, could you elaborate on what you think is problematic about that case? If they have both entry and exit animations, and set overlay in both, this'll work as expected. If they only have one or the other, it'll be broken anyway, locked permanently in the "displayed as top layer" or "displayed as normal" styles.

The worst case I can see is them setting overlay in one of the animations but not the other, but they can do the same thing with any other property as well, breaking their page without our help. I don't think this is unique or particularly dangerous.