1129227 - Fullscreen API is banned in all async callbacks

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 1

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

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.

Comment 2

[Xidorn Quan [:xidorn] UTC+10]

Attachment: Bug 1129227 - Have fullscreen request allowed within a short time since the latest user input. r=smaug

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.

Comment 3

[Edgar Chen [:edgar]]

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?

Comment 4

[Xidorn Quan [:xidorn] UTC+10]

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

Comment 5

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=c7c3630a24815ab0d31a82c8d2ac30a22b8c1c8c

Comment 6

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&group_state=expanded&tier=1&revision=6d61a9182579b313055cc28a120020061f4fc351

Comment 7

[Edgar Chen [:edgar]]

Attachment: Bug 1129227 - Part 1: Merge nsContentUtils::CheckRequestFullscreenAllowed into GetFullscreenError;

Comment 8

[Edgar Chen [:edgar]]

Attachment: Bug 1129227 - Part 2: Switch Fullscreen to use transient-user-activation API;

Comment 9

[Edgar Chen [:edgar]]

Attachment: Bug 1129227 - Part 3: Add test for requesting fullscreen in async callback;

Comment 10

[Pulsebot]

Pushed by [e-mail]:
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

Comment 11

[Cristian Brindusan [:cbrindusan]]

Backed out 3 changesets (Bug 1129227) for wpt failures at active-lock.html.

Backout: https://hg.mozilla.org/integration/autoland/rev/ce29409beea772afd6f6d152e56f0a43f031433a

Push that started the failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception&revision=44219f4bafb049ad6b830afceb711f609d9b00d1&selectedJob=268689023

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

Comment 12

[Edgar Chen [:edgar]]

(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.

Comment 13

[Edgar Chen [:edgar]]

(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.

Comment 14

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=8818d8e85618d872da31ab67d4c026b02706daea

Comment 15

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f393ad834544662867f3a64452b12990bdfed12

Comment 16

[Pulsebot]

Pushed by [e-mail]:
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

Comment 17

[Raul Gurzau (:RaulG)]

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

Comment 20

[Chris Mills [:cmills]]

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.