1195213 - Allow requesting fullscreen on element regardless of its relationship with the current fullscreen element

Status: RESOLVED INVALID

(Core :: General, defect)

Description

[Xidorn Quan [:xidorn] UTC+11]

The Fullscreen API has updated the "fullscreen element ready check" algorithm, and the new one no longer reject fullscreen request because the element is an ancestor of the current fullscreen element.

Comment 1

[Thomas Wisniewski]

It looks like this part of the spec has been changing a fair bit lately on github. It now reads:

>requestFullscreen() step 4. If any of the following conditions are false, then set error to true:
>  The element’s namespace is the HTML namespace or pending is an SVG svg or MathML math element. (bug 1305928)
>  The fullscreen element ready check for the element returns true:
>    - If document has no browsing context, then return false.
>    - If document's browsing context is a top-level browsing context, then return true.
>    - If document's browsing context has a browsing context container that is an iframe element with an allowfullscreen attribute specified, and whose node document is allowed to use the feature indicated by allowattribute, then return true.
>    - Return false.
>  Fullscreen is supported:
>    - There is no previously-established user preference, security risk, or platform limitation.
>  This algorithm is allowed to request fullscreen:
>    - The algorithm is triggered by user activation, or
>    - The algorithm is triggered by a user generated orientation change.

I suppose that means the following checks in Firefox are eligible for the chopping block: FullscreenDeniedHidden, FullscreenDeniedSubDocFullScreen, and FullscreenDeniedNotDescendant.

Also FullscreenDeniedMovedDocument if that doesn't fall under the "security risk" category.

And possibly also FullscreenDeniedNotFocusedTab and FullscreenDeniedFocusedPlugin, unless we just want to consider those default "user preferences".

Comment 2

[Xidorn Quan [:xidorn] UTC+11]

FullscreenDeniedSubDocFullScreen and FullscreenDeniedNotDescendant are the reasons we want to remove in this bug.

FullscreenDeniedHidden, FullscreenDeniedNotFocusedTab, and FullscreenDeniedFocusedPlugin need to be kept for security reason, I believe. Probably the first two should be added to the spec. The third is security risk from implementation details, which may not need to be speced.

FullscreenDeniedMovedDocument should probably be added to the spec, because it seems tricky to implement it right when an element is moved to a different document after it requests fullscreen.

Comment 3

[Thomas Wisniewski]

Attachment: 1195213-allow-requestFullscreen-on-elements-regardless-of-relationship-with-current-fullscreen-element.diff

Alright, here's a patch to just remove those two checks. Try seem fine: https://treeherder.mozilla.org/#/jobs?repo=try&revision=5a08852db77a

I'll ask smaug to review it when he's a bit less overloaded.

Comment 4

[Xidorn Quan [:xidorn] UTC+11]

I don't think it could be such a trivial change... I think there are some invariants we need to change in the fullscreen stack first, but it doesn't come to me immediately what are they.

Comment 5

[Xidorn Quan [:xidorn] UTC+11]

So there are at least two invariants we get for free with the hierarchy restriction:
(1) an ancestor of the current fullscreen element cannot request fullscreen
(2) any element already in the fullscreen stack cannot request fullscreen

I guess given fullscreen elements rendered as out-of-flow frames, (1) isn't that important, but it still needs check.

The change to invariant (2) needs more carefulness, otherwise some element may end up in a weird state. It is necessary to check the spec about what should happen when an element already in the fullscreen stack requests fullscreen again, and what should happen when the double-fullscreened element exit fullscreen.

Comment 6

[Xidorn Quan [:xidorn] UTC+11]

In addition, I'd prefer hold back implementing this for now.

We are coordinating shipping unprefixed Fullscreen API with Blink and Edge, and we have agreed on a certain feature set we would ship with unprefixing. This is not in that set, so implementing this could add compatibility risk to shipping unprefixed API for other engines (and actually for us as well), and consequently delay the unprefixing.

Since I'd prefer not to delay unprefixing at all costs, I don't think we should extend our feature set of Fullscreen API at present.

Comment 7

[Xidorn Quan [:xidorn] UTC+11]

We are reverting this in the spec at whatwg/fullscreen#91, for making it easier to dispatch events to element rather than document. Closing this as INVALID now.

Will file a new bug for the new way to dispatch event.