Closed Bug 966493 Opened 11 years ago Closed 10 years ago

Cannot request fullscreen when inside a touchstart event.

Categories

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

29 Branch
ARM
Android
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla35
Tracking Status
firefox33 --- wontfix
firefox34 --- fixed
firefox35 --- fixed
firefox36 --- fixed

People

(Reporter: jujjyl, Assigned: wesj)

References

Details

(Whiteboard: [games])

Attachments

(1 file)

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.
Whiteboard: [games]
Wouldn't "touchbegin" be more like "mousedown" than "click"?
Oh yes, indeed. Quick testing shows that "mousedown" events also allows fullscreen requests.
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.
Looks like the relevant bit is nsContentUtils::IsRequestFullScreenAllowed, which calls nsEventStateManager::IsHandlingUserInput. Olli, do we not flag touch events as user input?
Flags: needinfo?(bugs)
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.
Flags: needinfo?(bugs)
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.
Attached patch PatchSplinter Review
I set this for everything bug touch_cancel. Not sure if we care not to doit there (or touchmove?).
Attachment #8484414 - Flags: review?(bugs)
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).
Attachment #8484414 - Flags: review?(bugs) → review+
Thanks for working on this! Agreed, touchbegin and touchend are probably the important ones, whereas touchmove and touchcancel are not.
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecd31c5070b I'd like to uplift this if all goes well....
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
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.
Attachment #8484414 - Flags: approval-mozilla-beta?
Attachment #8484414 - Flags: approval-mozilla-aurora?
Comment on attachment 8484414 [details] [diff] [review] Patch Already on Aurora :)
Attachment #8484414 - Flags: approval-mozilla-aurora?
Attachment #8484414 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
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
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?
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 ?
> 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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: