Skip to content

Keep live view Caps Lock in sync on macOS - #443

Open
Jayko001 wants to merge 2 commits into
mainfrom
hypeship/fix-mac-caps-lock-live-view
Open

Jayko001 wants to merge 2 commits into
mainfrom
hypeship/fix-mac-caps-lock-live-view

Conversation

@Jayko001

@Jayko001 Jayko001 commented Oct 9, 2026 •

Copy link
Copy Markdown

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 keydown only when enabling it and a keyup only when disabling it. Because of the capsLockKeyupUnreliable quirk, the keyboard handler forwarded the keydown as a full press and release, then dropped the keyup because 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 Lock keydown or keyup is forwarded as one press and release through onkeydown/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 Lock keyupReliable branch 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:
    • macOS: Caps Lock on, type a, Caps Lock off forwards two full Caps Lock presses around the a
    • non-Mac: Caps Lock is still a single press/release
  • The new macOS test fails on main: it forwards one press/release and drops the disabling keyup.
  • npm run build succeeds. vue-cli-service lint --no-fix shows no new warnings. Prettier passes on the changed test file.
  • Not tested on a physical Mac in a live view session. Worth a manual check in Chrome, Safari and Firefox on macOS before merging: toggle Caps Lock on and off a few times while typing into a text field.

🤖 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 Lock keydown as having an unreliable keyup (which forced an immediate synthetic release and left later macOS keyup-only toggles ignored). On platforms with capsLockKeyupUnreliable, Caps Lock is handled up front in interpret_event: each queued Caps Lock event (0xFFE5) emits one press + release and is consumed from the log. Non-Mac behavior is unchanged.

Tests: Adds platform-mocked coverage that macOS forwards both enabling keydown and disabling keyup Caps 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.

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>
@Jayko001
Jayko001 marked this pull request as ready for review October 9, 2026 00:40
@Jayko001
Jayko001 requested a review from masnwilliams October 9, 2026 16:44

@masnwilliams masnwilliams left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

  1. 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). keyMap in video.vue already 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 main and 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 tests passes (24/24). the toggle still goes through onkeydown/onkeyup on the data channel, so it stays ordered with the keys typed after it, and it gets the existing hosting/locked/unmute checks for free. video.vue, store/remote.ts, guacamole-keyboard.ts and the video-input.test.ts block can all be reverted.

  2. the library's behavior shouldn't depend on whether a handler is set (guacamole-keyboard.js:1158). gating the fix on guac_keyboard.oncapslock turns registering a callback into a mode switch: without it, the library keeps the broken behavior. it also leaves the Caps Lock keyupReliable branch (:320-321) reachable only when no handler is set, and computes capsLock in the KeyEvent constructor (:228) for every key even though only Caps Lock reads it. (1) fixes the library for every consumer instead.

  3. second Caps Lock sync path in the component (video.vue:917-929, store/remote.ts:39-41, :63-67). onCapsLock re-implements the compare-cache, update, send pattern that syncKeyboardModifierState already owns, on a different transport. the keyboardModifierState bit layout is now spread across the helper, & 1 in the getter, and & ~1 in the mutation, and the -1 sentinel leaks out as a boolean | null getter plus a mutation that silently does nothing. onMouseLeave already 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 without setCapsLock.

  4. 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), onCapsLock toggles 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>
@Jayko001

Copy link
Copy Markdown
Author

@masnwilliams thanks, switched to your approach in 0c4663b.

  1. Applied your library diff as written. video.vue, store/remote.ts, guacamole-keyboard.ts and the video-input.test.ts block are reverted to main. The macOS library test now expects ['down:65509', 'up:65509', 'down:65', 'up:65', 'down:65509', 'up:65509'], and it still fails on main. bun test tests passes 24/24, and the build and lint are clean.
  2. and 3. Both resolved by the revert: there's no callback anymore, so no mode switch, no per-event capsLock, and no second sync path or bit twiddling in the store.
  3. I don't know of a macOS browser that sends both events per press. noVNC applies the same press-and-release handling to every browser on macOS and iOS with no per-browser exceptions, which suggests they haven't hit one either. I dropped that claim from the PR body. The manual Chrome/Safari/Firefox check on a Mac is still listed as a pre-merge step.

@Jayko001
Jayko001 requested a review from masnwilliams October 19, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants