Fix XKB group state and GetMap component filtering

[AprilGrimoire]

Summary

• clamp locked XKB groups to the active keymap before stamping key-event state or emitting XkbStateNotify
• honor XkbGetMap full/partial component masks instead of always advertising every implemented section
• keep CI-equivalent all-targets clippy clean and avoid repeated client token lookup in request dispatch

Tests

• cargo +nightly fmt --check
• cargo clippy --all-targets -- -D warnings
• cargo test --all-targets

[Copilot]

Pull request overview

This PR tightens yserver’s XKB behavior to better match real Xorg/client expectations: it clamps locked group state to the active keymap (preventing stale group bits from leaking into key-event/core modifier state) and makes XkbGetMap honor the client’s requested component masks instead of always advertising every implemented section. It also adjusts font-path alias resolution so stale aliases don’t block built-in fallback, while keeping the tree CI-clean under cargo clippy --all-targets -- -D warnings.

Changes:

• Clamp locked_group to the compiled keymap’s layout count when serializing modifier state and when handling group changes.
• Respect XkbGetMap full|partial masks by emitting only the requested map components (and test this behavior).
• Make stale font-path aliases fall through to later path elements / built-ins; remove redundant cfg(test) gating in the recording backend module.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
crates/yserver/src/kms/xkb.rs	Adds GetMap component-mask filtering and request-aware reply generation, plus targeted tests.
crates/yserver/src/kms/v2/backend.rs	Clamps locked group to keymap group count; routes GetMap through the request-aware reply; adds regression tests.
crates/yserver/src/kms/core.rs	Allows stale path aliases to fall through and treats XLFD-style names as resolvable via built-ins; adds tests.
crates/yserver-core/src/core_loop/run.rs	Refactors client-token match arm (introduces a small redundancy noted in review).
crates/yserver-core/src/core_loop/process_request.rs	Ensures LatchLockState group handling works correctly with clamping and avoids redundant StateNotify.
crates/yserver-core/src/backend/recording.rs	Removes redundant module-level #![cfg(test)] attribute (module is already #[cfg(test)] in backend/mod.rs).

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[joske]

@AprilGrimoire thanks for this PR. It seems to check out, didn't find any regressions in normal usage, now running the full XTS suite on it. Can you please sign the commit and fix the copilot comment please? 🙏

[AprilGrimoire]

@AprilGrimoire thanks for this PR. It seems to check out, didn't find any regressions in normal usage, now running the full XTS suite on it. Can you please sign the commit and fix the copilot comment please? 🙏

Sure. I'll test it on my physical machine. Some electron apps broke without this.

[joske]

Codex flagged this:

crates/yserver/src/kms/core.rs:375: the built-in fallback is now triggered for any XLFD-shaped name, not just names the built-ins can actually satisfy.
Combined with crates/yserver/src/kms/core.rs:596, which always feeds the parsed family into fontconfig and appends "monospace" as fallback, an invalid
XLFD can now resolve to some fallback font instead of returning BadName. That is a protocol-visible regression risk for X font clients that use bogus or
probing XLFDs and expect failure. The stale-alias fix looks right; the is_xlfd_pattern(name) shortcut is the part that seems too broad.

[joske]

Confirmed that this branch fixes SIGTRAP on balena etcher

[AprilGrimoire]

Codex flagged this:

crates/yserver/src/kms/core.rs:375: the built-in fallback is now triggered for any XLFD-shaped name, not just names the built-ins can actually satisfy. Combined with crates/yserver/src/kms/core.rs:596, which always feeds the parsed family into fontconfig and appends "monospace" as fallback, an invalid XLFD can now resolve to some fallback font instead of returning BadName. That is a protocol-visible regression risk for X font clients that use bogus or probing XLFDs and expect failure. The stale-alias fix looks right; the is_xlfd_pattern(name) shortcut is the part that seems too broad.

Oh, sorry. I know of this issue, but I thought it was in another unsubmitted pull request. I'm working on it.

[joske]

@AprilGrimoire any progress? Would be good to get this in.

[AprilGrimoire]

@AprilGrimoire any progress? Would be good to get this in.

Sorry, since I was developing on yserver for yserver, I got in trouble with an OpenGL bug. Hopefully I will fix this by at most tomorrow.

[joske]

That's proper dogfooding!

[joske]

Note that master is not sitting still, so you may have some conflicts on rebase (but my work didn't really touch xkb I think). Also please remember to sign the commits. You can do that after the fact.

[AprilGrimoire]

Hello. I noticed that some testcase cannot be cleanly satisfied, at least on my machine. The built-in font 'fixed' is aliased to '-misc-fixed-medium-r-normal--18-180-75-75-c-100-iso8859-1', which doesn't exist on my machine, and I believe that's what made my codex agent to make the dubious change. Why was that alias introduced though? Currently it seems that yserver doesn't come with fonts, and requiring so seems to rely on package managers, which seems to not be the focus of this stage of development. How do you use AI for development? Will it make sense to package some built-in fonts as a binary blob?

[joske]

I think you may be missing the xorg-mkfontscale package.

[joske]

Looking more closely, I think you should install the said xorg-mkfontscale package and the xorg-fonts-misc package (or equivalent for your distro) and then revert/drop a0595fb (not related to XKB).

[AprilGrimoire]

I installed xorg-fonts on Gentoo, however tests still don't pass on my machine. Now I'm missing -misc-fixed-medium-r-normal--13-130-75-75-c-60-iso8859-1. If it is advertised by yserver, is it a good idea to bundle them as a binary blob?

Looking more closely, I think you should install the said xorg-mkfontscale package and the xorg-fonts-misc package (or equivalent for your distro) and then revert/drop a0595fb (not related to XKB).

[joske]

No I don't think we should bundle a font blob.

The commits are signed-off, but the workflow wants the commits signed (with SSH key or GPG). This is the remaining blocker. You can use this

# one-time: register an SSH *signing* key on GitHub (Settings → SSH and GPG keys → New → type "Signing Key")
git config gpg.format ssh
git config user.signingkey ~/.ssh/id_ed25519.pub
git config commit.gpgsign true
# re-sign the existing commits (rewrites them):
git rebase --exec 'git commit --amend --no-edit -S' <base>   # <base> = the master commit they branched from
git push --force-with-lease