700123 - Implement Fullscreen Keyboard Lock

Status: RESOLVED FIXED

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

Description

[Boaz Sender]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_0) AppleWebKit/535.2 (KHTML, like Gecko) Chrome/15.0.874.106 Safari/535.2




Expected results:


* Use case
  * When I crouch and walk forward in a game, the window should not close
* Request description
  * allow developer to lock keyboard. Similar to emerging mouselock spec.
* Target W3C group
  * Web events group
* Traction
  * chrome issue: http://code.google.com/p/chromium/issues/detail?id=84332
  * chrome design doc draft: https://sites.google.com/a/chromium.org/dev/developers/design-documents/reserved-keys-api

Comment 1

[warcraftthreeft]

Why would the window close when you are crouching and walking? If you have focus on a canvas object or something and press say ctrl+w it doesn't close the window in any browser.

Comment 2

[Peter Kasting]

(In reply to warcraftthreeft from comment #1)
> If you have
> focus on a canvas object or something and press say ctrl+w it doesn't close
> the window in any browser.

That should not be true in Chrome, where we prevent some keystrokes (e.g. ctrl-w) from being sent to webpages by default so badly-behaving pages don't result in the user being unable to close/change tabs, etc.

Comment 3

[warcraftthreeft]

Woah, you're right. I just tested it with one of my canvas projects. Backspace goes back a page in Chrome (along with the ctrl+w close). I guess this is needed. Didn't realize user agents could take precedent over a web page with focus and key events.

Comment 4

[Chris Rebert]

Presumably the relevant spec here is now: https://w3c.github.io/keyboard-lock/

Comment 5

[Pierre Ossman]

Chrome seems to be shipping this in the next release:

https://www.chromestatus.com/features/5642959835889664

Comment 6

[:Harald Kirschner :digitarald]

Referenced in https://github.com/microsoft/vscode/issues/85252 . I wonder if other editors also run into this.

Comment 7

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Hi Thomas,
Apart from the "allow content to override browser shortcuts such as cmd+t in fullscreen mode" use case, are there any other webcompat use cases due to the lack of keyboard locking API support in Firefox? I don't see from the bug dependency and I would like to know if bug 1593883 comment 7 is a good enough solution. Thank you.

Comment 8

[Thomas Wisniewski [:twisniewski]]

No, but I definitely can see games or apps not wanting to use fullscreen mode (for instance, so they can use multiple canvases or other HTML areas of the page to present extra information). So we might still hear about webcompat issues even with that work-around. After all, as per comment 2, Chrome prevented some shortcuts/key combinations in fullscreen mode, but still went ahead with a full locking API, so I wouldn't be surprised if there were other issues encouraging them to do so. It may also be more relevant for pinned apps/"PWAs".

Comment 9

[Hsin-Yi Tsai (she/her)[:hsinyi]]

The standards-position is still not resolved: https://github.com/mozilla/standards-positions/issues/196
There is a new idea which aims to improve the one Google previously proposed. However, it's still very early in the spec process.

Comment 10

[Jeff Muizelaar [:jrmuizel]]

https://github.com/WebKit/standards-positions/issues/481#issuecomment-3677826930 suggests that this is shipping in Safari TP

Comment 11

[Kagami Rosylight [:saschanaz] (they/them)]

Apparently Chrome implicitly locks Control+N/T/W on fullscreen. ("lock" meaning the keys are passed to key events in a cancelable state.)

Comment 12

[Kagami Rosylight [:saschanaz] (they/them)]

We have a few reserved keys, should be worth checking whether those are already implicitly locked on other browsers.

Comment 13

[Kagami Rosylight [:saschanaz] (they/them)]

Bug 1821886 makes me concern that blocking Escape key would cause the same security issue - web pages may prevent the default of escape and show fake browser UI. Seems like we should at least show some UI for warning if such blocking happens. Thoughts?

Comment 14

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(PTO: 5/2-5/10)]

(I'm still sick so that my brain is not fully working...)

Yeah, silently allowing keyboard lock confuses the users since some shortcut keys are disabled.

Well, I don't find how the users to unlock it forcibly. Cannot do that unless changing the active tab with non-keyboard input device? If so, we still need to reserve some shortcut keys as for the users who use only keyboard... Oh, or, we should add a new reserved shortcut key to exit from keyboard lock like Ctrl + Alt + Delete (meaning enough complicated for using on apps) and show the shortcut key in the warning message.

Comment 15

[Kagami Rosylight [:saschanaz] (they/them)]

(In reply to Masayuki Nakano [:masayuki] (he/him)(JST, +0900) from comment #14)

Well, I don't find how the users to unlock it forcibly. Cannot do that unless changing the active tab with non-keyboard input device? If so, we still need to reserve some shortcut keys as for the users who use only keyboard... Oh, or, we should add a new reserved shortcut key to exit from keyboard lock like Ctrl + Alt + Delete (meaning enough complicated for using on apps) and show the shortcut key in the warning message.

So right now https://searchfox.org/firefox-main/rev/7b1a35d6c70c08d2966c6c8dfc9aa9c04319f721/layout/base/PresShell.cpp#9389-9435 makes double escape keydown to always escape from fullscreen regardless of chrome-context preventDefault(), we could do the same for fullscreened web content. Safari made long-keydown to always escape, we could follow that too.

Either way we should show some UI or otherwise users will be confused.

Comment 16

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Let me summarize the offline discussions here:

1. We reached consensus on long-press ESC to exit fullscreen. Long-press is Safari’s behavior.  We could land this part first to enable early testing without being blocked by the UI decision below.
2. UI design - we will need UX support to finalize it, basically we have the below considerations to address nicely:
   • Show UI to explain how to unlock the keyboard for users when entering into the fullscreen mode, because it's a risky feature from the security point of view.
   • Show UI when esc key is default-prevented, note user they need to long-press to exit fullscreen.
   • In Chrome, pressing ESC three times in sequence shows the message to press and hold ESC to exit fullscreen/keyboard lock.
   • In Safari, we don't seem to see warning UIs though
   • In games, it’s common to have ESC open and close the game menu. It would be annoying to exit fullscreen when pressing ESC multiple times.

Comment 17

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 3: Make exiting fullscreen require long-pressing the Escape key when the fullscreen keyboard lock is enabled. r?#dom-core,edgar

Comment 18

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Attachment: Chrome’s current message in fullscreen with keyboard lock active.png

Chrome’s current message when entering fullscreen with keyboard lock active

Comment 19

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Attachment: Chrome’s current message in fullscreen without keyboard lock active

Chrome’s current message when entering fullscreen without keyboard lock active

Comment 20

[Hsin-Yi Tsai (she/her)[:hsinyi]]

Another discussion summary:

(In reply to Hsin-Yi Tsai (she/her)[:hsinyi] from comment #16)

Let me summarize the offline discussions here:

1. We reached consensus on long-press ESC to exit fullscreen. Long-press is Safari’s behavior.  We could land this part first to enable early testing without being blocked by the UI decision below.

Long-press is still wanted. We plan to provide this behavior through keyboard lock API, rather than using preventDefault()-based opt-in approach. That means, we need to add the API part on top of the mechanism implemented in this patch.

2. UI design - we will need UX support to finalize it, basically we have the below considerations to address nicely:
   • Show UI to explain how to unlock the keyboard for users when entering into the fullscreen mode, because it's a risky feature from the security point of view.

This is still needed. With the keyboard lock API, we should show 2 messages when entering fullscreen depending on if keyboard lock is active or not.
Chrome's screenshots are attached: Chrome’s current message in fullscreen with keyboard lock active.png and Chrome’s current message in fullscreen without keyboard lock active

- Show UI when esc key is default-prevented, note user they need to long-press to exit fullscreen. 

This when esc key is default-prevented criterion is not applicable now.

- In Chrome, pressing ESC three times in sequence shows the message to press and hold ESC to exit fullscreen/keyboard lock.

Still needed/wanted.

- In Safari, we don't seem to see warning UIs though
- In games, it’s common to have ESC open and close the game menu. It would be annoying to exit fullscreen when pressing ESC one and two times.

Comment 21

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 1: Add keyboardLock option to requestFullscreen. r?#dom-core

Comment 22

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 2: Track the fullscreen keyboard lock status and propagate it to the browser chrome. r?#dom-core

Comment 23

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 4: Make exiting PointerLock occur on long-press when fullscreen keyboard lock is enabled. r?#dom-core

Comment 24

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 5: Display different UI message upon entering fullscreen with keyboard lock. r?#dom-core

Comment 25

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 7: Show a warning when user presses Escape while fullscreen keyboard lock is enabled. r?#dom-core

We warn the user the first time they press escape, and then on any
subsequent triple-clicks.

Comment 26

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 8: Update fullscreen keyboard lock status when entering fullscreen-in-fullscreen. r?#dom-core

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/ac8fd5e4de3e
https://hg.mozilla.org/integration/autoland/rev/ba3ad8f89a2b
Part 1: Add keyboardLock option to requestFullscreen. r=webidl,saschanaz
https://github.com/mozilla-firefox/firefox/commit/11794474237f
https://hg.mozilla.org/integration/autoland/rev/a7ac12f1c690
Part 2: Track the fullscreen keyboard lock status and propagate it to the browser chrome. r=dom-core,edgar
https://github.com/mozilla-firefox/firefox/commit/a11e06665e1c
https://hg.mozilla.org/integration/autoland/rev/2a216b639540
Part 3: Make exiting fullscreen require long-pressing the Escape key when the fullscreen keyboard lock is enabled. r=edgar,dom-core,masayuki
https://github.com/mozilla-firefox/firefox/commit/123362df12dd
https://hg.mozilla.org/integration/autoland/rev/b219718bda25
Part 4: Make exiting PointerLock occur on long-press when fullscreen keyboard lock is enabled. r=dom-core,edgar

Comment 28

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 9: Add browser tests for fullscreen keyboard lock. r?#dom-core

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/d2726a670e5f
https://hg.mozilla.org/integration/autoland/rev/c9626c59d7cd
Revert "Bug 700123 - Part 4: Make exiting PointerLock occur on long-press when fullscreen keyboard lock is enabled. r=dom-core,edgar" for causing failures at test_fullscreen-api.html.

Comment 30

[Atila Butkovits]

Backed out for causing failures at test_fullscreen-api.html.

Backout link: https://hg.mozilla.org/integration/autoland/rev/c9626c59d7cd

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=b219718bda25ea2d78c9dc7769fea5cef1099fd1&selectedTaskRun=MpF9Vi1wSNeAFcVrdd5jzg.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=560236415&repo=autoland&task=MpF9Vi1wSNeAFcVrdd5jzg.0&lineNumber=2532

Comment 31

[Hsin-Yi Tsai (she/her)[:hsinyi]]

(Edgar has been looking into comment 30)

Comment 32

[Edgar Chen [:edgar]]

The failures is caused by part 1, since now the signature of requestFullscreen() is changed to have FullscreenOption argument, which results in requestFullscreen(123) rejecting the promise now. Other browsers also reject the promise in such case, since they all have FullscreenOption argument support.
There is a wpt test, https://wpt.fyi/results/fullscreen/api/element-request-fullscreen-options.html?label=experimental&label=master&aligned, but the test actually doesn't test the invalid argument properly, because the requestFullscreen() always rejects the promise due to the lack of user activation. I fixed the test and also extended it to cover additional invalid argument cases.
Will land the patches again soon.

Comment 33

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/e842d362efa9
https://hg.mozilla.org/integration/autoland/rev/74902ad0984e
Part 1: Add keyboardLock option to requestFullscreen. r=webidl,saschanaz
https://github.com/mozilla-firefox/firefox/commit/76fd69fd5907
https://hg.mozilla.org/integration/autoland/rev/29f0fd2be07b
Part 2: Track the fullscreen keyboard lock status and propagate it to the browser chrome. r=dom-core,edgar
https://github.com/mozilla-firefox/firefox/commit/99d5373dbdfe
https://hg.mozilla.org/integration/autoland/rev/51cc4c86a765
Part 3: Make exiting fullscreen require long-pressing the Escape key when the fullscreen keyboard lock is enabled. r=edgar,dom-core,masayuki
https://github.com/mozilla-firefox/firefox/commit/1e21f0953e58
https://hg.mozilla.org/integration/autoland/rev/abec9e91d0f0
Part 4: Make exiting PointerLock occur on long-press when fullscreen keyboard lock is enabled. r=dom-core,edgar

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/33383f0c5575
https://hg.mozilla.org/integration/autoland/rev/c62afd5bd221
Part 5: Display different UI message upon entering fullscreen with keyboard lock. r=dom-core,fluent-reviewers,bolsson,edgar

Comment 35

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/74902ad0984e
https://hg.mozilla.org/mozilla-central/rev/29f0fd2be07b
https://hg.mozilla.org/mozilla-central/rev/51cc4c86a765
https://hg.mozilla.org/mozilla-central/rev/abec9e91d0f0
https://hg.mozilla.org/mozilla-central/rev/c62afd5bd221

Comment 36

[Kagami Rosylight [:saschanaz] (they/them)]

Attachment: Bug 700123 - Part 10: Add WPT for escape from fullscreen keyboard lock r=#dom-core

Comment 37

[Adam Vandolder [:avandolder]]

Attachment: Bug 700123 - Part 6: Exiting browser fullscreen on Mac should follow fullscreen keyboard lock. r?edgar,#dom-core

Comment 38

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/59258 for changes under testing/web-platform/tests

Comment 39

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0d98866521bd
https://hg.mozilla.org/integration/autoland/rev/d103ed915331
Part 6: Exiting browser fullscreen on Mac should follow fullscreen keyboard lock. r=dom-core,smaug

Comment 40

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 41

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/f8e6ba4cd5ac
https://hg.mozilla.org/integration/autoland/rev/d446b107bbeb
Part 7: Show a warning when user presses Escape while fullscreen keyboard lock is enabled. r=dom-core,smaug

Comment 42

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8440f7113c9a
https://hg.mozilla.org/integration/autoland/rev/146439a0c56d
Part 8: Update fullscreen keyboard lock status when entering fullscreen-in-fullscreen. r=dom-core,edgar
https://github.com/mozilla-firefox/firefox/commit/36588a212f82
https://hg.mozilla.org/integration/autoland/rev/457f140384d2
Part 9: Add browser tests for fullscreen keyboard lock. r=dom-core,edgar
https://github.com/mozilla-firefox/firefox/commit/ca7ad571daa0
https://hg.mozilla.org/integration/autoland/rev/efb4ceea4730
Part 10: Add WPT for escape from fullscreen keyboard lock r=dom-core,edgar

Comment 43

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/59275 for changes under testing/web-platform/tests

Comment 44

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/d103ed915331
https://hg.mozilla.org/mozilla-central/rev/d446b107bbeb

Comment 45

[Edgar Chen [:edgar]]

(just realized some parts of patches aren't merged into central yet)

Comment 46

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/146439a0c56d
https://hg.mozilla.org/mozilla-central/rev/457f140384d2
https://hg.mozilla.org/mozilla-central/rev/efb4ceea4730

Comment 47

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged by moz-wptsync-bot

Comment 49

[Hamish Willee]

FF151 MDN docs work or this can be tracked in https://github.com/mdn/content/issues/43868

FYI, I had some questions that are mostly around the spec update in https://github.com/whatwg/fullscreen/pull/232#issuecomment-4286060467. If you have any insight, I'd welcome your answers.

For example the spec seems to indicate that some event actions are removed by the browser, such as the Esc key, while others can be explicitly removed using preventDefault(). It would be good to know what is automatically disabled (is it just Esc key?), and what can and cannot be disabled by preventDefault (at least on Firefox)

Comment 50

[Adam Vandolder [:avandolder]]

I believe the questions were addressed by Simon Pieters (:zcorpan) in https://github.com/whatwg/fullscreen/pull/232#issuecomment-4295296243. Let us know if you need more information than that.