Reinstate the hierarchy restrictions#91
Conversation
|
@upsuper, in addition to these changes, something needs to be done to uphold the invariant when mixed with
What does Gecko do? |
|
Sadly, Gecko still doesn't have I don't recall exactly what the problems are for that... It seems one of the problem would be that 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 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... |
| <li><p><var>element</var> is <a>connected</a>. | ||
|
|
||
| <li><p><var>element</var>'s <a>node document</a>'s <a>fullscreen element</a> is null, or is an | ||
| <a>inclusive ancestor</a> of <var>element</var>. |
There was a problem hiding this comment.
You also need to invoke this check recursively for its browsing context container if any.
There was a problem hiding this comment.
Oh, right, the ready check used to be recursive but I removed that in #52.
Yep, we could add another "InvalidStateError" case to https://html.spec.whatwg.org/#dom-dialog-showmodal
Actually, if we add a check for |
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? |
|
It could be, I'll add use counters in Blink both for when the element is a |
|
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 |
|
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. |
|
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. |
To answer questions in whatwg/fullscreen#91. BUG=402376 Change-Id: Ibb7d2dcc271bdf18b094b5d100db52e60be9ca9a Reviewed-on: https://chromium-review.googlesource.com/512163 Commit-Queue: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Mike West <mkwst@chromium.org> Cr-Commit-Position: refs/heads/master@{#474270}
This way, interactions between the algorithms for fullscreen and dialog are simplified. Example concern: #102 (comment) This also makes it easier to reinstate hierarchy restrictions: #91 Tests: https://chromium-review.googlesource.com/c/chromium/src/+/684435
This way, interactions between the algorithms for fullscreen and dialog are simplified. Example concern: #102 (comment) This also makes it easier to reinstate hierarchy restrictions: #91 Tests: https://chromium-review.googlesource.com/c/chromium/src/+/684435
This way, interactions between the algorithms for fullscreen and dialog are simplified. Example concern: #102 (comment) This also makes it easier to reinstate hierarchy restrictions: #91 Tests: https://chromium-review.googlesource.com/c/chromium/src/+/684435
|
Abandoning this. |
|
Hmmm... why do we abounding this? I think this was raised due to concern on Web platform test |
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