@upsuper @foolip This is an alternate approach to (#126) to address issues in #102 that I think could be viable. Thoughts?

There are still some funny edge cases with this approach especially regarding nested fullscreen.

For example, if the removed fullscreen element is an <iframe> element, its content would keep being in fullscreen state (it's not clear whether other approach can solve this natively but it might be something we can keep in mind?).

Also, if the document is a sub-document, and the <iframe> of its browsing context doesn't have iframe fullscreen flag set, the <iframe> should also exit fullscreen when the document gets unfullscreened.

@upsuper Yes that is a problem how FullyExitFullscreen is defined. It doesn't recurse into iframes. If we want to keep this approach I suggest we take steps 3, 4, 5 from #126 but then it pretty much is #126

For example, if the removed fullscreen element is an <iframe> element, its content would keep being in fullscreen state (it's not clear whether other approach can solve this natively but it might be something we can keep in mind?).

Removing an iframe triggers https://html.spec.whatwg.org/multipage/window-object.html#a-browsing-context-is-discarded. If you have a handle to the document within you can still look at iframeDoc.fullscreenElement and see that it doesn't change. A test for this in the style of web-platform-tests/wpt#6302 should be possible. This is actually a funny thing that I don't think any solution to the "fullscreen element is removed" problem can get around. I think it already behaves as I described in Chromium.

Also, if the document is a sub-document, and the <iframe> of its browsing context doesn't have iframe fullscreen flag set, the <iframe> should also exit fullscreen when the document gets unfullscreened.

Yes. I think removing the "Unfullscreen doc" step and just continuing to run is the fix for this. Because document.fullscreenElement already changed, this can lead to exiting a step further than would have happened when calling document.exitFullscreen() (and not removing the fullscreen element) but this is similar to calling document.exitFullscreen() twice. The only way to avoid this is one of:

1. Keeping track in the async part of "exit fullscreen" that the fullscreen element was removed and acting as if it were still in top layer at certain steps, to avoid exiting too much.
2. Setting resize to true so that removing the fullscreen element always consistently exits fully.

I'd be OK with either just leaving it a bit odd or setting resize to true. Keeping track of more state doesn't sound great.

Ok I've removed the Unfullscreen doc step and added back the remove top layer code from #102 to make this complete.

I don't think this is going to work... When you have two fullscreen elements in the top layer, and you remove one of them, you would end up making the browser window still in fullscreen, but the document has exited the fullscreen state.

So I still think it is easier to just fully exit fullscreen in this case. That would natively solve all the edge cases and leave a sane state just like when the user presses escape.

Note that, setting resize wouldn't work either. That way, you may end up having the document still in fullscreen state, but the window has been restored to normal state.

When you have two fullscreen elements in the top layer, and you remove one of them, you would end up making the browser window still in fullscreen, but the document has exited the fullscreen state.

This is a variation on the problem of calling document.exitFullscreen() twice, where neither call thinks it will end up fully exiting in their synchronous steps, but when the async steps of both have run the top layer is empty. The fix for this has to be in the async part of "exit fullscreen". Seems like there's no bug tracking this, and it's just a TODO in the Chromium code. (It just stays in fullscreen in a weird state.)

So I still think it is easier to just fully exit fullscreen in this case.

For some redefinition of "fully exit fullscreen" I think that would be fine, although if we fix the above problem that seems slightly nicer to me.

The main problem with #126 is poking at the state of other frames synchronously, and it can't be implemented exactly like that in Chromium. Doing #65 would resolve the problem, making "fully exit fullscreen" more async. As part of that we'd have to make sure that setting resize to true fully and correctly cleans up all state, even if we're in some odd state.

I would definitely be nice if we can fix the issue of double-calling to exitFullscreen, but leaving in such state isn't really the first thing I noticed.

With the double-calling exitFullscreen issue gets fixed, this approach is still a bit odd that: if you have two fullscreen elements and you remove one, you end up fully exit fullscreen, but if you have three, you end up being left in fullscreen. People (maybe library authors?) may start depending on removing element to fully unfullscreen (since in majority of cases it does), but then it breaks in some rare cases when there are more than three fullscreen elements.

The main problem with #126 is poking at the state of other frames synchronously, and it can't be implemented exactly like that in Chromium. Doing #65 would resolve the problem, making "fully exit fullscreen" more async.

It's not completely clear to me how #65 would make things better... Do you mean moving more stuff into the async part would make it easier to implement in Chrome? I guess I'm fine with something like, invoke fully exit fullscreen asynchronously when the element gets removed... Would that resolve your problem?

It has been a while and I think last time @foolip indicated we probably needed a way to force a resize in a fullscreen exit. @upsuper does something like this make sense?

@foolip ping

It's not completely clear to me how #65 would make things better... Do you mean moving more stuff into the async part would make it easier to implement in Chrome? I guess I'm fine with something like, invoke fully exit fullscreen asynchronously when the element gets removed... Would that resolve your problem?

@upsuper, yes, if we use "fully exit fullscreen" to handle removing the current fullscreen element, and if we want to guarantee that it really does leave the document in question with an empty top layer stack, or alternatively that we resize and empty the top layers of all documents, then "fully exist fullscreen" needs to have some flag to really guarantee that, much like what @dtapuska has pushed now, in the same vein as #65.

However, reading this thread again, and looking at the "double-calling exitFullscreen issue", I'd like to ask if we can separate these issues. We have a bunch of very intricate interactions between entering and exiting / full exiting fullscreen that don't quite make sense, and I think that making only "fully exit fullscreen" have a stronger guarantee isn't that helpful.

I would suggest, rather, that if we are really troubled by these interactions, that we make it explicit that only one request to enter or exit fullscreen can be in-flight at once, then we can stop worrying about these cases or multiple requests racing. Effectively, then, the fullscreen state is a global thing and the first request always wins. (Details to be worked out.)

@upsuper, do you think leaving this wart for now would be acceptable? It would amount to going back to the state of this PR at f59df4d.

The problem of f59df4d isn't really identical to double-calling exitFullscreen. It's something happening internally which has the same effect as double-calling at this moment. That means, adding some mechanism to make only one in-flight enter / exit change isn't going to help for that case.

But I think the discussion probably does go too far and I think it's fine that we resolve on doing f59df4d for now and try to figure out how to resolve its consequences.

I mean, try to figure out how to resolve its consequences later in separate issues.

@upsuper, yeah, you're right, while double document.exitFullscreen() is currently another way that one can end up in the asynchronous part of "exit fullscreen" with nothing in the top layer stack, fixing that would still leave this situation:

We're in nested fullscreen, two elements in the top layer stack. Removing the fullscreen element will not cause us to exit fullscreen, but still we'll end up in the asynchronous part of "exit fullscreen" removing the second element from the top layer.

@dtapuska, new idea for a fix. In the "If doc’s fullscreen element is not connected" condition, set a fullscreenElementWasConnected variable to false. In the async part (whether resize happened or not) just do nothing if that variable is false.

@dtapuska or, file an issue and let's merge this as is. Tests that could be adapted for this PR are in web-platform-tests/wpt#6302.

@foolip wpt tests already have landed for this. I've merged the pull requests into a single change and opened an issue for the two fullscreen elements in the top layer scenario. Please merge.

Thanks @dtapuska!