Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-introduce the hierarchy restrictions #140

Open
upsuper opened this issue Sep 20, 2018 · 5 comments
Open

Re-introduce the hierarchy restrictions #140

upsuper opened this issue Sep 20, 2018 · 5 comments

Comments

@upsuper
Copy link
Member

upsuper commented Sep 20, 2018

The hierarchy restrictions were removed in 766dc87 and 9592913 because it seemed to be unnecessary after we moved to the top layer model.

However, it forms an unfortunate combination with the change to make fullscreenchange event dispatched to element, that if we have multiple leaf fullscreen element in the stack, some of the element wouldn't get the fullscreenchange event. I raised in #89 (comment), and @foolip said he's fine adding back these restrictions in #89 (comment) and opened #91 to do so.

Recently, #91 was abandoned probably because of lack of activity and people don't recall the reasoning to do so. So I'm opening this issue to track that.

Blink currently doesn't have the hierarchy restrictions, while Gecko still has. And if we decide to re-introduce it, Blink may want to change before it's too late.

cc @dtapuska

@upsuper
Copy link
Member Author

upsuper commented Sep 20, 2018

These manual tests are checking against hierarchy restrictions:

  • fullscreen/api/element-ready-check-fullscreen-element-sibling-manual.html
  • fullscreen/api/element-request-fullscreen-non-top-manual.html
  • fullscreen/api/element-request-fullscreen-two-elements-manual.html

@dtapuska
Copy link
Contributor

Adding back the hierachy restrictions for this event targetting is unforunate. We are forcing users into a model of how to design their application that the content must be present in the child of the current fullscreen element.Think of a series of divs that are all siblings and you click 'next'. Which would change the fullscreen element to the next element. In order to replicate that you'd need to do a bunch of tree modifications. Also it prevents you from re-entering a fullscreen element already on the stack.

@domenic Adding Domenic in case he has any ideas here.

@upsuper
Copy link
Member Author

upsuper commented Sep 21, 2018

(For my own note: there was a Gecko bug we can reuse if we end up not reverting the change.)

@upsuper
Copy link
Member Author

upsuper commented Oct 19, 2018

Think of a series of divs that are all siblings and you click 'next'. Which would change the fullscreen element to the next element.

That is not a usecase that the current Fullscreen API is supposed to support. You would end up having lots of elements in the fullscreen layer, and you cannot exit fullscreen in one statement (unless we add #70).

If we want to serve this usecase, we may want an API like requestFullscreen({ replace: true }) or so, rather than supporting multi-leaf.

@upsuper
Copy link
Member Author

upsuper commented Jan 4, 2019

Also, if we are re-introducing the hierarchy restriction, we may need to revisit bug 27865 and see how modal dialog would need to interact with fullscreen...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants