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)
Tracking
()
RESOLVED
FIXED
mozilla35
People
(Reporter: jujjyl, Assigned: wesj)
References
Details
(Whiteboard: [games])
Attachments
(1 file)
(deleted),
patch
|
smaug
:
review+
lmandel
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•11 years ago
|
Whiteboard: [games]
Comment 1•11 years ago
|
||
Wouldn't "touchbegin" be more like "mousedown" than "click"?
Reporter | ||
Comment 2•11 years ago
|
||
Oh yes, indeed. Quick testing shows that "mousedown" events also allows fullscreen requests.
Reporter | ||
Comment 3•11 years ago
|
||
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•11 years ago
|
||
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)
Comment 5•11 years ago
|
||
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)
Reporter | ||
Comment 6•11 years ago
|
||
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.
Assignee | ||
Comment 7•10 years ago
|
||
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 8•10 years ago
|
||
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+
Reporter | ||
Comment 9•10 years ago
|
||
Thanks for working on this! Agreed, touchbegin and touchend are probably the important ones, whereas touchmove and touchcancel are not.
Assignee | ||
Comment 10•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/1ecd31c5070b
I'd like to uplift this if all goes well....
Comment 11•10 years ago
|
||
Assignee: nobody → wjohnston
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Assignee | ||
Comment 12•10 years ago
|
||
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?
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8484414 [details] [diff] [review]
Patch
Already on Aurora :)
Attachment #8484414 -
Flags: approval-mozilla-aurora?
Updated•10 years ago
|
status-firefox33:
--- → wontfix
status-firefox34:
--- → unaffected
status-firefox35:
--- → fixed
status-firefox36:
--- → fixed
Comment 14•10 years ago
|
||
Comment on attachment 8484414 [details] [diff] [review]
Patch
Beta+
Attachment #8484414 -
Flags: approval-mozilla-beta? → approval-mozilla-beta+
Comment 15•10 years ago
|
||
Comment 18•10 years ago
|
||
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
Reporter | ||
Comment 19•10 years ago
|
||
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•10 years ago
|
||
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•10 years ago
|
||
> 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.
Description
•