Skip to content

fix(input): harden focus-steal lifecycle and lock UX#2

Merged
hiking90 merged 2 commits into
mainfrom
fix/post-review-hardening
May 15, 2026
Merged

fix(input): harden focus-steal lifecycle and lock UX#2
hiking90 merged 2 commits into
mainfrom
fix/post-review-hardening

Conversation

@hiking90
Copy link
Copy Markdown
Owner

Summary

Post-review hardening of OngeulInputController and main.swift. Four independent issues uncovered during a full Swift+Rust review, each verified against the actual code paths.

  • focus-steal replay race — bare DispatchQueue.main.async with no cancel handle. If deactivateServer fired between enqueue and dispatch, the closure resolved self.client() against the next active app and leaked buffered Korean keys into the wrong window. Now wrapped in a stored DispatchWorkItem, cancelled from activate/deactivate, with an enqueue-time currentBundleId snapshot used as a sanity check at dispatch time.
  • silent Lock rejectionsetValue:forTag: rejected external mode changes while English Lock was active without any visible feedback. LockOverlay is now re-shown so users see why the menu-bar switch didn't take.
  • per-keystroke UserDefaults lookupisCurrentAppLocked() is called from the CGEventTap callback on every key event. It walked through coordinator.isLocked → EnglishLockStore → UserDefaults.dictionary(forKey:) every time. Now cached on the controller, refreshed on activateApp and after each toggleLock.
  • main.swift force-unwrapsinfoDictionary!["..."] as! String and bundleIdentifier! would SIGABRT silently on a malformed bundle. Replaced with guard let … else { fatalError(...) } so the missing key is visible in the crash log.

Build + all 78 unit tests pass locally.

Test plan

  • Korean composing → cmd-tab to another app quickly → no Korean keys appear in the wrong window
  • Activate English Lock → click menu-bar input source picker to switch to Korean → menu shows English, LockOverlay flashes the locked icon
  • Hold a key and observe no perceptible cost from isCurrentAppLocked under autorepeat (verifiable via profiler if desired)
  • Build with broken Info.plist (rename InputMethodConnectionName) → crash report contains the missing-key name instead of an opaque SIGABRT

🤖 Generated with Claude Code

hiking90 and others added 2 commits May 14, 2026 21:04
Post-review fixes verified against the codebase. Four independent issues:

- focus-steal replay was a bare DispatchQueue.main.async with no cancel
  handle; if deactivateServer fired between enqueue and dispatch, the
  closure would resolve self.client() against the *next* active app and
  leak buffered Korean keys into the wrong window. Wrap it in a stored
  DispatchWorkItem, cancel from activate/deactivate, and bail if
  currentBundleId no longer matches the enqueue-time target.
- setValue:forTag: silently rejected external mode changes while
  English Lock was active, leaving users wondering why the menu bar
  switch did nothing. Show LockOverlay on rejection so the lock state
  is visible.
- isCurrentAppLocked() ran a UserDefaults.dictionary(forKey:) lookup
  per key event from the CGEventTap callback. Cache the lock state on
  the controller; refresh on activateApp and after each toggleLock.
- main.swift force-unwrapped infoDictionary["InputMethodConnectionName"]
  and bundleIdentifier; a malformed bundle would SIGABRT silently.
  Replace with guard let + fatalError carrying the missing-key name.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
forceKoreanForReplay was calling perAppStore.saveMode(.korean, ...) as a
side effect of transient focus-steal correction. This meant a single
focus-steal event in an app that the user normally uses in English would
flip that app's stored default to Korean, surprising the user on the next
activation.

Per-app mode is now only updated by paths that reflect explicit user
intent: mode toggle, external menu-bar change, ESC-to-English, and
deactivate-time save. Focus-steal correction stays purely transient.

The bundleId parameter on forceKoreanForReplay was only used for the
removed save call, so it's gone too.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.

1 participant