garykac / system-keyboard-lock Goto Github PK

The isFullscreen check should check if a specific document's fullscreen element is non-null, instead of the current vague phrasing + implementation suggestion.

I'm not sure what "input focus" is, but if it's the same thing as normal focus, then it should check if a specific document's fullscreen element is the currently focused area of a top-level browsing context

In general this algorithm should probably either get moved into or replace https://html.spec.whatwg.org/#fire-a-focus-event. It seems like they are both trying to do the same thing. The HTML version has some more precise wording, but is of course missing the fullscreen and system keyboard lock stuff. I'd suggest copying as much from HTML as looks correct then adding your system keyboard lock stuff as you've done, with the tweaks above. We can eventually either move it back into HTML or have HTML link to this.

Will anyone want actually mapped keys? Not specifically games or chromoting (where the keyboard layout is irrevlant to the webpage). But say someone wants to provide their own Ctrl-P (printing functionality). How do they know that on dvorak keyboard they need to actually register the 'R' in this API to get that functionality? But on a qwerty keyboard they use 'P'.

The fact that the page doesn't know the current keyboard layout and when the keyboard layout changes can be difficult to implement that functionality on top of this API. Perhaps these aren't cases we need to solve.

@smaug---- Probably someone from Gecko might have some feedback on this proposal.

Once a page is allowed to request keys, the requested keys should not be intercepted by the browser. i.e. Browser should always forward the reserved keys to the webpage directly.
For example, if the page requested "W" key, ctrl + W should not close the page.

They're linked from the top of https://garykac.github.io/system-keyboard-lock/

Relatedly, please pick a single "shortname" and use it everywhere. In https://codereview.chromium.org/2805763004 tests are being added under keyboard-lock/ in web-platform-tests. Bikeshed: I prefer that to the longer system-keyboard-lock.

Right now it links to an IDL member, but ideally there should be a definition like "a KeyboardEvent code is one of the following strings" in some spec.

This should cover the #18, as well as the implementation specific limitations. A must-have is a field to indicate whether the error could recover after a retry, or the request is rejected by user agents because of a permission request.

When reading:
https://garykac.github.io/system-keyboard-lock/#key-press-handler

It seems that the "system key press handler" is describing an implementation. An alternate implementation is that you could just have a table and know that the registered document/navigator has priority over executing the UA default action for these events.

It isn't clear to me that you need to hook the system listeners in order to implement this.

If the web page has not actively called cancelSystemKeyboardLock(), whenever the unloading document cleanup steps run with a document, cancelSystemKeyboardLock() is implicitly called. i.e. The keyboard lock API is session-based, and won't be kept by the user agents.

Considering the API is for all keys instead of OS reserved key combinations, I would suggest to change,
Navigator.requestSystemKeyboardLock()
into
Navigator.requestKeyLock()

And similar as
Navigator.cancelSystemKeyboardLock().

This term should probably be defined somewhere separate from the algorithm that registers it. Then it can get the proper treatment it deserves in terms of what normative effects it has, the interesting notes nearby, and so on.

It should explain what normative effect it has in as much detail as possible, although understandably this is kind of difficult. Currently it says "process keyboard events". Maybe expand on this a bit, i.e. something like "ensure that KeyboardEvent events are fired instead of performing the usual action of the system keyboard shortcut".

Also, it would be best to not use "event handler" since that has a specific meaning. Maybe "keyboard trap" or similar would be better.

Also, consistently use low-level or low level. I think with a hyphen is a bit better?

• Missing closing parens in "(which would capture all keys if enableKeyboardLock was enabled."
• Is reservedKeyCodes a sequence or list? Either seems fine, but pick one and stick with it.

One big issue to figure it is where the enableKeyboardLock and reservedKeyCodes are meant to be stored. They're certainly not stored in "the user agent", as that would mean that they are the same across all web sites. I see a few choices:

• The browsing context (~ a "tab" in your browser). This seems unlikely, as it would mean they persist across navigation within that browsing context (even to cross-origin sites).
• Every Window/environment settings object (they are 1:1). This would mean that if you inject code into about:blank and then navigate, the keyboard lock would be preserved (the Window is reused so all its values remain). But if you do document.open(), the keyboard lock would be lost (the Window is re-created).
• Every Document that has a browsing context. (The latter clause is important so that document.implement.createDocument() or XHR responseDocument do not get this ability.) This would mean that navigating from about:blank to a new document loses keyboard lock, but doing document.open() does not.

So probably either Window or Document. I don't see a real reason to prefer one over the other; maybe Document is most conventional.

Once you've figured that out, it's important to say how to get to that variable inside your algorithms. So, let's say it was Window. Then step 1 of requestSystemKeyboardLock would be:

1. Reset this Navigator object's relevant global object's reservedKeyCodes to be an empty sequence.

If it was the Document, it would be

1. Reset this Navigator object's relevant global object's associated Document's reservedKeyCodes to be an empty sequence.

Similarly for references to enableKeyboardLock and so on.

If we would prefer to return Promise from requestSystemKeyboardLock() function, once a web page requests twice in parallel, the second one should be rejected immediately. E.g.

function f() {
  const p1 = navigator.requestSystemKeyboardLock(['a', 'b']);
  const p2 = navigator.requestSystemKeyboardLock(['c', 'd']);

  p2.then(() => {
    assert_not_reachable();
  }).catch((e) => {
    // e should be a predefined DOMException.
  });
}

This seems a little odd given what is currently on the Navigator object (there aren't any functions and most of values are only readonly). Not sure what the motivation to add it to the Navigator object is.

@foolip Do you have any opinion?

Given that implementation is starting in https://codereview.chromium.org/2805763004 and the rough shape of the spec is done, this would be a good time to request TAG review at https://github.com/w3ctag/spec-reviews/issues

[SecureContext] void requestSystemKeyboardLock(optional sequence<DOMString> keyCodes);

should be

[SecureContext] void requestSystemKeyboardLock(optional sequence<DOMString> keyCodes = []);

https://garykac.github.io/system-keyboard-lock/#enable-keyboard-lock is a little confusing. Are you defining a property on the Navigator IDL? If so why isn't it there? Is it read only, what are the semantics of it?

Considering this is an extremely powerful API, which allows webpage to intercept any keys. Some browser vendors may implement a user notification or prompt to decide whether these APIs should be allowed or now. Meanwhile, web page may need a signal to indicate when the reserved keys are available, or whether the user declined the request. So it may fall back to use alternative keys.
So I suggest to change
void Navigator.requestSystemKeyboardLock()
into
Promise<void> Navigator.requestSystemKeyboardLock()

Once the Promise resolves, the web page should be able to retrieve the keys it requested.
The Promise will reject only if the user actively disallowed the API.
Due to the "best effort" principle of these APIs, no matter whether the keys or key combinations can be successfully retrieved, e.g. ctrl+alt+del is never cancelable on Windows, the Promise always resolves, once users or browser allowed the API.

void Navigator.cancelSystemKeyboardLock()
is not impacted by this change.

There should be an event fired when keyboard lock is exited. It should be possible for the User Agent to decide to kick the user out of keyboard lock at any time, and the page should be notified.

See #9 (should kick out when exiting full-screen).

The spec currently proposes that requestSystemKeyboardLock always succeed (in fact it does not define any possibility of failure; see #7). If not in full-screen, it is supposed to succeed but do nothing until fullscreen is entered.

Rather, it should reject if called when not in full-screen. When the enable keyboard lock flag is set, keyboard lock should work (and there should be no clause in "Handling Keyboard Events" that checks if full-screen).

Similarly, enable keyboard lock should be cleared when exiting full-screen.

@dtapuska mentioned this good question.
Once a web page requests some keys, according to #9, these keys should not be processed by browser. The behavior may introduce the out of order key events. E.g.

• Web page requests "W" key.
• User types "Q" and "W" in sequence.
• "Q" is consumed or delayed by browser processing logic, maybe IME.
• Web page receives "W" then "Q", or "W" only.

I do not think this could be a big deal. If the web page needs both "Q" and "W", it should request both keys. Otherwise, relying on "Q" is not a reasonable behavior. But we may explicitly call out this issue in the spec.