966493 - Cannot request fullscreen when inside a touchstart event.

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Jukka Jylänki]

The specs say that due to security reasons, both fullscreen and pointerlock requests need to be run "inside short-running user-generated event handlers".

Steps to reproduce:

1. Visit https://dl.dropboxusercontent.com/u/40949268/Bugs/request_fullscreen_touchbegin_bug.html with a Nexus 4 Android phone running Firefox Nightly. (did not test other versions)
2. Connect 'adb logcat -s GeckoConsole' on a host PC to obtain a log of the page
3. Tap on the image.

Expected: Fullscreen requests should be allowed from inside "touchbegin" events and the cat should go fullscreen.

Observed: The following text is printed to log on each tap:

E/GeckoConsole(10289): [JavaScript Warning: "Request for full-screen was denied because Element.mozRequestFullScreen() was not called from inside a short running user-generated event handler." {file: "http://192.168.14.4:8000/request_fullscreen_touchbegin_bug.html" line: 12}]

In the code, the fullscreen request is being performed inside a "touchbegin" event handler. This is comparable to mouse click events with the "click" event handler, where fullscreen requests do work, so it would be expected that touchbegin should allow fullscreen requests as well.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Wouldn't "touchbegin" be more like "mousedown" than "click"?

Comment 2

[Jukka Jylänki]

Oh yes, indeed. Quick testing shows that "mousedown" events also allows fullscreen requests.

Comment 3

[Jukka Jylänki]

Also to consider, would it be good to set "touchmove" and "touchend" events to also allow fullscreen and pointerlock requests? I am indifferent for this - it's important mostly for touchbegins, but good to give those events a thought at least as well.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Looks like the relevant bit is nsContentUtils::IsRequestFullScreenAllowed, which calls nsEventStateManager::IsHandlingUserInput.

Olli, do we not flag touch events as user input?

Comment 5

[Olli Pettay [:smaug][[e-mail]]]

Currently no
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsPresShell.cpp?rev=c1a6fdd3fa01&mark=6947-6947#6945

touchstart should be able to start fullscreen. Maybe also touchend.

Comment 6

[Jukka Jylänki]

Yeah, thinking about this - touchend is also very important to be supported, since a lot of UIs implement e.g. buttons in games in such a way that the buttons are triggered when the touch is released, and not immediately on press.

Comment 7

[Wesley Johnston (:wesj)]

Attachment: Patch

I set this for everything bug touch_cancel. Not sure if we care not to doit there (or touchmove?).

Comment 8

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 8484414 [details] [diff] [review]
Patch

Maybe drop touchmove. We don't set isHandlingUserInput with mousemove either (even if left button is down).

Comment 9

[Jukka Jylänki]

Thanks for working on this! Agreed, touchbegin and touchend are probably the important ones, whereas touchmove and touchcancel are not.

Comment 10

[Wesley Johnston (:wesj)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecd31c5070b

I'd like to uplift this if all goes well....

Comment 11

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1ecd31c5070b

Comment 12

[Wesley Johnston (:wesj)]

Comment on attachment 8484414 [details] [diff] [review]
Patch

Requesting beta approval as well because this is bugging some webdevs.

Approval Request Comment
[Feature/regressing bug #]: 984271 (Firefox 31) 
[User impact if declined]: Can't use touchevent-fast-click tricks to go into fullscreen mode.
[Describe test coverage new/current, TBPL]: none
[Risks and why]: Low risk. Sets a flag on touch events traveling through presShell the same way we do for mouse events.
[String/UUID change made/needed]: None.

Comment 13

[Wesley Johnston (:wesj)]

Comment on attachment 8484414 [details] [diff] [review]
Patch

Already on Aurora :)

Comment 14

[Lawrence Mandel [:lmandel] (use needinfo)]

Comment on attachment 8484414 [details] [diff] [review]
Patch

Beta+

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/f6c14ee20738

Comment 18

[syed]

I'm using selenium to automate a website testing process, the web site has videos like youtube and im suppose to test the in fullscreen mode.
I'm getting the following error in browser console when i click on the fullscreen element via selenium.

Request for full-screen was denied because Element.mozRequestFullScreen() was not called from inside a short running user-generated event handler.

Browser Version:35
Selenium Version:2.44.0

Comment 19

[Jukka Jylänki]

I am not familiar with Selenium, but if it is based on Firefox/gecko 35, then this issue should have been fixed there already. Can you double check and print the call stack where the fullscreen request occurs to check which call stack it is running from and it is indeed inside a user-generated event handler?

Comment 20

[syed]

I checked the issue with the latest version 36 and the issue is still replicable.
The problem is that i dont get any exception in my code when i try to click on the fullscreen element (as selenium is able to find and click on the element) but nothing happen's in the browse. The only place where i can see the exception is within FF browser console and it states Request for full-screen was denied because Element.mozRequestFullScreen() was not called from inside a short running user-generated event handler.
So not sure where should i print the call stack ?

Comment 21

[Boris Zbarsky [:bzbarsky]]

> So not sure where should i print the call stack ?

In the script that makes the mozRequestFullScreen() call, assuming you control that script.  If you don't control it, things are a bit more complicated.

> as selenium is able to find and click on the element

How exactly does it do that?  Note that the fullscreen logic checks for actual OS-level events (or a reasonable facsimile thereof), not just DOM events...

> The only place where i can see the exception is within FF browser console 

That's not an exception, just an informational message.