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

Time to dispatch the fullscreenchange event #5

Closed
upsuper opened this issue Jul 8, 2015 · 11 comments
Closed

Time to dispatch the fullscreenchange event #5

upsuper opened this issue Jul 8, 2015 · 11 comments

Comments

@upsuper
Copy link
Member

upsuper commented Jul 8, 2015

In the current spec, the fullscreenchange event is dispatched synchronously inside the loop of changing the document state, which means the event handler may reach an incomplete state of style. For requestFullscreen, the state of the child document isn't flipped. For exitFullscreen, the state of the parent document isn't flipped.

As far as I can see, it doesn't match any of the current impl. Gecko, Blink (assuming WebKit shares the same code), Trident (including Edge which is shipping the unprefixed Fullscreen API) dispatch all fullscreenchange events with a complete state.

Personally I don't have a strong opinion about which is better. But given all impls agree with the same way, and I don't see reaching incomplete state makes much sense, I propose we change the spec to match the impls.

@annevk
Copy link
Member

annevk commented Jul 9, 2015

Are you proposing that we have two loops? One to change the state, and then another one to dispatch the events?

@upsuper
Copy link
Member Author

upsuper commented Jul 9, 2015

Yes. Actually it would be the third loop for exiting fullscreen.

@annevk
Copy link
Member

annevk commented Jul 9, 2015

@foolip ^^

@foolip
Copy link
Member

foolip commented Jul 9, 2015

Blink still implements the events using the old "Queue a task to fire an event named fullscreenchange with its bubbles attribute set to true on doc" and similar steps.

I think what I would like to see is that requestFullscren() and exitFullscreen() both return promises that are resolved when the transition is complete, and that the events are all fired (synchronously) either immediately before or after that. I suppose it amounts to what @upsuper is suggesting except with promises.

@annevk
Copy link
Member

annevk commented Jul 9, 2015

We want to synchronize the events with animation frames, no? I guess we could resolve a promise from such a frame too.

@foolip
Copy link
Member

foolip commented Jul 9, 2015

Yes, the promises should be resolved and the events fired in an animation frame task, but after everything else is done like @upsuper suggests sure seems less risky.

@annevk
Copy link
Member

annevk commented Jul 9, 2015

Agreed, will change.

@annevk
Copy link
Member

annevk commented Jul 13, 2015

So I think the problem with this was that with the potential for cross-process scenarios we'd end up with a way where you could observe a state change while the event hasn't fired. But maybe we should cross that bridge when it comes.

@foolip
Copy link
Member

foolip commented Jul 13, 2015

Do you mean that the new size of an iframe could be observed before the fullscreenchange event is fired? That does seem like a plausible bug if this is implemented as the top-level browsing context exiting fullscreen and notifying its child frames as two separate steps. I don't think it's a fundamental problem though, there's a resize event on window and it ought to be possible to somehow synchronize the fullscreenchange event with that...

@upsuper
Copy link
Member Author

upsuper commented Jul 13, 2015

I don't think it's a fundamental problem either, especially given that we don't even specify the order of the fullscreenchange event and resize event. (Actually Gecko currently can trigger multiple resize events for fullscreen... which I'm going to fix some day...)

@annevk annevk closed this as completed in 6d66d6d Jul 13, 2015
@foolip
Copy link
Member

foolip commented Jul 13, 2015

Changes look good. The substeps of the exit fullscreen algorithm are a bit hard to follow, but I don't know what would improve it. Maybe a note to say that the result is that the events will be fired from the innermost document to the outermost, or something.

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

3 participants