Repository navigation
Conversation
macOS browsers send Caps Lock keydown only when enabling it and keyup only when disabling it. The keyboard handler forwarded the keydown as a full press and release, then dropped the keyup because the key was no longer pressed, so every other toggle never reached the remote and its Caps Lock state inverted. On macOS, report Caps Lock events as the resulting lock state instead. Live view toggles the remote with a keystroke only when that state differs from the remote's last known state, and records the change so mouse-entry modifier sync stays accurate. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
masnwilliams
left a comment
There was a problem hiding this comment.
requesting changes on the approach. the diagnosis is right and the fix works, but there's a much smaller fix that removes the new callback, the store changes, and the component handler entirely.
-
report each macOS Caps Lock event as one press and release in the library. on mac, a Caps Lock keydown or keyup each mean one toggle, so the library can forward each one as a press and release, the same thing noVNC does (
core/input/keyboard.js,_handleKeyDown/_handleKeyUp).keyMapinvideo.vuealready follows noVNC for mac key handling.--- a/images/chromium-headful/client/src/utils/guacamole-keyboard.js +++ b/images/chromium-headful/client/src/utils/guacamole-keyboard.js @@ -298,7 +298,4 @@ var KeydownEvent = function KeydownEvent(orig) { if (this.modifiers.meta && this.keysym !== 0xFFE7 && this.keysym !== 0xFFE8) this.keyupReliable = false; - // We cannot rely on receiving keyup for Caps Lock on certain platforms - else if (this.keysym === 0xFFE5 && quirks.capsLockKeyupUnreliable) - this.keyupReliable = false; - @@ -1135,6 +1132,14 @@ var interpret_event = function interpret_event() { if (!first) return null; + // macOS sends Caps Lock keydown only when enabling it and keyup only + // when disabling it, so each event is one full press of the key + if (first.keysym === 0xFFE5 && quirks.capsLockKeyupUnreliable) { + first.defaultPrevented = !guac_keyboard.press(0xFFE5); + guac_keyboard.release(0xFFE5); + return eventLog.shift(); + } + // Keydown event if (first instanceof KeydownEvent) {
i applied this to
mainand kept both new library tests from this PR. the only change is the macOS expectation, which becomes['down:65509', 'up:65509', 'down:65', 'up:65', 'down:65509', 'up:65509'].bun test testspasses (24/24). the toggle still goes throughonkeydown/onkeyupon the data channel, so it stays ordered with the keys typed after it, and it gets the existinghosting/locked/unmute checks for free.video.vue,store/remote.ts,guacamole-keyboard.tsand thevideo-input.test.tsblock can all be reverted. -
the library's behavior shouldn't depend on whether a handler is set (
guacamole-keyboard.js:1158). gating the fix onguac_keyboard.oncapslockturns registering a callback into a mode switch: without it, the library keeps the broken behavior. it also leaves the Caps LockkeyupReliablebranch (:320-321) reachable only when no handler is set, and computescapsLockin theKeyEventconstructor (:228) for every key even though only Caps Lock reads it. (1) fixes the library for every consumer instead. -
second Caps Lock sync path in the component (
video.vue:917-929,store/remote.ts:39-41,:63-67).onCapsLockre-implements the compare-cache, update, send pattern thatsyncKeyboardModifierStatealready owns, on a different transport. thekeyboardModifierStatebit layout is now spread across the helper,& 1in the getter, and& ~1in the mutation, and the-1sentinel leaks out as aboolean | nullgetter plus a mutation that silently does nothing.onMouseLeavealready rewrites the cache from local modifier state on the way out, so with keystrokes mirroring each toggle the cache is correct at the next mouse entry withoutsetCapsLock. -
question on the "browser sends both keydown and keyup" case. the PR body says comparing against state holds up if a macOS browser sends both events per press. that only holds once the remote state is known. while it's
-1(before the first mouse entry, or right after leaving read-only),onCapsLocktoggles on every event, so that browser would cancel each press out. do we know of a macOS browser that does this? if so, that's worth calling out here, and it would argue for one store-owned model rather than the hybrid. if not, (1) covers the browsers we know about.
agree a manual check on a mac in chrome, safari and firefox is worth doing before merge, with either version.
Replace the oncapslock callback and the component/store state sync with the noVNC approach: on macOS each Caps Lock keydown or keyup is one toggle, so the library forwards each as a press and release through onkeydown/onkeyup. This fixes the library for every consumer and keeps the toggle on the existing hosting/locked/unmute path. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@masnwilliams thanks, switched to your approach in 0c4663b.
|
Summary
On macOS, toggling Caps Lock inside live view could leave the remote browser's Caps Lock inverted, so text (including masked password fields) was entered in the wrong case.
macOS browsers send a Caps Lock
keydownonly when enabling it and akeyuponly when disabling it. Because of thecapsLockKeyupUnreliablequirk, the keyboard handler forwarded thekeydownas a full press and release, then dropped thekeyupbecause the key was no longer marked pressed. Only the "on" toggles reached the remote, so after one on→off cycle the remote stayed in caps mode, and a second cycle brought it back in sync. That made the bug look intermittent.Changes
guacamole-keyboard.js: on macOS (capsLockKeyupUnreliable), each Caps Lockkeydownorkeyupis forwarded as one press and release throughonkeydown/onkeyup. This matches noVNC (core/input/keyboard.js,_handleKeyDown/_handleKeyUp), which does the same for every browser on macOS and iOS. The now-unreachable Caps LockkeyupReliablebranch is removed. Other platforms are unchanged.The toggle goes over the data channel like any other key, so it stays ordered with the keys typed after it and goes through the existing
hosting/locked/unmute checks.Testing
bun test tests: 24 pass. New tests cover:a, Caps Lock off forwards two full Caps Lock presses around theamain: it forwards one press/release and drops the disablingkeyup.npm run buildsucceeds.vue-cli-service lint --no-fixshows no new warnings. Prettier passes on the changed test file.🤖 Generated with Claude Code
Note
Medium Risk
Changes remote keyboard event synthesis for Caps Lock on macOS; incorrect handling could still desync lock state or affect password/text entry in live view.
Overview
Fixes macOS Caps Lock forwarding in the Guacamole keyboard layer so each browser toggle reaches the remote as a complete press/release instead of only half the cycle.
guacamole-keyboard.js: Stops marking Caps Lockkeydownas having an unreliablekeyup(which forced an immediate synthetic release and left later macOSkeyup-only toggles ignored). On platforms withcapsLockKeyupUnreliable, Caps Lock is handled up front ininterpret_event: each queued Caps Lock event (0xFFE5) emits onepress+releaseand is consumed from the log. Non-Mac behavior is unchanged.Tests: Adds platform-mocked coverage that macOS forwards both enabling
keydownand disablingkeyupCaps Lock events (plus a normal key in between), while Windows still uses a single keydown/keyup pair.Reviewed by Cursor Bugbot for commit 0c4663b. Bugbot is set up for automated code reviews on this repo. Configure here.