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

Introduce document.exitFullscreen({ fully: true })? #70

Open
foolip opened this issue Dec 19, 2016 · 18 comments
Open

Introduce document.exitFullscreen({ fully: true })? #70

foolip opened this issue Dec 19, 2016 · 18 comments

Comments

@foolip
Copy link
Member

@foolip foolip commented Dec 19, 2016

Split from #65

This would expose to the web a primitive that's used internally when navigating and when the user presses Esc (or similar), and would make it easier to test. The IDL would look like:

partial interface Document {
  Promise<void> exitFullscreen(optional ExitFullscreenOptions options);
};

dictionary ExitFullscreenOptions {
  boolean fully = false;
};
foolip added a commit that referenced this issue Jan 4, 2017
The previous definition of fully exit fullscreen could unfullscreen
elements in top layer synchronously. If those elements were iframes,
the contentDocument would get handled in a following animation frame
task, leaving everything in an odd state in the interim.

Exposing the concept of fully exiting to scripts is a separate issue:
#70
foolip added a commit that referenced this issue May 10, 2017
The previous definition of fully exit fullscreen could unfullscreen
elements in top layer synchronously. If those elements were iframes,
the contentDocument would get handled in a following animation frame
task, leaving everything in an odd state in the interim.

Exposing the concept of fully exiting to scripts is a separate issue:
#70
foolip added a commit that referenced this issue May 11, 2017
The previous definition of fully exit fullscreen could unfullscreen
elements in top layer synchronously. If those elements were iframes,
the contentDocument would get handled in a following animation frame
task, leaving everything in an odd state in the interim.

Exposing the concept of fully exiting to scripts is a separate issue:
#70
foolip added a commit that referenced this issue May 12, 2017
The previous definition of fully exit fullscreen could unfullscreen
elements in top layer synchronously. If those elements were iframes,
the contentDocument would get handled in a following animation frame
task, leaving everything in an odd state in the interim.

Exposing the concept of fully exiting to scripts is a separate issue:
#70
foolip added a commit that referenced this issue May 29, 2017
The previous definition of fully exit fullscreen could unfullscreen
elements in top layer synchronously. If those elements were iframes,
the contentDocument would get handled in a following animation frame
task, leaving everything in an odd state in the interim.

Exposing the concept of fully exiting to scripts is a separate issue:
#70
@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Feb 26, 2019

Apologies, I don't know if you are taking comments from third-parties, but +1 for:
element.requestFullscreen({ replace: true })
document.exitFullscreen({ fully: true })
or similar functionality.
And maybe a readable array of LIFO fullscreen elements?
The current state of browser handling of nested fullscreen elements is a disaster. For example, a single-page website that might have the HTML element fullscreen and then also allow a video element within it to go fullscreen. Without adding some extra tools to the Fullscreen API the current situation is intolerable!

@foolip

This comment has been minimized.

Copy link
Member Author

@foolip foolip commented Feb 26, 2019

@richardbushnell, all feedback welcome, just note that if you propose a solution that makes it into the spec, you may be asked to sign https://participate.whatwg.org/agreement.

Can you describe what you're trying to make work with nested fullscreen and can't currently? Is the problem that you can't exit all the way with a click of a button in the UI?

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Feb 27, 2019

@foolip

No problem signing the agreement if any ideas or solutions are implemented into the spec. The fundamantal problem is that, to my understanding, the Browser UA has information about the fullscreen stack that is unavailable to the developer. In particular, the developer has no way that I am aware of to query if there are more than one elements in fullscreen concurrently, which is essential in making certain decisions. Currently, you cannot read/get in advance whether there are multiple elements nested/stacked. If there was a way to read/get that information, then desired actions based on that state can be actioned to a plan. For instance, why not allow a getter for document.fullscreenElements (plural) to return a read-only array of elements, and by extension document.fullscreenElements.length? Optionally, even a readable property like .isFullscreen on each element too? Optionally, even allow element.exitFullscreen (at Element level, wherever it is in the stack) to counter the current element.requestFullscreen method. I think that many fundamental issues to the developer could be circumvented by exposing the multiple elements (or at least knowledge/number of them). That would be my desired solution, BUT failing that exposure, then "element.requestFullscreen({ replace: true })" or "document.exitFullscreen({ fully: true })" provide a much easier method to take control of the current situation of the fullscreen elements stack. Obviously, in terms of spec only, I am trying to avoid detailing the current mess of implementation across browsers.

@foolip

This comment has been minimized.

Copy link
Member Author

@foolip foolip commented Feb 27, 2019

@richardbushell exposing more of that information would be fairly straightforward, but I'd also like to understand how it would be used. If you know that you're in nested fullscreen and what the other elements in addition to document.fullscreenElement are, what would your code do differently?

Or is the main ask to avoid entering nested fullscreen to begin with?

Are you writing library code that doesn't know if it's the only thing in the page using the fullscreen API?

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Feb 28, 2019

@foolip our use case is as of web developers, but in working with current implementation briefs for websites that perform as single-page web apps (often designed to be viewed in fullscreen mode throughout) we've had a lot of issues. Many of the other libraries and components that are included have no knowledge of each other and many complications have arisen.

Sometimes the other libraries haven't implemented the spec with consideration that other elements could also be fullscreen, and in those cases Issues and Pull Requests have and can be raised.

In other situations, Browsers haven't considered the spec implications. Currently, Edge doesn't trigger fullscreenchange on the ELEMENT first (just on document directly), and Safari fails to enter a nested element into fullscreen mode at all. Again, browser bugs have been filed.

Even mainstream reference material does not take nested fullscreen properly into account. For instance:-
https://developer.mozilla.org/en-US/docs/Web/API/Fullscreen_API#Toggling_full-screen_mode
https://github.com/rafrex/fscreen#usage

As developers, we just need the knowledge of the fullscreen stack to make appropriate decisions when third-party libraries have no knowledge of each other.

As a minimum, I would simply propose a getter for document.fullscreenElements (plural) to return a read-only array of elements, and by extension document.fullscreenElements.length
Ideally, a "fully" method to simulate the ESC-key currently, either exitFullscreenFully() or exitFullscreen({ fully: true })

The other suggestions in my previous post would be useful, such as element.exitFullscreen, but I would strongly desire the minimum of exposure to read the fullscreen elements in advance without popping each in turn to try and make appropriate decisions.

@foolip

This comment has been minimized.

Copy link
Member Author

@foolip foolip commented Feb 28, 2019

@upsuper @jernoble what are your thoughts on exposing the top layer elements or possibly only the fullscreen elements within, as well as document.exitFullscreen({ fully: true })?

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 3, 2019

First of all, there is a way to get all fullscreen elements, and there is a way to check whether a specific element is in fullscreen state. That way is called CSS selector :)

All fullscreen elements match :fullscreen pseudo-class, thus you can always use document.querySelectorAll(':fullscreen') and element.matches(':fullscreen') for the aforementioned information correspondingly.

Having said that, querySelectorAll is essentially a full document / subtree scan, so it could be relatively slow, but if its usage is common enough, we can certainly optimize it from the UA side (based on top layer stack that UAs are aware of) without adding other API, I would argue. So I'm moderately against adding new API for fetching that information.

In other situations, Browsers haven't considered the spec implications. Currently, Edge doesn't trigger fullscreenchange on the ELEMENT first (just on document directly), and Safari fails to enter a nested element into fullscreen mode at all. Again, browser bugs have been filed.

This is not going to be resolved by adding new APIs in the spec. When a browser doesn't have developer to maintain a feature actively, it's not very likely that they would follow changes to such spec closely.

Even mainstream reference material does not take nested fullscreen properly into account. For instance:

Maybe issues should be filed against those materials to cover nested fullscreen.

@upsuper @jernoble what are your thoughts on exposing the top layer elements or possibly only the fullscreen elements within, as well as document.exitFullscreen({ fully: true })?

As I mentioned, we already have API to query fullscreen elements, so I don't think we should add API for that.

And I think I'm kinda relatively strongly against exposing top layer elements, because top layer itself is not a web-exposed primitive, and it may be used for different stuff in the future, so if we expose it prematurely, we may footgun ourselves for that websites rely on it only returning fullscreen and modal dialog.

It doesn't seem to me that there is request to add API to query modal dialog, but if there is, I think we should as well just add a pseudo-class for that, so that authors can even style it accordingly, although I'm not sure whether that's actually useful at all.

As for document.exitFullscreen({ fully: true }), I'm not fully sure how I feel about it. I guess it's fine...

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 3, 2019

querySelectorAll ... usage is common enough, we can certainly optimize it from the UA side (based on top layer stack that UAs are aware of)

Think about it twice, I think it would be easier to optimize if fullscreen elements have hierarchy restriction, since querySelectorAll needs to return element in tree order. Maybe we can revisit #140?

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 3, 2019

@upsuper - Thanks for your thoughts. I genuinely hadn't considered good old CSS as a way of accessing the fullscreen stack elements via JavaScript. Yes, knowledge of the ORDER of the elements in the fullscreen stack would be important for developers I think.

And if there's no objections then I think document.exitFullscreen({ fully: true }) would be very useful for developers, simply mimicking the ESC key, and the algorithm is already set up for that anyway.

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 3, 2019

@upsuper - I have tested your proposed CSS selector workaround, but unfortunately the :fullscreen pseudo-class seems to only ever match ONE element (the current fullscreenElement at the top of the fullscreen stack). Here's my test:-

https://living.video/fullscreenchange.html

I used the same test to double-check the fullscreenchange listeners at both Element and Document level too. If you view the page source then it's pretty self-explanatory. Just view the Console to see the Event activity.

Current browser adherence to the spec is mixed. Blink & Gecko seem perfect. Edge does not fire fullscreenchange at the Nested Video Player element when it EXITS fullscreen from the nested element. Safari Webkit has a major bug which I've already filed, i.e. that as soon as you enter fullscreen on a nested element it returns NULL as the fullscreenElement (breaking everything).

But, in the base context of the further discussion arising in this thread, I believe there is still no way to know if there are multiple/nested elements in the fullscreen stack. You could only discover them by popping each in turn. So, I believe my earlier comment about not having any exposure to the fullscreen stack, or even it's length, is still correct (unless I've mis-implemented the method, or unless there's another way?)

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 3, 2019

The output on Firefox is:

0 The VENDOR-specific :fullscreen Pseudo Class is :fullscreen fullscreenchange.html:35:3
----------------------------------- fullscreenchange.html:37:3
5.236 DOCUMENT fullscreenchange: There is at least one element in fullscreen mode. The current fullscreenElement is: [object HTMLHtmlElement] fullscreenchange.html:41:5
5.237 DOC (continued): ... and, here are ALL the current fullscreen elements:- fullscreenchange.html:46:4
5.237 [object HTMLHtmlElement] fullscreenchange.html:48:5
5.238 DOC (continued): ... and, is the HTML element fullscreen?: true fullscreenchange.html:50:4
5.238 DOC (continued): ... and, is the PLAYER element fullscreen?: false fullscreenchange.html:51:4
################################### fullscreenchange.html:52:4
22.941 ELEMENT (Video PLAYER DIV) fullscreenchange: The current fullscreenElement is: [object HTMLDivElement] fullscreenchange.html:72:4
22.941 EL (continued): ... and, here are ALL the current fullscreen elements:- fullscreenchange.html:73:4
22.942 [object HTMLHtmlElement] fullscreenchange.html:75:5
22.942 [object HTMLDivElement] fullscreenchange.html:75:5
22.942 EL (continued): ... and, is the HTML element fullscreen?: true fullscreenchange.html:77:4
22.943 EL (continued): ... and, is the PLAYER element fullscreen?:  true fullscreenchange.html:78:4
########## fullscreenchange.html:79:4
22.944 DOCUMENT fullscreenchange: There is at least one element in fullscreen mode. The current fullscreenElement is: [object HTMLDivElement] fullscreenchange.html:41:5
22.944 DOC (continued): ... and, here are ALL the current fullscreen elements:- fullscreenchange.html:46:4
22.945 [object HTMLHtmlElement] fullscreenchange.html:48:5
22.945 [object HTMLDivElement] fullscreenchange.html:48:5
22.946 DOC (continued): ... and, is the HTML element fullscreen?: true fullscreenchange.html:50:4
22.946 DOC (continued): ... and, is the PLAYER element fullscreen?: true fullscreenchange.html:51:4
###################################

looks like it works as expected.

I see that Chrome doesn't behave this way, but this is a browser bug. The spec says:

The :fullscreen pseudo-class must match any element element for which one of the following conditions is true:

  • element’s fullscreen flag is set.
  • element is a shadow host and the result of retargeting its node document’s fullscreen element against element is element.

You should file bugs to browsers which are not doing the right thing.

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 3, 2019

@upsuper - You are right! Apologies, I didn't re-run my own initial test on Firefox after I made a final tweak earlier to the code to handle all prefixed and non-prefixed selectors.

Yes, Firefox is indeed the only browser that implements the current spec correctly, and therefore is currently the only browser that allows a developer to know if there are multiple (nested) fullscreen elements, by querying all elements matching the CSS :fullscreen selector.

@foolip - have you got an inner channel within Google to chase this through for Chrome? Blink seems compliant in every other respect that I can see.

@jernoble - have you got an inner channel to chase the Fullscreen API issues with Webkit through? The CSS selector issue isn't disastrous, BUT the issue where WebKit sets the fullscreenElement to null as soon as trying to enter a nested/secondary fullscreen capable element is a major problem, and has just been exposed to a much larger audience for Apple since implementing the Fullscreen API on iPad in the last couple of weeks. This would probably break all affected websites.

Assuming CSS selectors (when fixed/implemented) then offer a mechanism to have knowledge of multiple fullscreen elements, could their ORDER be determined?

Finally, please let me know your final decision on document.exitFullscreen({ fully: true }) once you've all had a chance to discuss, this would be a really useful option for developers.

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 3, 2019

Assuming CSS selectors (when fixed/implemented) then offer a mechanism to have knowledge of multiple fullscreen elements, could their ORDER be determined?

querySelectorAll is guaranteed to return elements in the tree order, and in Firefox currently, you can only nestedly fullscreen an element only if it's a descendant of the current fullscreen element (which is the hierarchy restriction I mentioned before), so the order is guaranteed to be from the bottommost to the topmost fullscreen element.

If we remove the hierarchy restriction, the tree order may not be that useful to determine the order in top layer. But for other reasons, I would argue that we should enforce the hierarchy restriction. But that's a separate topic.

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 3, 2019

@upsuper Thanks very much, crystal clear.

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 7, 2019

I have filed bugs with Chromium and Webkit for the Fullscreen API spec non-compliance issues mentioned above.

I have one more question too in regard to document.exitFullscreen({ fully: true }). Is there also anything in the spec that covers the Browser's own fullscreen mode. e.g. On a PC it seems to have been standardised as the F11 key. This enters fullscreen mode but does not seem to fire any events or set any fullscreen flags. The document does not know about this and in a typical site a button/message to Enter Fullscreen will still remain. In this mode, only F11 will exit fullscreen too, the ESC key does nothing.

Is this covered by the spec?

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 7, 2019

No. Fullscreen mode is not a web-exposed feature, and website is not supposed to be aware of that. Website should just get a resize event for the viewport size change.

The spec should probably make it clear that fullscreen mode is out of scope.

@richardbushell

This comment has been minimized.

Copy link

@richardbushell richardbushell commented Mar 7, 2019

OK, it's a bit weird that they'd then fire fullscreenchange events (rather than just resize) when they exit. Seems an anomaly. Two-worlds colliding I guess?

Anyway, I'll await feedback about the option for a "fully"-based exit, as per the initial title of this issue.

@upsuper

This comment has been minimized.

Copy link
Member

@upsuper upsuper commented Mar 7, 2019

It sounds like a browser bug that it fires fullscreenchange for fullscreen mode.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.