Reinstate the hierarchy restrictions

[foolip]

This reinstates parts of the fullscreen element ready check removed in
commit 766dc87 and
commit 9592913, with some
simplification of the iframe ancestor check.

---

Preview | Diff

[foolip]

@upsuper, in addition to these changes, something needs to be done to uphold the invariant when mixed with dialog. I see two options:

• Add checks in HTML to avoid moving elements within top layer if it would end up violating the hierarchy restrictions.
• Add something in https://fullscreen.spec.whatwg.org/#top-layer-add to remove elements to maintain the invariants.

What does Gecko do?

[upsuper]

Sadly, Gecko still doesn't have dialog... so this is not applicable.

I don't recall exactly what the problems are for that...

It seems one of the problem would be that showModal may be called on a dialog element which is already a fullscreen element?

For that, I would probably prefer the second way, specifically, we pop out fullscreen elements until the dialog is no longer a fullscreen element, and then we add the element back as a dialog.

But thinking about this a bit further, this option is probably unnecessarily complicated, because that means we would need to consider dispatching event, and even unfullscreen the window, for a showModal call, which sounds unfortunate.

Given this, I guess we should make HTML avoid invoking top-layer-add when the element is already a fullscreen element by rejecting it with a exception, and I think we should assert in top-layer-add that the element to be append is not currently an element in the top layer.

If we add that assertion, we would also need to think about what if a modal dialog requests fullscreen. Maybe we should reject as well. I don't think those cases are common, and I don't have idea how authors may find those cases useful...

[upsuper]

You also need to invoke this check recursively for its browsing context container if any.

[foolip]

Oh, right, the ready check used to be recursive but I removed that in #52.

[foolip]

I guess we should make HTML avoid invoking top-layer-add when the element is already a fullscreen element by rejecting it with a exception

Yep, we could add another "InvalidStateError" case to https://html.spec.whatwg.org/#dom-dialog-showmodal

If we add that assertion, we would also need to think about what if a modal dialog requests fullscreen. Maybe we should reject as well.

Actually, if we add a check for dialog in requestFullscreen(), then no check should be needed in showModal(). With the current state of things, no dialog elements in top layer would be fullscreen, and all non-dialog elements would be.

[upsuper]

Actually, if we add a check for dialog in requestFullscreen(), then no check should be needed in showModal(). With the current state of things, no dialog elements in top layer would be fullscreen, and all non-dialog elements would be.

I mean we check whether the element is a modal dialog, not that whether it is a dialog element. But well, I guess we can just simple reject any dialog element from entering fullscreen if that sounds simpler... but could that be breaking?

[foolip]

It could be, I'll add use counters in Blink both for when the element is a dialog element and when it is additionally in the top layer, which would mean that it's showing as modal. I think it's fairly unlikely that this would break real content, but we could leave it as an open issue until the results are in, or make a tentative change and revert if necessary.

[foolip]

https://chromium-review.googlesource.com/c/512163/

[foolip]

OK, I've pushed something that should work. I worry a bit about making the ready check recursive again, because it's a cross-process access and can't actually be checked synchronously. In other words, hierarchy restrictions is a simplifications in some ways, but not in others. However, I lean towards having them for the time being.

@upsuper, are you OK with the dialog bit, or should I add a TODO there and file a separate issue, blocked on usage data? Yet another possibility would be to change https://fullscreen.spec.whatwg.org/#top-layer-add to do nothing if the element already exists, so that things can never move in the top layer. Then showModal() would need to handle that situation instead.

[upsuper]

I think this is fine. I'd still want to also change the "top-layer-add" algorithm to not move things around, but instead assert that the element to add is not in the top layer currently. It seems to me the HTML spec doesn't allow a open dialog to be opened again, so it wouldn't break that assertion either.

[foolip]

Yeah, I think you're right that such an assert would hold without any other changes. I've pushed the assert you suggested, and think that whatwg/html#2341 might need a tweak for that.

[foolip]

Abandoning this.

[upsuper]

Hmmm... why do we abounding this?

I think this was raised due to concern on fullscreenchange event for exiting fullscreen wouldn't go through every fullscreen element in the stack when there are multiple leaf fullscreen elements, which I raised in #89 (comment) and you agreed that it makes sense.

Web platform test fullscreen/api/element-ready-check-fullscreen-element-sibling-manual.html checks this behavior and it's currently failing on Gecko as we still enforce the hierarchy restrictions, and given that concern I think that still makes sense. So we should revive this PR and change that test.

[foolip]

Sorry, I moved a bit fast when abandoning old PRs after #128 obsoleted a lot of them. I'm a bit swamped until mid-next-week, but will come around to #140.