This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 26366 - It's possible to go fullscreen with an element not in the document
Summary: It's possible to go fullscreen with an element not in the document
Status: RESOLVED FIXED
Alias: None
Product: WebAppsWG
Classification: Unclassified
Component: Fullscreen (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Target Milestone: ---
Assignee: Anne
QA Contact: public-webapps-bugzilla
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-07-17 08:57 UTC by Philip Jägenstedt
Modified: 2014-07-31 20:12 UTC (History)
5 users (show)

See Also:


Attachments

Description Philip Jägenstedt 2014-07-17 08:57:09 UTC
This is one example of a class of problems caused by the checks made synchronously in requestFullscreen() not necessarily holding when the rest of the function runs async.

To go fullscreen with an element not in the document:
element.requestFullscreen();
element.parentNode.removeChild(element);

To end up with a fullscreen element stack with siblings are put on the stack:
element.requestFullscreen();
element.nextSibling.requestFullscreen();

I'm not sure what bad things can result, but it seems silly to make checks that aren't actually guaranteed to hold.
Comment 1 Philip Jägenstedt 2014-07-28 09:22:10 UTC
I did some ad-hoc testing on calling requestFullscreen() and exitFullscreen() multiple times in the same script and found that there wasn't great interoperability even when one doesn't try to circumvent the checks.

The basic problem is that the script-visible fullscreen element stack shouldn't be updated until one has actually entered/exited fullscreen, and there's no script-invisible state available to do these checks.

I can see a few options:

1. Have a script-invisible stack of pending requests which is pushed to the script-visible stack before the fullscreenchange event is fired.

2. Have a single pending element which is replaced by successive calls to requestFullscreen().

3. Make requestFullscreen() and exitFullscreen() do nothing if there's already a pending change.
Comment 2 Anne 2014-07-28 09:47:17 UTC
3 seems to be how we generally do this. Introduce a document-wide "fullscreen flag" and clear it once either algorithm terminates? And just make the methods no-ops if the flag is set?
Comment 3 Anne 2014-07-28 09:53:00 UTC
Although I guess we don't really want to block exitFullscreen(). Especially for things such as navigation.

A document-wide pending element could work. exitFullscreen() would just clear the element and return. And the queued task from requestFullscreen would become a no-op. Does that make sense?
Comment 4 Robert O'Callahan (Mozilla) 2014-07-28 09:57:24 UTC
I think so.
Comment 5 Philip Jägenstedt 2014-07-28 11:04:45 UTC
(In reply to Anne from comment #3)
> Although I guess we don't really want to block exitFullscreen(). Especially
> for things such as navigation.
> 
> A document-wide pending element could work. exitFullscreen() would just
> clear the element and return. And the queued task from requestFullscreen
> would become a no-op. Does that make sense?

This depends on what the internal state is. If the window resize has already been requested, simply clearing the pending element would lead to something strange when the resize is completed. One could immediately exit fullscreen in that case, but should that cause an additional fullscreenchange event to be fired?
Comment 6 Anne 2014-07-28 13:08:17 UTC
requestFullscreen()
- sets pending element
- queues a task to run these steps
  1) if pending element is unset, return
  2) push pending element to stack
  3) unset pending element
  4) animate (async?)

exitFullscreen()
- queues a task to run these steps
  1) if pending element is set, unset it and return
  2) pop stack
  3) animate (async?)

Since they're separate tasks, there should be no problem, right? They can happen quite shortly after each other. Making that animate smoothly would be up to the implementation.
Comment 7 Philip Jägenstedt 2014-07-28 13:31:06 UTC
That doesn't seem quite right... we can't manipulate the fullscreen element stack from a task posted by requestFullscreen() or exitFullscreen() if it's to be synchronized with animation frames as per bug 26440.
Comment 8 Anne 2014-07-28 15:06:29 UTC
More problems: the checks requestFullscreen() does are crossing origin boundaries and therefore need to be asynchronous somehow. As the event is already dispatched from a task, that may be feasible.
Comment 9 Philip Jägenstedt 2014-07-28 15:10:05 UTC
Maybe the checks can just be done async before the resize, that way they can take however long they want. What about race conditions, what happens when two frames request fullscreen at the same time and passes the checks, which would be violated if the timing were different?
Comment 10 Anne 2014-07-28 15:26:08 UTC
Yes, we need to account for the resizing process to fail.
Comment 11 Anne 2014-07-30 12:59:54 UTC
This should bring us a lot closer. Please review!

https://github.com/whatwg/fullscreen/commit/5187282e5fd24a1c4ff0164d444e1bfc2bdf44ef
Comment 12 Anne 2014-07-31 20:12:17 UTC
New bugs for new issues I guess. Thanks all!