fullscreen icon indicating copy to clipboard operation
fullscreen copied to clipboard

Consume user gestures in requestFullscreen

Open foolip opened this issue 5 years ago • 20 comments

There is a Chromium (non-public) spoofing bug involving requestFullscreen that makes it seem like a good idea to consume user gestures in requestFullscreen as a mitigation. There may be web compat issues with such a change so it will be tested before a spec change is proposed in earnest.

This is a tracking bug for the tentative tests that may go with this change, so that they don't remain tentative forever.

@SummerLW @mustaqahmed

foolip avatar May 13 '19 19:05 foolip

What do you mean by consume user gesture?

upsuper avatar May 17 '19 09:05 upsuper

cc @EdgarChen

annevk avatar May 17 '19 09:05 annevk

Consuming user activation (or user gesture) means after requestFullscreen() is successful, the user activation is "gone" so another requestFullscreen() call will fail unless there is another user activation in-between. Essentially we are proposing one call per user interaction. Currently any number of calls can be made until the user activation expires, which seems counter-intuitive (and it is one reason the UI spoofing bug was possible).

A bit of a background: here are three different ways APIs typically depend on user activation.

mustaqahmed avatar May 17 '19 13:05 mustaqahmed

Sounds like a reasonable idea.

Another question would be whether other operations which require user activation / user gesture consumes that as well? i.e. if I trigger something else in user activation, and then requestFullscreen() should that still work?

upsuper avatar May 19 '19 00:05 upsuper

Other operation like pop up windows consume user activation. For the same user activation, if the pop up window opens first and it consume the user activation, the requestFullScreen() will not work. This is defined in the current user activation v2 spec.

On Sat, May 18, 2019 at 8:50 PM Xidorn Quan [email protected] wrote:

Sounds like a reasonable idea.

Another question would be whether other operations which require user activation / user gesture consumes that as well? i.e. if I trigger something else in user activation, and then requestFullscreen() should that still work?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/whatwg/fullscreen/issues/152?email_source=notifications&email_token=ACN2LK3KIYN6R5D3Z4WFKBLPWCP5HA5CNFSM4HMSX7EKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODVWYSIA#issuecomment-493717792, or mute the thread https://github.com/notifications/unsubscribe-auth/ACN2LK4Y6FU33QVR7MDPIOTPWCP5HANCNFSM4HMSX7EA .

LanWei22 avatar May 21 '19 03:05 LanWei22

FWIW, the main compat risk I see with this is that people might call video.requestFullscreen() + video.play() in the same event handler, and that consuming the gesture will mean the video won't play. Combination with other APIs that consume user activation are also possible of course, but we'll have to see which come up in practice.

@mustaqahmed @SummerLW do you anticipate this type of problem, and is there a fix we can make if it break sites?

foolip avatar May 22 '19 14:05 foolip

Yes, we expect problems like this. One possible way to fix this is to reverse the order of the two calls, so replacing: video.requestFullscreen() then video.play() with video.play() then video.requestFullscreen() would fix this specific problem. We expect this to work in most cases because only a handful of APIs consume user activation today (e.g., in Chrome only popups and some "types" of navigation consume). If the second API used with fullscreen is either popup or navigation, it could be hard to fix easily.

mustaqahmed avatar May 22 '19 14:05 mustaqahmed

Right, as long as no two APIs that consume user activation make sense together, then it can be avoided.

foolip avatar May 22 '19 15:05 foolip

@mustaqahmed is there an existing spec concept that can be referenced to produce a PR for what this change would be spec-side?

foolip avatar May 27 '19 15:05 foolip

The consumption concept is still not covered by any spec. We have this PR for User Activation v2 which would make the concept formal.

mustaqahmed avatar May 27 '19 15:05 mustaqahmed

I see, so the concept would be "consume user activation" which takes a window. Then the spec change would be to just do that somewhere in https://fullscreen.spec.whatwg.org/#dom-element-requestfullscreen, would it be after step 6 if error is false?

foolip avatar May 27 '19 16:05 foolip

Correct. I think we would add a step "consume the user activation" somewhere in between Steps 11~14. We have to make sure a rejected promise (Step 10) doesn't consume.

mustaqahmed avatar May 27 '19 16:05 mustaqahmed

@mustaqahmed I put together https://github.com/whatwg/fullscreen/pull/153, preview at https://whatpr.org/fullscreen/153.html#dom-element-requestfullscreen.

I put the consume in the sync part of the algorithm. This does mean that there are some error conditions which will still consume user activation, but the alternative would be to re-check the user activation in the "in parallel" part and have it be racy which of two calls to element.requestFullscreen() will succeed.

foolip avatar May 28 '19 12:05 foolip

https://github.com/web-platform-tests/wpt/pull/16758 has been merged into wpt repository. Three new tests about this behavior are now in https://w3c-test.org/fullscreen/api/ element-request-fullscreen-twice-manual.tentative.html element-request-fullscreen-two-elements-manual.tentative.html element-request-fullscreen-two-iframes-manual.tentative.html

LanWei22 avatar May 29 '19 18:05 LanWei22

We have User Activation v2 model in the HTML spec now. "Triggered by user activation" should now be changed accordingly, in addition to adding consumption behavior.

mustaqahmed avatar Dec 05 '19 18:12 mustaqahmed

Please ignore my last comment, filed #160 for that.

mustaqahmed avatar Dec 10 '19 16:12 mustaqahmed

Combination with other APIs that consume user activation are also possible of course, but we'll have to see which come up in practice.

pointer lock + fullscreen is rather common to be called together in the same event handler. If pointer lock uses the transient activation bit or even consumes it, it might be a compat risk. ~~And it seems like pointer lock doesn't use the concept of transient activation in Chromium yet?~~


Edit: It seems chromium does use the transient activation concept for pointer lock, but always allows pointer lock request if in fullscreen mode?

EdgarChen avatar Apr 24 '20 14:04 EdgarChen

In Chrome, PointerLock is gated by transient user activation but doesn't consume it; this is unlike Fullscreen which actually consumes it.

So if a click handler calls pointerlock then fullscreen, it would work, but not the other way around. @EdgarChen Does it match your observation?

We started consuming at fullscreen request last year, don't recall any compat issues. I couldn't confirm if Chrome may have pointerLock + fullscreen special handling. @NavidZ might know.

mustaqahmed avatar Apr 24 '20 17:04 mustaqahmed

Okay, so my concern is calling fullscreen then pointerlock. And It works in Chrome. Looks like Chrome does have special handling for pointerlock + fullscreen, https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/exclusive_access/mouse_lock_controller.cc;l=63;drc=d5c9de46626708f710bed9634e1c598d4ac506fa

EdgarChen avatar Apr 24 '20 21:04 EdgarChen

Okay, so my concern is calling fullscreen then pointerlock. And It works in Chrome. Looks like Chrome does have special handling for pointerlock + fullscreen, https://source.chromium.org/chromium/chromium/src/+/master:chrome/browser/ui/exclusive_access/mouse_lock_controller.cc;l=63;drc=d5c9de46626708f710bed9634e1c598d4ac506fa

Yeah. That seems like a hack to get this to work. Not sure what the right way around it would be with this proposal. We can try and remove the hack and see whether we get any compat and try to move the eco system. But it might take a while.

NavidZ avatar Apr 29 '20 00:04 NavidZ

An update to the original post:

There is a Chromium (non-public) spoofing bug involving requestFullscreen that ...

That bug is publicly accessible after Chrome fixed the problem (by consuming): https://crbug.com/852645.

mustaqahmed avatar Nov 03 '22 15:11 mustaqahmed

I'm somewhat concerned that consuming user activation limits legitimate use cases (e.g. fullscreen presentation and speakernotes on two different screens, fullscreen followed by calling other user activation consuming apis like pointerlock or playing a video?). Have we considered alternative ideas for preventing developers from setting up a confusing exiting from fullscreen into another fullscreen, e.g.

  • Requesting fullscreen when a fullscreen window is currently in the foreground drops out of fullscreen in that window, or
  • Exiting fullscreen using escape or other browser UI exits all (or all topmost) fullscreen windows on the current screen.

@EdgarChen would these ideas fix the issue you were referring to in https://github.com/whatwg/fullscreen/pull/153#issuecomment-1298746998 ?

flackr avatar Nov 03 '22 20:11 flackr

The bug is also public now: https://bugzilla.mozilla.org/show_bug.cgi?id=1631251. It doesn't involve multiple window, the issue is that page tries to requestFullscreen when user press the Esc key to exit fullscreen and the window can instantly enter fullscreen again.

EdgarChen avatar Nov 08 '22 12:11 EdgarChen

The bug is also public now: https://bugzilla.mozilla.org/show_bug.cgi?id=1631251. It doesn't involve multiple window, the issue is that page tries to requestFullscreen when user press the Esc key to exit fullscreen and the window can instantly enter fullscreen again.

It seems to me that consumption is not required here because Escape key is not an activation triggering input event. What am I missing here?

mustaqahmed avatar Nov 08 '22 16:11 mustaqahmed

Yes, escape key is not a activation triggering event. But if we are in the same valid transient activation where the fullscreen is requested, page could tries to trap us, for example, user found page enter fullscreen mode unexpectedly, so user would like to exit the fullscreen mode via Esc key, but page could instantly bring it back to fullscreen again.

EdgarChen avatar Nov 08 '22 20:11 EdgarChen