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.
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 platforms with thecapsLockKeyupUnreliablequirk (macOS), Caps Lock events now fire a newoncapslock(capsLock)callback with the resulting lock state fromgetModifierState('CapsLock'). They no longer go through press/release. Other platforms are unchanged. Without anoncapslockhandler, the library keeps its old behavior.video.vue:onCapsLocksends a Caps Lock keystroke over the data channel only when the local state differs from the remote's last known state. It uses a keystroke instead of thecontrol/keyboardWebSocket message so the toggle stays ordered with the keys typed right after it.store/remote.ts: adds acapsLockgetter and asetCapsLockmutation on the existingkeyboardModifierStatecache, so the mouse-entry modifier sync stays accurate after keyboard toggles.Comparing against state, instead of treating every event as a toggle, also holds up if a macOS browser sends both
keydownandkeyupfor each press. When the remote state is unknown (-1, before the first mouse-entry sync or right after leaving read-only while hovered), each event toggles, which matches how Chrome on macOS reports Caps Lock.Testing
bun test tests: 27 pass. New tests cover:true/falsewith no press/release, alongside a normal keymain: it forwards one press/release and drops the disablingkeyup.npm run buildsucceeds.vue-cli-service lint --no-fixshows no new warnings (20 before and after). Prettier passes on the changed TS files.🤖 Generated with Claude Code
Note
Medium Risk
Changes remote keyboard input synchronization for Caps Lock on macOS; incorrect handling could still mis-type into remote sessions, but scope is limited to the live-view keyboard path with new tests.
Overview
Fixes macOS live view Caps Lock drift by treating Caps Lock as a lock-state change instead of a normal key press/release on platforms where
keyupfor Caps Lock is unreliable.Guacamole keyboard adds an
oncapslock(capsLock)hook (typed inguacamole-keyboard.ts). On macOS, Caps Lockkeydown/keyupnow invoke that callback withgetModifierState('CapsLock')and skip synthetic press/release; other platforms still emit standard Caps Lock key events.video.vuewiresonCapsLockto send a Caps Lock keystroke over the WebRTC data channel only when the local lock state differs from the cached remote state (and input isn’t locked), so toggles stay ordered with following typed keys.store/remote.tsadds acapsLockgetter andsetCapsLockmutation on the existingkeyboardModifierStatebitmask so mouse-entry modifier sync stays accurate after keyboard toggles.New unit tests cover macOS vs non-Mac Caps Lock in the keyboard helper and component sync behavior (mismatch-only toggles, unknown state, locked).
Reviewed by Cursor Bugbot for commit 61f0b3b. Bugbot is set up for automated code reviews on this repo. Configure here.