1543439 - requestFullscreen denied from select onchange handler

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect, P2)

Description

[Graham King]

User Agent: Mozilla/5.0 (X11; Fedora; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0

Steps to reproduce:

Attempting to go full screen from a pulldown like this:

<select onchange="document.body.requestFullscreen()">
<option value="none">Do nothing</option>
<option value="full">Full screen</option>
</select>

Actual results:

This message appears in the console:

Request for fullscreen was denied because Element.requestFullscreen() was not called from inside a short running user-generated event handler.

The browser does not go full screen.

Expected results:

The browser should have gone full screen. The request WAS "called from inside a short running user-generated event handler".

This works in Chromium (71.0.3578.98).

Putting requestFullscreen in the option's click handler is also denied with the same message.

Putting the request in a button's click handler works correctly.

This bug might help locate the part of the code to fix: https://bugzilla.mozilla.org/show_bug.cgi?format=default&id=966493 . It refers to the same problem but with touch events.

Comment 1

[Henri Sivonen (:hsivonen)]

Johann, could this be related to the recent user interaction API blocking changes?

Comment 2

[Johann Hofmann [:johannh]]

Unlikely that this is related to our work. We did not change (and definitely not remove) places where user interaction is initiated. Maybe the onchange event is just separately dispatched and thus doesn't get the same user input stack? Smaug, can you take a look at this?

Considering that we want to use user interaction more extensively we should probably try to match Chrome's behavior here.

Graham, is this a regression (does it work in older release of Firefox, e.g. with https://mozilla.github.io/mozregression/)? Also, does this work if you try a handling a different event type on the select element?

Thanks!

Comment 3

[Graham King]

is this a regression

I don't know, I don't have access to an older Firefox. I do know that it doesn't work in Edge 38 or Safari either.

does this work if you try a handling a different event type on the select element?

No. <select onclick="document.body.requestFullscreen()"> is also denied with the same message.

Comment 4

[Olli Pettay [:smaug][[e-mail]](vacation until Nov 16)]

This smells like an issue with e10s select. I don't recall where we set the flag in that case that we're dealing with user input.

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Here's where we fire the change event:

https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/toolkit/actors/SelectChild.jsm#230-233

Perhaps we need to add a setHandlingUserInput RAII to this block where we're firing events?: https://searchfox.org/mozilla-central/rev/75294521381b331f821aad3d6b60636844080ee2/dom/interfaces/base/nsIDOMWindowUtils.idl#1733-1740

Comment 6

[Edgar Chen [:edgar]]

We allow open popup with e10s select, the popup state was pushed in https://searchfox.org/mozilla-central/rev/f4c39907e0b527dc4b9356a1eeb8c6e6c62d383a/dom/events/EventListenerManager.cpp#1149-1150.
Perhaps we need to handle IsHandlingUesrInput flag here as well?

Comment 7

[Edgar Chen [:edgar]]

(In reply to Graham King from comment #3)

No. <select onclick="document.body.requestFullscreen()"> is also denied with the same message.

I think this is a different bug, file bug 1552803 for click event on the select element.

Comment 8

[Edgar Chen [:edgar]]

We have a similar issue on the input date, like

<input type="date" onchange="document.body.requestFullscreen()">

Comment 9

[Edgar Chen [:edgar]]

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

Comment 10

[Edgar Chen [:edgar]]

Attachment: Bug 1543439 - Part 2: Move is-user-interaction checks out of PresShell::EventHandler::PrepareToDispatchEvent;

Comment 11

[Edgar Chen [:edgar]]

Attachment: Bug 1543439 - Part 3: change and click event fired from browser UI should be considered as an user interaction event;

For the case like e10s select and input date, etc.

Comment 12

[Edgar Chen [:edgar]]

Attachment: Bug 1543439 - Part 1: Move special handling for MouseDown/Up event out of AutoHandlingUserInputStatePusher;

PresShell::EventHandler::HandleEventWithCurrentEventInfo is the only possible
place will do such handling; other places pass either a nullptr or a
non-MouseDown/Up event.

Comment 13

[Edgar Chen [:edgar]]

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

Comment 14

[Edgar Chen [:edgar]]

Attachment: Bug 1543439 - Part 4: Add tests;

Comment 15

[Edgar Chen [:edgar]]

https://treeherder.mozilla.org/#/jobs?repo=try&tier=1&exclusion_profile=false&group_state=expanded&revision=66bef27d82115691258001d762e31c05ba8461a8

Comment 16

[Edgar Chen [:edgar]]

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

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/750e3e013c4c
Part 1: Move special handling for MouseDown/Up event out of AutoHandlingUserInputStatePusher; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/ffc50cffcf1e
Part 2: Move is-user-interaction checks out of PresShell::EventHandler::PrepareToDispatchEvent; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/febbc0b025c5
Part 3: change and click event fired from browser UI should be considered as an user interaction event; r=masayuki
https://hg.mozilla.org/integration/autoland/rev/81792a8b6423
Part 4: Add tests; r=mconley

Comment 18

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/750e3e013c4c
https://hg.mozilla.org/mozilla-central/rev/ffc50cffcf1e
https://hg.mozilla.org/mozilla-central/rev/febbc0b025c5
https://hg.mozilla.org/mozilla-central/rev/81792a8b6423

Comment 19

[Graham King]

Fedora has Firefox 69 now, and I can confirm this is fixed. Thanks!