Closed Bug 1129227 Opened 6 years ago Closed 10 months ago

Fullscreen API is banned in all async callbacks

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla71
Tracking Status
firefox71 --- fixed

People

(Reporter: xidorn, Assigned: edgar)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files, 1 obsolete file)

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: 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
Status: NEW → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla71
Duplicate of this bug: 1441881
Duplicate of this bug: 1436474

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
You need to log in before you can comment on or make changes to this bug.