Closed Bug 1129227 Opened 10 years ago Closed 5 years ago

Fullscreen API is banned in all async callbacks

Categories

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

Product:
Core
Component:
DOM: Core & HTML
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla71
Tracking Flags:
Tracking Status
firefox71 --- fixed

People

(Reporter: xidorn, Assigned: edgar)

References

(Blocks 2 open bugs, Regressed 1 open bug)

Details

Attachments

(3 files, 1 obsolete file)

Bug 1129227 - Part 1: Merge nsContentUtils::CheckRequestFullscreenAllowed into GetFullscreenError;
(deleted), text/x-phabricator-request
Details
Bug 1129227 - Part 2: Switch Fullscreen to use transient-user-activation API;
(deleted), text/x-phabricator-request
Details
Bug 1129227 - Part 3: Add test for requesting fullscreen in async callback;
(deleted), text/x-phabricator-request
Details
With code for example: element.onclick = function() { setTimeout(function() { element.requestFullscreen(); }, 0); }; the request won't succeed because it is not called during user event handling. In Chrome, this async delay is allowed up to 1s. I guess the behavior of Chrome is actually what we wanted, or we wouldn't have the timeout checking [1]. https://dxr.mozilla.org/mozilla-central/source/dom/events/EventStateManager.cpp#3620
Component: Document Navigation → DOM
That time out check is different. It is about the case if handling user input takes more than X time. For example if there is a synchronous XHR in the event listener.
Depends on: 1185052
Blocks: fullscreen-api
Blocks: 1436474
Priority: -- → P5
Assignee: nobody → xidorn+moz
This enables fullscreen request in async functions, but still apply the restriction that it has to appear to be originated from a user input. This isn't strictly spec-conformant, nor were we. This would likely resolve some webcompat issue we are seeing due to asynchronous fullscreen request, and I don't think it would lead to anything insecure.
Attachment #9008620 - Attachment is obsolete: true
Component: DOM → DOM: Core & HTML

I am currently working on a new API that could cover this usage, bug 1577499. Do you mind if I take this bug for experiment?

Flags: needinfo?(xidorn+moz)

It's yours now. Thanks for taking it :)

Assignee: xidorn+moz → echen
Flags: needinfo?(xidorn+moz)
Blocks: 1441881
Priority: P5 → P3
Depends on: 1577499
No longer depends on: 1185052
Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/3edc8921609b Part 1: Merge nsContentUtils::CheckRequestFullscreenAllowed into GetFullscreenError; r=smaug https://hg.mozilla.org/integration/autoland/rev/69f7db4e75bf Part 2: Switch Fullscreen to use transient-user-activation API; r=smaug https://hg.mozilla.org/integration/autoland/rev/44219f4bafb0 Part 3: Add test for requesting fullscreen in async callback; r=smaug
Depends on: 1584411
Flags: needinfo?(echen)

(In reply to Cristian Brindusan [:cbrindusan] from comment #11)

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268689023&repo=autoland&lineNumber=4740

Hmm, not sure why some fullscreen tests get timed out on android, needs some investigation.

(In reply to Edgar Chen [:edgar] from comment #12)

(In reply to Cristian Brindusan [:cbrindusan] from comment #11)

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=268689023&repo=autoland&lineNumber=4740

Hmm, not sure why some fullscreen tests get timed out on android, needs some investigation.

Okay, the timed-out happens on the tests for screen.orientation, only Android supports this API.
We should update the meta data to expect the different result on Android.

No longer depends on: 1584411
Regressions: 1584411
Pushed by echen@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d9f6b3bbf9f7 Part 1: Merge nsContentUtils::CheckRequestFullscreenAllowed into GetFullscreenError; r=smaug https://hg.mozilla.org/integration/autoland/rev/7dfb03851178 Part 2: Switch Fullscreen to use transient-user-activation API; r=smaug https://hg.mozilla.org/integration/autoland/rev/603a39b6be5a Part 3: Add test for requesting fullscreen in async callback; r=smaug

https://hg.mozilla.org/mozilla-central/rev/d9f6b3bbf9f7
https://hg.mozilla.org/mozilla-central/rev/7dfb03851178
https://hg.mozilla.org/mozilla-central/rev/603a39b6be5a

Status: NEW → RESOLVED
Closed: 5 years ago
status-firefox71: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla71

I'm not convinced this needs any documentation; sounds like more of a bug fix/compat change than a new feature. If I'm wrong, please let me know what you think it needs.

Keywords: dev-doc-needed
Blocks: 1577516
Regressions: 1609180
Regressions: 1631251
Regressions: 1691887
Regressions: 1790576
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: