Experimental: Add XKB extension for locks

[wismill]

Following the discussion in #432, I have done a little experiment that implements two new options:

• lockOnRelease for LockGroup action: intended to fix Need to kick hotkeys on release, not press.
• unlockOnPress for LockMods action: intended to fix Option to apparently unlock the Lock modifier on press of Caps Lock.

This is backward-incompatible as it introduces a new syntax for the keymap.

I see at least 4 points to discuss here:

1. Is the behavior of the new options correct?
2. How do we make this change more compatible?
   I think there are 2 aspects to consider:
   1. Compatibility with older versions of xkbcommon. We could propose an additional syntax using the Private action, useful for exchange/ouput. Older xkbcommon would just ignore this action and thus we avoid a syntax error.
   2. Compatibility with X11. This feature could be implemented as well in the Xorg universe. Anyway, we should probably use Private as presented in the previous section, in order to maximize compatibility.
3. Should these options extended to their respective actions? I would say yes for symmetry, but we could also simply wait to see it there is a need.
4. Does it need a new version of the corresponding Wayland protocol? While I would like to have version negociation, I also hope that the spec is lax enough to avoid to have to wait for this.

I think that if we answer 1) and 2), then we could publish it as a tech preview, with a big warning. That could get us feedback before engaging with protocol updates.

@whot @bluetech @fooishbar

[whot]

Is the behavior of the new options correct?

The code LGTM, but tests would answer that question :)

How do we make this change more compatible?

Adding XKB_KEYMAP_FORMAT_TEXT_V2 solves the case of xkb_keymap_as_string().

And we have a pretty clear split between compositor-side xkb_keymap_new_from_names and client-side xkb_keymap_new_from_string(). As long as xkb_keymap_as_string() spits out the right version, we shouldn't have much of an issue in actual deployments because we can mostly assume that compositors generate from the keymap files (and thus will handle v2 if they want) and clients receive from the compositor and thus will get v1 (old compositors) or v2 (new compositors + client saying they want it).

I don't think we need to worry about X11, that ship has sailed and is listing.

So what we need is the Wayland negotiation which I at minimum needs a wl_keyboard (and wl_seat/pointer/touch) version bump, a wl_keyboard.done event and then the documentation that if the client has version $whatever the compositor may send multiple keymaps for the client to pick-and-choose:

• wl_seat.get_keyboard - unchanged
• wl_keyboard.keymap(xkb_v1) - unchanged
• wl_keyboard.keymap(xkb_v2) <- new in $version
• wl_keyboard.done <- new in $version

Not sure whether we need a wl_keyboard.use_keymap(xkb_v2) request to confirm to the compositor which keymap we ended up using. The problem is that every time we bump the XKB version we need to bump the wayland protocol too.

A potentially better approach is to add a subversion so we use xkb_v1 as identifier for our general format ("xkb_keymap {...}") and use numeric versions within:

• wl_seat.get_keyboard - unchanged
• wl_keyboard.keymap(xkb_v1) - unchanged or not sent at all if client >= $version
• wl_keyboard.keymap_version(xkb_v1, 1) <- optional, same as wl_keyboard.keymap(xkb_v1)
• wl_keyboard.keymap_version(xkb_v1, 2) <- new in $version
• wl_keyboard.done <- new in $version

This way we can introduce future XKB versions like this without having to bump the wayland protocol for each of them. Note that doing so decouples XKB_KEYMAP_FORMAT_TEXT_V2 from Wayland's xkb_v1 which needs to be documented to avoid confusion.

We could do a subversion here too like xkb_keymap_as_string_with_version(XKB_KEYMAP_FORMAT_TEXT_V1, 2) but not sure it provides the same benefits.

Should these options extended to their respective actions? I would say yes for symmetry, but we could also simply wait to see it there is a need.

Can you rephrase/expand on this, I'm not sure what you mean here.

Does it need a new version of the corresponding Wayland protocol?

Yes, we can only send keymaps to the client that are compatible v1.

[wismill]

Should these options extended to their respective actions? I would say yes for symmetry, but we could also simply wait to see it there is a need.

Can you rephrase/expand on this, I'm not sure what you mean here.

@whot I meant to add also lockOnRelease to LockMods and unlockOnPress to LockGroup. However, I do not think it is sound for the latter. And I am not sure there is a use case for the former… So let’s forget about symmetry here.

[bluetech]

Just a quick question, what actually happens on old xkbcommon and xkbcomp if you set these options? Do they count towards the fatal error count, or just issue a log and ignored?

[bluetech]

Regarding

Is the behavior of the new options correct?

Do you think you can document the new flags in keymap-format-text-v1.md? (Mostly so I can remind myself about these things 😀 but also because it seems better to discuss a "spec" change before a code change).

(Also, link to a previous Xorg proposal https://lists.x.org/archives/xorg-devel/2012-November/034427.html by Andreas Wettstein)

[wismill]

@bluetech

Just a quick question, what actually happens on old xkbcommon and xkbcomp if you set these options? Do they count towards the fatal error count, or just issue a log and ignored?

xkbcommon 1.6 (xkbcli compile-keymap)

xkbcommon: ERROR: Unknown field name unlockOnPress
xkbcommon: ERROR: Failed to compile xkb_compatibility
xkbcommon: ERROR: Failed to compile keymap
Couldn't create xkb keymap

xkbcomp 1.4.7:

Warning:          Unsupported maximum keycode 569, clipping.
                  X11 cannot support keycodes above 255.
Keycodes above 372 (e.g. <I372>) are not supported by X and are ignored
Error:            Unknown field name unlockOnPress

So error in both cases.

[wismill]

Do you think you can document the new flags in keymap-format-text-v1.md? (Mostly so I can remind myself about these things 😀 but also because it seems better to discuss a "spec" change before a code change).

@bluetech I tried to adapt the fix of Andreas (thanks for the link) to xkbcommon. I agree we should ideally discuss spec before the code, but since I have limited time and we are not going to merge this fix before handling the compatibility question, I though I would leave the nice doc for now.

• lockOnRelease locks group on release rather than press as in the protocol. Intended to fix issue for group lock on e.g. shift+alt.
• unlockOnPress unlocks modifier on second press rather than the second release, as in the protocol. Intended to fix issue with CapsLock for fast typists.

[wismill]

Reworked at #485