Remove top layer definitions, now that CSS Position 4 contains them.

[tabatkins]

Also reword algos as appropriate.

As far as Fullscreen is concerned, this is almost a no-op change, as things are merely moving. The only normative change that's relevant here is in the "fullscreen an element" algo, as it's no longer valid to add something to the top layer when it's already there (see next para for details). At the moment I've changed it to simply check if it's already in the top layer and only add if it's not, but this can be changed as necessary. (There are some other changes to the top-layer handling as it exists in Position 4, but they're in response to CSSWG decisions and are being discussed over there.)

The issue with adding something to the top layer that's already there is that, now that we have multiple things interacting with the top layer, they can have complex interactions of their own, and arbitrarily rearranging an element put in the top layer by some other process isn't necessarily safe. For example, a popover can be part of a popover stack, with each attached to the element below. Fullscreening the bottom popover must not just move that bottom popover to the end of the top layer and leave the rest of the popovers behind; it has to do something else, probably closing the popover (and thus closing its associated stack).

I'm not 100% sure how we want to handle the interaction of these separate features. I suspect it'll have to be something like each defining its own "hey you're being removed from the top layer" algo, which the others can invoke before they add the element to the top layer for their own purposes. But this'll need more thought; I went with what I felt was the simplest model for right now, where if you fullscreen something already in the top layer for other reasons, it just gets its flag and that's that.

(This is technically a behavior change per spec, but doesn't appear to be a behavior change in practice, at least in Chrome - if you fullscreen an element, show a modal, then ask to fullscreen the first element again, it does not move on top of the dialog, it stays where it is. But opening a dialog then fullscreening an element does successfully put the fullscreen over the dialog. I'm having trouble getting Firefox to successfully fullscreen something so I can test this.)

/cc @mfreed7 @josepharhar @nt1m @annevk

---

Currently this PR won't build because I'm using a new Bikeshed metadata to ignore the MDN issue; I should have a release out supporting this new metadata either today or tomorrow. (It won't build without the metadata either, since it's a fatal error to be missing an MDN anchor, too.).

Potentially we could kill both by keeping the ID in the spec, but just having it explicitly say that the definition has been moved to CSS Position. Would that be better?

More generally, for all the IDs that are being moved to Position, do you want to keep them in the spec with a redirect section?

---

• At least two implementers are interested (and none opposed):
  • Chrome
  • Safari
• 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: Already Chrome's behavior.
  • Gecko: https://bugzilla.mozilla.org/show_bug.cgi?id=1834542
  • WebKit: https://bugs.webkit.org/show_bug.cgi?id=257199
• MDN issue is filed: ::backdrop is now defined by CSS Position 4, not Fullscreen mdn/browser-compat-data#19367 for redirecting ::backdrop to Position; don't think anything else needs tweaking.

---

Preview | Diff

[foolip]

@tabatkins the Bikeshed build is still failing. Did you release a new Bikeshed yet?

[foolip]

This is where the important behavioral change (not moving to the top of top layer) is. Once settled, can you add a note here describing the non-obvious consequences if the element is already in top layer? Right now it might be:

Note: If element is already in the top layer, it will not be moved, and may be obscured by other elements later in top layer.

[tabatkins]

It seems like the behavior we're going for, actually, is that an element can only be in the top layer for one reason at a time, so actually this might need to change a little more to properly reflect that. (And possibly allow it to be moved if it's currently in the top layer for fullscreen reasons, as per your comment.)

[nt1m]

We already have safeguards (exceptions notably) for this so we shouldn't change behavior unless necessary. Dialogs can't be fullscreened, Popovers can't be fullscreened (with #204 merged at least). Fullscreen elements also can't be made popovers (check popover validity algorithm prevents it). They can't be made dialogs either since you can't change the tag name.

[foolip]

I think here we should first check if the element is in top layer and if so remove it immediately. That will have the effect of moving it to the top of the stack.

[tabatkins]

Okay, I've gone ahead and applied this - when fullscreening, we first request removal then add it, so it'll cycle to the end if it was already there.

[foolip]

I think the prose here looks good and it's just a question of what the behavior should be in two cases. For the "already in top layer case", I think it's a very rare situation in practice, but there are the situations I can think of worth testing:

• Requesting fullscreen on the same element twice (should be unaffected)
• Switching between two elements in fullscreen by calling a.requestFullscreen() and b.requestFullscreen() in turns, over and over. This would have worked before this change, but not any longer.
• An open <dialog> or popver element in top layer in some position other than the top, so most likely nested popovers and requesting fullscreen on a popover that isn't currently on top.

@tabatkins what do you think about the second case here?

To also respond to your PR description:

I'm having trouble getting Firefox to successfully fullscreen something so I can test this.

Do you have the test code available that you were trying?

Potentially we could kill both by keeping the ID in the spec, but just having it explicitly say that the definition has been moved to CSS Position. Would that be better?

Do you have open PRs for all specs that link to the IDs that will go away? If so I think it's fine to let the cross-spec links be broken for a short time.

More generally, for all the IDs that are being moved to Position, do you want to keep them in the spec with a redirect section?

I would say no. @annevk how have you done things for other specs?

[tabatkins]

Did you release a new Bikeshed yet?

No, I was blocked on whatwg/meta#271, but I've gone ahead and unblocked myself there. The Bikeshed release is now public, 3.11.22.

Do you have the test code available that you were trying?

I was just doing el.requestFullScreen() in the console on the Fullscreen spec's logo, along with a manually inserted dialog element.

Do you have open PRs for all specs that link to the IDs that will go away? If so I think it's fine to let the cross-spec links be broken for a short time.

No, I need to figure out how to get this info, I think it's available from WebRef.

[tabatkins]

Requesting fullscreen on the same element twice (should be unaffected)

Yes, it's unaffected afaict.

Switching between two elements in fullscreen by calling a.requestFullscreen() and b.requestFullscreen() in turns, over and over. This would have worked before this change, but not any longer.

Hm, we can make this work if necessary.

An open or popver element in top layer in some position other than the top, so most likely nested popovers and requesting fullscreen on a popover that isn't currently on top.

You already can't fullscreen a dialog, and #204 closes popovers when an element fullscreens.

[foolip]

Thanks @tabatkins, this now builds!

I've poked #204 to see if we can get that landed, although it doesn't block this PR.

I think we can put interactions between fullscreen and other top layer-using features to the side, since as you point out <dialog> already can't go fullscreen, and popovers are new so there's no site compat risk to consider.

That leaves pure uses of the Fullscreen API with multiple elements. I think the conservative thing to do is to conserve the existing behavior where we move an element within the top layer. I'm not aware of real-world uses of it, but it's not especially far-fetched and the web is big.

I think the most straightforward approach is to call "remove an element from the top layer immediately" followed by " an element to the top layer", is that what you think we should do?

[tabatkins]

Yeah, if we currently have compat with A.rFS(); B.rFS(); A.rFS(); working and ending with A fullscreened on top, then I think we should indeed just edit that behavior in. I suspect the check should be that A is in the top layer and has the fullscreen flag, but isn't the doc's fullscreen element, then we remove and re-add it so it cycles to the top.

If it's already in the top layer but doesn't have the fullscreen flag, that means some other feature has put it in and we probably shouldn't touch it, at least not until we have the cross-feature agreement figured out.

[foolip]

OK, I've tested the A.rFS(); B.rFS(); A.rFS(); scenario with this test case:

<style>
div {
    padding: 1em;
    display: grid;
    place-items: center;
}
</style>
<div id="A" style="background-color:lavender">
 <span>This is A. <button onclick="fs(event)">go/toggle fullscreen</button></span>
</div>
<div id="B" style="background-color: lemonchiffon">
 <span>This is B. <button onclick="fs(event)">go/toggle fullscreen</button></span>
</div>
<script>
const divs = Array.from(document.querySelectorAll('div'));
function fs(event) {
    let target = event.target.closest('div');
    if (target === document.fullscreenElement) {
        // Toggle between the two divs when already fullscreen.
        target = divs.find((div) => div !== document.fullscreenElement);
    }
    console.log(`Requesting fullscreen for ${target.id}`);
    target.requestFullscreen().then(() => {
        console.log(`Success for ${target.id}`);
    }, (error) => {
        console.log(`Error for ${target.id}: ${error.message}`);
    });
}
</script>

As it turns out, only Chrome supports toggling between fullscreen elements like this.

Firefox fails the toggle request with "Request for fullscreen was denied because requesting element is not a descendant of the current fullscreen element."

Safari also rejects the requestFullscreen() call, without a message, but it must be this descendant check in the source:
https://github.com/WebKit/WebKit/blob/f462028e09b021b5d2768fee9ea0ffc056565b94/Source/WebCore/dom/FullscreenManager.cpp#L224-L230

In other words, the original hierarchy restrictions are still in Gecko and WebKit, see #140 for further links.

@josepharhar @mfreed7 do you have preferences on how we handle this?

[annevk]

@annevk how have you done things for other specs?

We do sometimes leave a note or small appendix for cases like this. See https://fetch.spec.whatwg.org/#websocket-protocol (and in particular the source). I think that is warranted here as well given how long Fullscreen hosted top layer.

[foolip]

@tabatkins can you update this to leave a section preserving the top layer IDs pointing to their new location in https://drafts.csswg.org/css-position-4/?

[tabatkins]

IDs added. I think we're just waiting on final confirmation of the decision in #223 (comment), then?

[foolip]

@tabatkins that's right, let me ping folks about that.

[foolip]

To help decide on the flip-flop case I checked how <dialog> behaves with this test:

<dialog id="A" style="background-color:lavender">
    <span>This is A. <button onclick="show(event)">open/toggle dialog</button></span>
</dialog>
<dialog id="B" style="background-color: lemonchiffon">
    <span>This is B. <button onclick="show(event)">open/toggle dialog</button></span>
</dialog>
<button onclick="show(event)">open/toggle dialog</button>
<script>
const dialogs = Array.from(document.querySelectorAll('dialog'));
function show(event) {
    let dialog = dialogs.find((dialog) => !dialog.open);
    if (!dialog) {
        // Try to open the first one anyway, expect it to fail.
        dialog = dialogs[0];
    }
    console.log(`Showing dialog ${dialog.id}`);
    try {
        dialog.showModal();
        console.log(`Success for ${dialog.id}`);
    } catch (error) {
        console.log(`Error for ${dialog.id}: ${error.message}`);
    }
}
</script>

These are sibling <dialog> elements. Chrome, Firefox and Safari all open A and then B, but won't open A again because it's already open.

To get the equivalent behavior for the fullscreen API, we should reject if the element is already in top layer stack, but we shouldn't add back a hierarchy restriction.

@josepharhar @mfreed7 would that work for you?

[mfreed7]

To get the equivalent behavior for the fullscreen API, we should reject if the element is already in top layer stack, but we shouldn't add back a hierarchy restriction.

@josepharhar @mfreed7 would that work for you?

If this ends up being web compatible, I'm definitely supportive. It feels right to:

1. Try to match other top layer APIs like dialog and popover, generally. That includes:
2. Reject/throw/no-op when a "second" top layer API call is made while an element is already in the top layer.
3. Allow fullscreen to be flexible and not require elements to be DOM-nested in order to both use fullscreen.

[foolip]

@mfreed7 sounds good! Let's hammer out the details of point 2 then, what happens for a second call to elm.requestFullscreen() in different scenarios.

Bottom line first: no-op success when requesting fullscreen again matches current behavior. That's what this PR already does, so no changes required.

The investigation:

One scenario is calling requestFullscreen() twice in the same click event handler:

button.onclick = () => {
  const p1 = div.requestFullscreen();
  const p2 = div.requestFullscreen();
  // Does p2 reject and why?
}

This is tested in element-request-fullscreen-twice.html and Chrome, Firefox and Safari all reject the second promise. Per spec (with #153 just merged) it should reject because the user gesture was consumed by the first call.

Another scenario is calling requestFullscreen() on the element that's already fullscreen, in a second click event handler:

button.onclick = () => {
  const p = div.requestFullscreen();
  // Does p reject if div is document.fullscreenElement?
}

This is tested in element-request-fullscreen-same-element.html. That times out in Safari, I don't know why, so I tested this manually:

button.onclick = () => {
    if (!document.fullscreenElement) {
        div.requestFullscreen().then(() => {
            console.log('1st request succeeded');
        }, (error) => {
            console.log('1st request failed:', error.message);
        });
    } else {
        div.requestFullscreen().then(() => {
            console.log('2nd request succeeded');
        }, (error) => {
            console.log('2nd request failed:', error.message);
        });
    }
}

I've found that Chrome, Firefox and Safari all resolve the 2nd promise, in other words it's a no-op.

[foolip]

For testing, a think a single new test would cover this PR. It should have two sibling divs, A and B. Request fullscreen once per click in order A-B-A and assert that all requests succeed but the last is a no-op and that document.fullscreenElement is still B. Bonus points for exiting fullscreen once and asserting that document.fullscreenElement is then A again, and then once more to make document.fullscreenElement null.

[nt1m]

@nt1m yes, that's exactly the A-B-A behavior I referred to, as an A-B-A sequence of requests is necessary to observe the behavior in question. I think we should preserve the existing spec behavior, which matches Chromium but not Gecko+WebKit. Is that your position too?

Yes, pretty sure that's how WebKit implements it already: https://searchfox.org/wubkat/rev/17b431999014409e4831dce71e69e0033d87ede5/Source/WebCore/dom/FullscreenManager.cpp#514-517

(If the test is failing, it's probably failing for different reasons fwiw)

[mfreed7]

(If the test is failing, it's probably failing for different reasons fwiw)

I believe it’s failing because WebKit doesn’t allow A and B to be siblings.

[foolip]

I've discussed the invocation style of the algorithms in https://drafts.csswg.org/css-position-4/#top-manip with @annevk. We've converged on something like this being best for Fullscreen and HTML:

Request removal from the top layer given element's node document and element

I've filed w3c/csswg-drafts#8849 about the document argument, but for now I'll add suggestions to this PR and whatwg/html#9093 to align on this style. If w3c/csswg-drafts#8849 is resolved, it will be trivial edits to remove the document argument.

[foolip]

A bunch of style changes to keep this spec's more archaic style, and also to match how the algorithms are invoked with how it's done in HTML.

[foolip]

There are conflicts from #204, I'll try to resolve...

[foolip]

Thanks @tabatkins for w3c/csswg-drafts@0385164. I'll update this PR to drop the extra arguments. (Updated algorithms are at https://drafts.csswg.org/css-position-4/#top-manip.)

[foolip]

Changes to adapt to w3c/csswg-drafts@0385164 and stick with <var> as the rest of the spec does.

[foolip]

This is all good now IMHO, much nicer with the dropped document argument to many algorithms.

@annevk can you review this from the WebKit perspective?

[foolip]

This is all good now IMHO, much nicer with the dropped document argument to many algorithms.

@annevk can you review this from the WebKit perspective?

[annevk]

I think all my concerns are editorial, but they would still be good to fix.

[annevk]

Only the second step is needed here looking at the algorithm for "add".

[foolip]

Without the call to "request removal from the top layer", the assert would be hit, right?

[annevk]

I see. So we preserve the behavior but allow for slightly novel behavior in new APIs. I guess that's okay. If we don't end up using the new behavior though we should simplify this again.

Perhaps it's best to wait for @nt1m and merge this tomorrow. I can ping him.

[tabatkins]

I'm not sure what you mean by "allow for slightly novel behavior in new APIs". All APIs would cycle thru the algorithms in this same way, unless they wanted to get into the guts and directly manipulate the top layer list (which they shouldn't).

The only two behaviors that specs should end up landing on for "put something in the top layer when it's already there" are this (cycle it to the end of the list) and "fail", probably throwing an error when they do so.

[annevk]

It does kinda seem like unless they would always remove prior to adding they'd have to perform a contains check. Which would be the somewhat novel behavior, but not entirely novel as the old API technically allowed for that as well.

[foolip]

Right, specs have to either check if the element is already in top layer and treat it as an error, or call "request removal from the top layer" to get the moving behavior. The latter preserves the behavior mentioned in the last note in https://fullscreen.spec.whatwg.org/commit-snapshots/afd56a35f409e5595dd861f41390a1016ebd6aa2/#new-stacking-layer.

[nt1m]

Looks fine aside from my one comment.

[foolip]

I've filed https://bugzilla.mozilla.org/show_bug.cgi?id=1834542 and https://bugs.webkit.org/show_bug.cgi?id=257199.

@annevk do you want to approve this?

[annevk]

🎉 Thanks all!