This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.
I'd much prefer the Promise or callback-based workflow for handling full screen rejections or approvals: document.body.requestFullScreen() .then(function () { /* ... */ }) .catch(function (e) { /* ... */ }); vs var errHandler = function () { document.removeElementListener('fullscreenerror', errHandler); /* ... */ }; document.addEventListener('fullscreenerror', errHandler); document.body.requestFullscreen(); Using promises or callbacks also means better encapsulation for libraries. Imagine I'm using a video library such as jwPlayer means any fullscreenchange or fullscreenerror handlers I've added will be triggered by that library's request for full screen unless I use the "once" pattern as above. Generally, those events will always be assigned at the same time as the request, so returning a promise to be fulfilled or rejected just makes sense. The fullscreenchange is still relevant, but it seems strange to me that the fullscreenerror event even exists, because I can't think of a situation where you'd want to catch *all* full screen rejections and not just one pertaining to each request made.
This is a pre-promise API. The only question here I guess is whether not granting a page's request to go fullscreen is exceptional...
Are there any other APIs that have both promises and events? We certainly can't stop firing any of the events, so at best there would be a promise that resolves after (or before?) the events are fired? Anyway, I think this is probably a good idea, as people are having to add some logic to avoid handling the wrong fullscreenchange event, at least I found that in JW Player: https://github.com/jwplayer/jwplayer/pull/103/files I guess exitFullscreen() should also return a promise, to make it easier to do something after exiting fullscreen?
Yeah, either we resolve the promise in the task that dispatches the event (which means event -> promise) or we dispatch the event as the first callback from the promise (which means promise -> event). (There's also resolve the promise and queue a task to dispatch the event, which means promise -> event, but also allows any number of tasks to happen inbetween and therefore seems less desirable.) I suggested to Domenic that we standardize on one of these patterns, not sure if that has happened.
Simply resolving a promise as the last step of requestFullscreen() or exitFullscreen(), i.e. after any events fire, seems like it'd be simple enough. Because exitFullscreen() fires events and modified the fullscreen element stack(s) interleaved, it has to be after the events for the promise callbacks to see the final state.
(In reply to Philip Jägenstedt from comment #2) > Are there any other APIs that have both promises and events? There's the Font Loading API. See http://dev.w3.org/csswg/css-font-loading/#dom-fontfaceset-loadingdone. To save you time, the promise in that series of steps is fulfilled/rejected at the final step, after any events have been queued.
Yeah, I guess once I sort out the animation frame integration something sane will fall out.
I think it's pretty sorted already, while animation frame task isn't defined, I think everything you need to do would happen inside one of those tasks. AFAICT the Font Loading API doesn't queue tasks for fire events at all, it just fires loadingdone/loadingerror and then resolves the promise. I think the same would make sense for the Fullscreen API.
You can't really fire events from an asynchronous piece of code without queuing a task. That's a bug.
We won't have that problem assuming everything is done in an animation frame task.
FWIW, Screen Orientation API is also using Promises + events.
Is there anything blocking this other than making the time for the spec edits? Seems like a nice addition to me, and one that would be nice to have before anyone ships the unprefixed API.
Xidorn, what do you think? I'd like to have this done before shipping the unprefixed API, although that is not imminent in Chromium.
(In reply to Philip Jägenstedt from comment #12) > Xidorn, what do you think? I'd like to have this done before shipping the > unprefixed API, although that is not imminent in Chromium. It looks okay to me, although I'm concerned that in Gecko, having exitFullscreen() return promise probably needs more work than only having requestFullscreen() do that, because there is only one way to enter fullscreen, but about 3-4 ways to exit, and thus we currently don't track exitFullscreen() call at all. Other than this impl consideration, I don't see other issue off the top of my head.
I guess you still manage to fire the fullscreenchange events in these cases, but that you don't track why you're firing them so that you don't know which promise to resolve? If this would in any way slow down the effort to ship unprefixed fullscreen I think it can wait, as that's more important than polishing the API.
(In reply to Philip Jägenstedt from comment #14) > I guess you still manage to fire the fullscreenchange events in these cases, > but that you don't track why you're firing them so that you don't know which > promise to resolve? Yes, exactly.
Given that, do you think it's worth trying to promise-ify this API now, or would it become an obstacle for shipping in Gecko?
(In reply to Philip Jägenstedt from comment #16) > Given that, do you think it's worth trying to promise-ify this API now, or > would it become an obstacle for shipping in Gecko? Since promise-ifying this API later won't break any existing usage (although may introduce backward-compatible issue for authors), and Trident is shipping the unprefixed verion of the Fullscreen API (without returning promise, I believe), I don't think it is something we need to do immediately. I'm fine to see it being added to the spec, but probably not going to make it a blocker of unprefixing the API in Gecko.
Oh, it was news to me that IE has successfully shipped this API unprefixed. That's great, and it changes the balance on this I think. Promises would be nice, but I'm also not going to wait for it in order to ship in Blink.
I think we should promisify as soon as possible, so that implementers who are up for it feel sanctioned to do so.
(In reply to Philip Jägenstedt from comment #18) > Oh, it was news to me that IE has successfully shipped this API unprefixed. > That's great, and it changes the balance on this I think. Promises would be > nice, but I'm also not going to wait for it in order to ship in Blink. Well, accurately IE is not shipping that, but Edge is. [1] [1] https://msdn.microsoft.com/en-us/library/dn254939%28v=vs.85%29.aspx
Sure, no disagreement there. I think it would be very easy spec-wise. Here I must admit some ignorance about promises, but the timing is important, and the fullscreenchange event fires precisely when it does to avoid a bad frame of layout in case scripts change something when going fullscreen. If a promise is resolved at the same time, when will its callbacks run? It needs to be before that animation frame is "over" for the API to make sense.
This is always a bit tricky, but given that promises run on the microtask queue, and you queue a task to fire fullscreenchange, you should be able to get the promise callbacks to fire before the fullscreenchange listeners, which will be early enough. As long as you just do "resolve the promise" instead of "queue a task to resolve the promise" you'll be fine.
(In reply to Domenic Denicola from comment #22) > This is always a bit tricky, but given that promises run on the microtask > queue, and you queue a task to fire fullscreenchange, you should be able to > get the promise callbacks to fire before the fullscreenchange listeners, > which will be early enough. As long as you just do "resolve the promise" > instead of "queue a task to resolve the promise" you'll be fine. Actually it isn't "queue a task to fire an event named fullscreenchange" any longer, but just "fire an event named fullscreenchange" inside "as part of the next animation frame task, run these substeps." That was done in order to get the timing of the events right. So there's a risk that if the promise is resolved at the same time as those events are fired (not queued), the callbacks for that will be run too late. However, this is all actually undefined right now because "animation frame task" isn't defined anywhere. Quite possibly it should be defined to be followed by a microtask checkpoint then, but as a matter of implementation I think that's all now in V8, so how?
It seems in HTML the "Update the rendering" stuff happens after a microtask checkpoint... So the promise would end up resolving at end of the first task after painting happens. Hmm.
Oh, interesting. > Quite possibly it should be defined to be followed by a microtask checkpoint then, but as a matter of implementation I think that's all now in V8, so how? In Blink every task that's performed is followed by a microtask checkpoint: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/web/WebKit.cpp&sq=package:chromium&type=cs&l=80&rcl=1436599777 I would imagine that an animation frame task would also be followed by one. The question is indeed about the "update the rendering" stuff... > It seems in HTML the "Update the rendering" stuff happens after a microtask checkpoint... So the promise would end up resolving at end of the first task after painting happens. Hmm. I don't understand how the second sentence follows from the first. It seems like if it happens *after* a microtask checkpoint, then the promise callbacks will have run before painting.
To understand it you have to read HTML... It clearly says to run a set of callbacks as part of the quoted algorithm step and then do painting with no mention of tasks. Since there are no tasks, there's microtasks involved, and any promises resolved during those callbacks will run at the end of the first task after painting.
So you're assuming (or have previously discussed) that the animation frame task in which the fullscreenchange event is fired would happen at the same time as https://html.spec.whatwg.org/multipage/webappapis.html#run-the-animation-frame-callbacks, I guess. That makes sense. But all of those invoke a callback, which triggers https://html.spec.whatwg.org/multipage/webappapis.html#clean-up-after-running-a-callback, which will perform a microtask checkpoint. We just have to be sure that if animation frame task gets expanded, it also performs a microtask checkpoint.
Ah, good point. I was actually talking about https://html.spec.whatwg.org/multipage/webappapis.html#processing-model-9 step 8, but indeed, callbacks end up running microtasks. Okay, so fixing this is less troublesome than I thought.
But what if there are no callbacks to invoke, i.e. the "callbacks" in "For each entry in callbacks" in <https://html.spec.whatwg.org/multipage/webappapis.html#run-the-animation-frame-callbacks> is empty?
Yeah. I think when the animation frame task is defined, it should just include "perform a microtask checkpoint" at the end. Either that, or it should be based on the existing task machinery in such a way that it automatically gets the usual perform a microtask checkpoint that all tasks get.
Perhaps we should focus first on the semantics of fulfilling vs rejection. It's not entirely clear to me what we want to do there. HTML integration needs some changes either way (bug 28001).
(In reply to Anne from comment #31) > Perhaps we should focus first on the semantics of fulfilling vs rejection. > It's not entirely clear to me what we want to do there. First attempt: Create a promise in requestFullscreen() and exitFullscreen(). Fulfil it after firing fullscreenchange in the same algorithm invocation, and reject it when firing fullscreen error. What remains, I think, is the "No need to notify observers when nothing has changed" step, where I think it should be fulfilled.
https://github.com/whatwg/fullscreen/commit/fd19ec7a0f35e09b0cc4de0f676d37f6bd480560 https://github.com/whatwg/fullscreen/commit/d316cce76a5a5dad073425b973dd529dfa0d2a1a
I've reviewed the change and they look good, filed https://github.com/whatwg/fullscreen/issues/16 to keep track of the timing issue.