Use CSS spec for top layer #9093

josepharhar · 2023-03-30T18:14:31Z

The top layer has been added to the CSS spec, and this patch moves the top layer references from the fullscreen spec to the CSS spec and uses new algorithms to add and remove from the top layer defined in the CSS spec.

Moving the top layer to the CSS spec has been discussed in these issues:
w3c/csswg-drafts#4998
w3c/csswg-drafts#6939
w3c/csswg-drafts#7845
w3c/csswg-drafts#8240

At least two implementers are interested (and none opposed):
- Chrome
- WebKit
Tests are written and can be reviewed and commented upon at:
- Add a test for fullscreen for the A/B/A case web-platform-tests/wpt#39828
Implementation bugs are filed:
- Chromium: Passes test
- Gecko: Passes test
- WebKit: https://bugs.webkit.org/show_bug.cgi?id=258035
MDN issue is filed: Fullscreen reordering behavior mdn/content#27303

/infrastructure.html ( diff )
/interaction.html ( diff )
/interactive-elements.html ( diff )
/popover.html ( diff )
/webappapis.html ( diff )

josepharhar · 2023-03-30T18:14:46Z

@annevk FYI

nt1m · 2023-03-30T18:20:26Z

Is there a corresponding fullscreen spec PR?

josepharhar · 2023-03-30T18:21:16Z

The fullscreen PR is underway but not ready yet. I will post a link to it when it is ready.

tabatkins · 2023-03-30T19:53:07Z

Yeah I'll be writing the fullscreen PR momentarily, when I've finished shifting all the relevant bits of it out into Position 4.

annevk · 2023-03-31T08:36:46Z

Great to see this happen in my lifetime! Thanks for tackling it @tabatkins and @josepharhar.

This seems to change the semantics for add. Is that intended? Does that have test coverage?

If el is not contained doc’s top layer

(In the CSS draft.) Has a grammar issue.

Now as for this PR: it doesn't seem to call "process top layer removals". It should as otherwise we have a leak. This also needs some amount of test coverage.

josepharhar · 2023-03-31T16:14:36Z

Now as for this PR: it doesn't seem to call "process top layer removals". It should as otherwise we have a leak. This also needs some amount of test coverage.

Ah I forgot that, I just pushed a commit to add it.

I also forgot to call remove from top layer immediately when any element is removed from the document, and I forgot to add a new user-agent style rule which prevents authors from triggering inconsistent states. Both of these have been added in the new commit.

mfreed7 · 2023-03-31T17:07:39Z

This seems to change the semantics for add. Is that intended? Does that have test coverage?

I believe you're referring to the removal of the "bubbling" behavior, where re-adding something to the top layer moves it to the top of the stack?

If that's correct, then the reason for this removal was that it was "dead code" in the spec. All entrypoints to the "add" algorithm did a pre-check of one sort or another that ensured this bubbling was never executed. I believe all of those behaviors (e.g. calling showModal() on an already-modal <dialog>) have existing WPTs.

nt1m

I'm a bit disappointed the CSS top layer spec wasn't defined to be a 1:1 replacement for the fullscreen top layer spec. I'm not necessarily opposed to these changes, but makes it harder to review. I have a lot of questions notably around the removal model.

tabatkins · 2023-03-31T18:29:47Z

The spec text I put into CSS was based on Rune's outline of behavior in #8189.

The removal model defined there ensures that presence in the top layer is synced to rendering behavior; you can't drop out of the top layer list while still being painted in the top layer. (At least, not without invoking the "remove immediately" algo, which should be called only when necessary, such as the element being removed from the document, which also makes the painting update.)

nt1m · 2023-03-31T19:10:32Z

@tabatkins Can we put that spec text through CSSWG discussion potentially in w3c/csswg-drafts#4998? The CSSWG resolution was specifically about the overlay property, afaik this other part hasn't been reviewed nor discussed. I'd like to spend some time reviewing this text. Some things that would be helpful on the issue are:

Why is there a need to sync the top layer with rendering updates? (Is it to allow for exit transitions while limiting the duration to rendering updates? or is there a different reason?)
Some more detail into how the model detailed by Rune helps with transitions in general.

In the meanwhile, it would be nice if the spec matched the fullscreen spec (+ what the overlay property that was resolved on by the CSSWG if you want), rather than Chrome's implementation, these things that are fundamental to top layer are missing:

::backdrop
The style adjustments detailed here: https://fullscreen.spec.whatwg.org/#new-stacking-layer

Thanks for taking the time to move the top layer spec to CSS btw, very much appreciated! Sorry I didn't look into Rune's comment in more detail before, it's hard to review an issue that covers so many things at once :)

tabatkins · 2023-03-31T20:36:14Z

Can we put that spec text through CSSWG discussion potentially in w3c/csswg-drafts#4998?

Sure

these things that are fundamental to top layer are missing:

Those were all put in yesterday afternoon.

annevk · 2023-04-01T06:35:11Z

If that's correct, then the reason for this removal was that it was "dead code" in the spec.

Did someone do the archeology in Fullscreen to determine when it became "dead code" there? I'm pretty sure I added it for a reason.

mfreed7 · 2023-04-01T11:54:34Z

If that's correct, then the reason for this removal was that it was "dead code" in the spec.

Did someone do the archeology in Fullscreen to determine when it became "dead code" there? I'm pretty sure I added it for a reason.

Hmm, I did not, and it’d probably be really good if someone verified that I’m right about it being dead. Perhaps there’s a corner case somewhere I missed.

tabatkins · 2023-04-03T20:15:01Z

Diving backwards in blame, that algorithm was added to fullscreen in whatwg/fullscreen@766dc87. As part of that, the "fullscreen an element" algorithm was simplified to no longer manually "move it already present", and instead just called the "add to top layer" algo.

This text is still present (https://fullscreen.spec.whatwg.org/#fullscreen-an-element), so per spec, the change to "add to top layer" that I put into Position would result in a behavior change - if you opened a modal dialog, then a second one on top, then fullscreened the first, it would be fullscreen under the second modal dialog. (Which is undesirable.)

So either I need to revert that "add to top layer" change to match what Fullscreen previously said, or Fullscreen needs editting to restore the "move if already present" behavior manually. I'm going to assume that fixing Position is the right move for now; if all the other calling locations do manually implement the "remove and re-add" behavior anyway, having Position's algo do it as well won't affect them.

foolip · 2023-04-06T18:46:50Z

@tabatkins moving if already present like the note in https://fullscreen.spec.whatwg.org/#top-layer-add says seems like the simpler approach here. It's hard to imagine a call site to "add to top layer" that isn't expecting the element to end up on top.

mfreed7 · 2023-04-11T21:21:14Z

This text is still present (https://fullscreen.spec.whatwg.org/#fullscreen-an-element), so per spec, the change to "add to top layer" that I put into Position would result in a behavior change - if you opened a modal dialog, then a second one on top, then fullscreened the first, it would be fullscreen under the second modal dialog. (Which is undesirable.)

Yep, you're right that I missed that (very!) corner case. We already discussed (PR here) throwing exceptions when two APIs are used at the same time. What about if we just throw an exception in the above corner case. I.e. requestFullscreen() after showModal()? @jakearchibald

Edit: it was recently pointed out that the fullscreen spec already outlaws dialog elements, so the above example (dialog A/dialog B/fullscreen A) isn't actually an example of a problem. I've come back to my position that the "pop to top" behavior is dead code.

lilles · 2023-04-14T12:33:37Z

Assuming I'm reading this correctly ...

With the current css-position-4 draft and this PR, the top layer ordering of the two dialogs A and B below will depend on the presence of an overlay transition on A because the check that a dialog is already present in the top layer is kept in this PR:

A.showModal();
B.showModal();
A.close();
A.showModal();

tabatkins · 2023-04-14T18:17:31Z

Yeah, that niggled at me when I was making the fullscreen edits too. I should add an algo for checking if something is in the top layer, that pays attention to the pending removals, so we don't have this odd behavior.

tabatkins · 2023-04-14T20:08:12Z

And done, https://w3c.github.io/csswg-drafts/css-position-4/#in-the-top-layer now checks if it's in the pending removals list too, so your example code will correctly put A on top.

Also add a test for re-adding dialog elements to the top layer during an overlay transition. whatwg/html#9093 Bug: 1411264 Change-Id: Ic9dcd45cd2224995c1f64a7ae5caa01e788d17ee

Also add a test for re-adding dialog elements to the top layer during an overlay transition. whatwg/html#9093 Bug: 1411264 Change-Id: Ic9dcd45cd2224995c1f64a7ae5caa01e788d17ee Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4431857 Auto-Submit: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131588}

Also add a test for re-adding dialog elements to the top layer during an overlay transition. whatwg/html#9093 Bug: 1411264 Change-Id: Ic9dcd45cd2224995c1f64a7ae5caa01e788d17ee Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4431857 Auto-Submit: Rune Lillesveen <futhark@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Rune Lillesveen <futhark@chromium.org> Cr-Commit-Position: refs/heads/main@{#1131588}

nt1m · 2023-05-12T18:20:37Z

What's not entirely clear to me is if that comes with behavior changes or not. It would help if OP makes a statement about that one way or another.

Yes, there are behavior changes: web-platform-tests/wpt#39828 https://wpt.fyi/results/fullscreen/api/fullscreen-reordering.html

Do they involve only fullscreen or also dialogs & popovers?

josepharhar · 2023-05-12T20:55:11Z

Do they involve only fullscreen or also dialogs & popovers?

I believe it only involves fullscreen as represented by the WPT i linked

nt1m · 2023-05-12T21:03:06Z

As I commented on whatwg/fullscreen#223, I don't think the fullscreen behavior changes are necessary (notably the entry ones), we have exceptions that cover top layer conflicts? Please let me know if I'm missing something however.

source

annevk · 2023-05-19T12:47:28Z

source

+  <h3>The top layer</h3>
+
+  <p>The <span>top layer</span> is defined by CSS, and HTML needs some top layer behavior to apply
+  to all elements as follows:</p>


This seems a tad weird. Why does this not apply to SVG, MathML, or arbitrary elements?

Do we have a test where a null namespace element does have overlay?

Why does this not apply to SVG, MathML, or arbitrary elements?

Why does this sentence imply that the top layer logic does not apply to SVG etc. elements? Is it the namespace in the following CSS that does it?

Do we have a test where a null namespace element does have overlay?

Is this the right way to create a null namespace element? document.createElementNS(null, 'div')

Yeah, the namespace makes that restriction.

And yeah, that's the right way. You also want to test an arbitrary string as namespace, for completeness.

I am adding a WPT here: web-platform-tests/wpt#40171

It looks like this user-agent style rule is intended to apply to all elements, not just HTML elements, so I am removing the @namespace from this

See also w3c/csswg-drafts#8573

See whatwg/html#9093 for HTML changes. Co-authored-by: Anne van Kesteren <annevk@annevk.nl> Co-authored-by: Philip Jägenstedt <philip@foolip.org>

This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b

source

This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b

This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555988 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1150875}

josepharhar · 2023-06-05T15:19:51Z

Anything else I can do to move this forward? @nt1m @annevk

annevk · 2023-06-06T11:26:02Z

OP needs to be filled out. You can count WebKit as supporting this. If implementations are failing tests we need implementation bugs. If no MDN issue has been filed for this general movement of top layer that would be good to do as well.
It looks like the PR needs rebasing again. In general I'd recommend not using merge commits for that, but instead force push. (Though still use fixup commits for review comments at least until the reviewer has seen the changes.)

Hope that helps!

…testonly Automatic update from web-platform-tests Add WPT for overlay user-agent rules This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555988 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1150875} -- wpt-commits: 363864de8e79f2a88a9ea97841e4c396384b7900 wpt-pr: 40171

josepharhar · 2023-06-13T17:08:36Z

OP needs to be filled out. You can count WebKit as supporting this. If implementations are failing tests we need implementation bugs. If no MDN issue has been filed for this general movement of top layer that would be good to do as well.

Done

It looks like the PR needs rebasing again. In general I'd recommend not using merge commits for that, but instead force push. (Though still use fixup commits for review comments at least until the reviewer has seen the changes.)

Done

annevk · 2023-06-14T09:12:57Z

source

+  <h3>The top layer</h3>
+
+  <p>The <span>top layer</span> is defined by CSS, and HTML needs some top layer behavior to apply
+  to all elements as follows:</p>
+
+  <pre><code class="css">*|* {
+  overlay: none !important
+}</code></pre>


This is already required by the CSS specification we rely on. We shouldn't be duplicating that.

So should the CSS spec have this user agent style rule in it instead of having it here?

It's required in the property definition: https://drafts.csswg.org/css-position-4/#overlay.

We can't duplicate normative requirements. So either that is done differently or we remove this. It makes some sense for it to be in CSS, although user agent style sheet requirements appearing all over the place is not great for implementers. (See also #9093 (comment).)

cc @tabatkins

Ah, I see it here in that spec: https://drafts.csswg.org/css-position-4/#overlay:~:text=overlay%3A%20none%20!important

I removed this user-agent style from the PR

…testonly Automatic update from web-platform-tests Add WPT for overlay user-agent rules This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555988 Reviewed-by: Rune Lillesveen <futhark@chromium.org> Commit-Queue: Joey Arhar <jarhar@chromium.org> Auto-Submit: Joey Arhar <jarhar@chromium.org> Cr-Commit-Position: refs/heads/main@{#1150875} -- wpt-commits: 363864de8e79f2a88a9ea97841e4c396384b7900 wpt-pr: 40171

…testonly Automatic update from web-platform-tests Add WPT for overlay user-agent rules This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4555988 Reviewed-by: Rune Lillesveen <futharkchromium.org> Commit-Queue: Joey Arhar <jarharchromium.org> Auto-Submit: Joey Arhar <jarharchromium.org> Cr-Commit-Position: refs/heads/main{#1150875} -- wpt-commits: 363864de8e79f2a88a9ea97841e4c396384b7900 wpt-pr: 40171 UltraBlame original commit: 7cacc3ee42bfcc339942fa4bdf95af8719309de8

annevk

Thanks @josepharhar, @tabatkins, and everyone else who helped out with this!

josepharhar requested a review from foolip March 30, 2023 18:14

annevk added topic: rendering integration labels Mar 31, 2023

annevk mentioned this pull request Mar 31, 2023

::backdrop and :fullscreen now defined in CSS specs whatwg/fullscreen#221

Open

nt1m reviewed Mar 31, 2023

View reviewed changes

This was referenced Apr 13, 2023

The overlay CSS property mozilla/standards-positions#777

Open

The overlay CSS property WebKit/standards-positions#169

Open

chromium-wpt-export-bot mentioned this pull request Apr 17, 2023

Move overlay tests for dialog/popover into wpt web-platform-tests/wpt#39570

Merged

This was referenced May 17, 2023

[css-position-4] redundant document argument for top layer manipulation algorithms w3c/csswg-drafts#8849

Closed

Remove top layer definitions, now that CSS Position 4 contains them. whatwg/fullscreen#223

Merged

annevk reviewed May 19, 2023

View reviewed changes

chromium-wpt-export-bot mentioned this pull request May 23, 2023

Add WPT for overlay user-agent rules web-platform-tests/wpt#40171

Merged

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 23, 2023

Add WPT for overlay user-agent rules

fe57c4c

This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b

zcorpan reviewed May 24, 2023

View reviewed changes

source Outdated Show resolved Hide resolved

chromium-wpt-export-bot pushed a commit to web-platform-tests/wpt that referenced this pull request May 30, 2023

Add WPT for overlay user-agent rules

b9319a7

This was asked for in the HTML PR here: whatwg/html#9093 (comment) Change-Id: I94e6960803a3fa9c6e1da4dc4393691b1c969c4b

squash and rebase

c1cebe8

josepharhar force-pushed the toplayer branch from 78d2edb to c1cebe8 Compare June 13, 2023 16:14

josepharhar mentioned this pull request Jun 13, 2023

Fullscreen reordering behavior mdn/content#27303

Open

annevk reviewed Jun 14, 2023

View reviewed changes

remove overlay user-agent stylesheet

90dedc8

annevk approved these changes Jun 21, 2023

View reviewed changes

annevk merged commit 8430871 into whatwg:main Jun 21, 2023
2 checks passed

josepharhar mentioned this pull request Sep 4, 2023

Top layer research #4633

Open

Use CSS spec for top layer #9093

Use CSS spec for top layer #9093

Conversation

josepharhar commented Mar 30, 2023 • edited by pr-preview bot

josepharhar commented Mar 30, 2023

nt1m commented Mar 30, 2023

josepharhar commented Mar 30, 2023

tabatkins commented Mar 30, 2023

annevk commented Mar 31, 2023

josepharhar commented Mar 31, 2023

mfreed7 commented Mar 31, 2023

nt1m left a comment

Choose a reason for hiding this comment

tabatkins commented Mar 31, 2023

nt1m commented Mar 31, 2023

tabatkins commented Mar 31, 2023

annevk commented Apr 1, 2023

mfreed7 commented Apr 1, 2023

tabatkins commented Apr 3, 2023

foolip commented Apr 6, 2023

mfreed7 commented Apr 11, 2023 • edited

lilles commented Apr 14, 2023

tabatkins commented Apr 14, 2023

tabatkins commented Apr 14, 2023

nt1m commented May 12, 2023

josepharhar commented May 12, 2023

nt1m commented May 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josepharhar commented Jun 5, 2023

annevk commented Jun 6, 2023

josepharhar commented Jun 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

annevk left a comment

Choose a reason for hiding this comment

josepharhar commented Mar 30, 2023 •

edited by pr-preview bot

mfreed7 commented Apr 11, 2023 •

edited