Skip to content

fix(hotkey): resolve modifier-mask mismatch causing bare-key global hotkeys - #450

Merged
julienmerconsulting merged 5 commits into
oculix-org:chore/global-bug-fixesfrom
davidyoung8196504567-sudo:fix/hotkey-modifier-mask-bug
Aug 27, 2026
Merged

fix(hotkey): resolve modifier-mask mismatch causing bare-key global hotkeys#450
julienmerconsulting merged 5 commits into
oculix-org:chore/global-bug-fixesfrom
davidyoung8196504567-sudo:fix/hotkey-modifier-mask-bug

Conversation

@davidyoung8196504567-sudo

@davidyoung8196504567-sudo davidyoung8196504567-sudo commented Jul 31, 2026

Copy link
Copy Markdown

Summary

Fixes #449 — the bare C key was being registered as a global system hotkey, swallowing the letter C in every application while the IDE ran.

Rebased since first review: the unrelated key == 0 change has been lifted out (see below), so this PR is now scoped to the #449 fix and its tests — 4 files, 2 production classes.

Root cause

Not a keymaster/Carbon defect (initial hypothesis, ruled out). KeyModifier.getModifierNames() recognized only the legacy InputEvent.*_MASK bit values, while the stored STOP_HOTKEY_MODIFIERS uses the extended *_DOWN_MASK convention (320 = SHIFT_DOWN_MASK + META_DOWN_MASK). No bits matched, so the modifier string resolved to "", HotkeyController composed KeyStroke.getKeyStroke("C") instead of "shift meta C", and keymaster faithfully registered exactly what it was asked for.

Full trace, plus @julienmerconsulting's archaeology on where 320 comes from (a SikuliX 2.0.x → OculiX preferences-migration hazard, latent until b159a252/#388 correctly made the getter read the key the setter writes), is in #449.

Changes

  • KeyModifier.getModifierNames() — recognize both the legacy *_MASK and extended *_DOWN_MASK bit pattern for each modifier. The two ranges are disjoint (1/2/4/8/32 vs 64/128/256/512/8192), so this cannot produce a false positive.
  • HotkeyController.addHotkey(HotkeyListener, int, int) — refuse to register when a non-zero modifier resolves to no known modifier name, instead of silently degrading to an unmodified global hotkey. This is the more durable half: it prevents this class of bug regardless of what future convention or data issue appears.

Tests

  • KeyModifierTest — broadened 4 → 9 cases per review: legacy Shift+Meta (5), extended Shift+Meta (320, the observed value), extended Shift+Alt (576, the Windows-side equivalent), extended Ctrl, extended AltGraph, a mixed legacy/extended value, and a non-modifier bit (BUTTON1_DOWN_MASK) that must still resolve to nothing.
  • HotkeyControllerTest (new) — direct coverage of the refusal safety net, which previously had no test of its own.

Review feedback

# Ask Status
1 Refresh the issue write-up Done — #449 now carries the full origin timeline
2 Broaden regression tests Done — 9 modifier cases + the safety net now tested directly
3 Isolate or justify key == 0 Isolated — dropped from this PR

On key == 0

Lifted out per your item 3 rather than justified, because the rationale I would have given does not hold up on macOS.

Java's MacOSXPreferences reads and writes ~/Library/Preferences/org.sikuli.script.plist through cfprefsd, which caches and owns that file. Direct edits (plutil, Python plistlib) never reach the JVM — verified: the JVM reports STOP_HOTKEY = 67 after the file was set to 0, and the file has since been rewritten back to 67 by the daemon. So there is no route a user can actually take to set a key code to 0 on macOS today, given the Preferences UI exposes a binding control for Capture but not Stop.

The escape hatch needs a supported path — UI, defaults write, or something else — before it is worth proposing. Happy to open it separately if you think it is wanted.

Test plan

  • mvn test -pl API — full suite green, 50 tests, 0 failures, 0 errors
  • Manual: oculixide-4.0.0-complete-mac.jar on macOS 26.6 / Apple M4 Pro — bare C types normally in other applications while the IDE runs; Abort and Capture hotkeys still fire on their real combinations
  • Manual: oculixide-4.0.0-complete-lux.jar on Ubuntu 24.04 (x86-64, X11 session) — Abort (Shift+Alt+C) and Capture (Shift+Ctrl+2) both fire correctly, no bare-key capture. Linux routes through LinuxHotkeyManager/jxgrabkey rather than keymaster, so this exercises getModifierNames() on a completely separate registration path and confirms the change is not macOS-shaped. Wayland untested — global grabs there are a separate question, unaffected by this PR.
  • CI (compile-API/IDE workflows)

julienmerconsulting and others added 3 commits July 7, 2026 10:24
Round 1 + 2 + 3 of the macOS Apple Silicon emoji/tofu fix reported by
@genequ. Replaces all emoji glyphs with SVG icons (17 menu + 5 sidebar
assets), strips box-drawing chars (U+2500, U+2502), swaps the gecko
dialog icon for the real gecko_cyclope.png artwork, and drops the
'-- section label --' decorations.

User validated Round 3 and closed the issue himself.
Co-authored-by: Claude (Anthropic) <noreply@anthropic.com>
Add .github/CREDITS.md to make three native-language reviewers and nine bug reporters visible in the GitHub Contributors graph. Their work landed in OculiX between the March 2026 reprise and the v4.0.0 release, but they never appeared in the graph because their contribution was routed through @-mentions inside commit bodies rather than Co-authored-by trailers — GitHub only credits authors and Co-authored-by trailers.

Three native-language reviewers validated the locales that shipped in the v3.0.4 i18n campaign (oculix-org#277): @daniele-paltrinieri-79 for Italian across three batches (210+ keys, 18 false-friends), @tcc for Traditional Chinese, and @peixuana for Simplified Chinese.

Nine bug reporters whose diagnosis shaped a fix that shipped in a release: @genequ (oculix-org#432 emoji/tofu macOS), @andresluuk (oculix-org#416 Unicode PUA), @Zdenda3D (oculix-org#395 solid color Match), @robserm (oculix-org#286 NPE recording), @blackball (oculix-org#232 IME Chinese/Japanese), @emoQin (oculix-org#229 Android multi-device), @roboraptor (oculix-org#224 CLI validation), @micves (oculix-org#163 oculix-org#162 oculix-org#209 oculix-org#208 oculix-org#207 thumbnail pipeline), and @shaworth (oculix-org#15 OpenCV Ubuntu 24 that motivated the Legerix native refactor).

Related to oculix-org#277.

Co-authored-by: Daniele Paltrinieri <daniele-paltrinieri-79@users.noreply.github.com>
Co-authored-by: tcc <tcc@users.noreply.github.com>
Co-authored-by: peixuana <peixuana@users.noreply.github.com>
Co-authored-by: genequ <genequ@users.noreply.github.com>
Co-authored-by: andresluuk <andresluuk@users.noreply.github.com>
Co-authored-by: Zdenda3D <Zdenda3D@users.noreply.github.com>
Co-authored-by: robserm <robserm@users.noreply.github.com>
Co-authored-by: blackball <blackball@users.noreply.github.com>
Co-authored-by: emoQin <emoQin@users.noreply.github.com>
Co-authored-by: roboraptor <roboraptor@users.noreply.github.com>
Co-authored-by: micves <micves@users.noreply.github.com>
Co-authored-by: shaworth <shaworth@users.noreply.github.com>
Co-authored-by: Claude (Anthropic) <noreply@anthropic.com>
…kuliX1

Extend .github/CREDITS.md with a full "Original Sikuli lineage" section that credits every human whose commits landed in the original Sikuli repository at github.com/sikuli/sikuli, from the 2009 UIST paper co-authors down to the 1-commit contributors. Nobody gets left off — the current OculiX Java code descends from that codebase and inherits its DNA even where lines have been rewritten.

Founders visible in the GitHub Contributors graph via Co-authored-by trailers below : @doubleshow (Tom Yeh, 87 commits, feature-dense contributions) and @vgod (Tsung-Hsiang Chang, 981 commits, main codebase author on the original repository).

Early Sikuli contributors (2010-2013) also credited via Co-authored-by : @karlmicha (Karl-Michael Schneider, 28 commits), @techtonik (anatoly techtonik, 8 commits), @niknah (2 commits), @EarthCitizen (1 commit), @ksmyth (Kevin Smyth, 1 commit), @ChrisOelmueller (Chris Oelmueller, 1 commit), and @Liam-Deacon (Liam Deacon, 1 commit).

Prof. Rob Miller, who founded the UI Design Group at MIT CSAIL (now Usable Programming) and supervised the original Sikuli research from 2003 onward, is honored by name in the CREDITS file without a Co-authored-by trailer. His personal MIT page does not link a public GitHub account, mirroring the treatment I gave him in the SikuliX1 README lineage timeline.

@RaiMan is already at rank oculix-org#1 in the OculiX Contributors graph and needs no additional crediting here.

Sixteen years of the Sikuli-then-OculiX lineage in one file. The git graph will finally see everyone.

Co-authored-by: Tom Yeh <doubleshow@users.noreply.github.com>
Co-authored-by: Tsung-Hsiang Chang <vgod@users.noreply.github.com>
Co-authored-by: Karl-Michael Schneider <karlmicha@users.noreply.github.com>
Co-authored-by: anatoly techtonik <techtonik@users.noreply.github.com>
Co-authored-by: niknah <niknah@users.noreply.github.com>
Co-authored-by: EarthCitizen <EarthCitizen@users.noreply.github.com>
Co-authored-by: Kevin Smyth <ksmyth@users.noreply.github.com>
Co-authored-by: Chris Oelmueller <ChrisOelmueller@users.noreply.github.com>
Co-authored-by: Liam Deacon <Liam-Deacon@users.noreply.github.com>

Co-authored-by: Claude (Anthropic) <noreply@anthropic.com>
davidyoung8196504567-sudo pushed a commit to davidyoung8196504567-sudo/Oculix that referenced this pull request Aug 4, 2026
…ulix-org#450

Addresses the three finishing touches requested before merge:

1. KeyModifierTest: broadened from 4 to 9 cases — legacy Shift+Meta(5),
   extended Shift+Meta(320, the actual oculix-org#449 value), extended Shift+Alt(576,
   the Windows-side equivalent), extended Ctrl, extended AltGraph, a mixed
   legacy/extended value, and an unrelated-bit value (BUTTON1_DOWN_MASK)
   that must still resolve to no modifiers.

2. HotkeyControllerTest (new): direct test of the safety net itself — an
   unresolvable non-zero modifier must be refused (return "") rather than
   silently registering a bare-key global hotkey. This was the fix's most
   important line of defense and previously had no direct test.

3. HotkeyManagerDisableTest (new) + justification: covers the key == 0
   "hotkey disabled" short-circuit carried over from the investigation.
   Kept in this PR rather than split out because it's the mechanism the
   investigation itself used to disable the Stop hotkey via the
   preferences plist while isolating the real bug, and remains the only
   way to turn off a hotkey given the Preferences UI only exposes a
   binding control for Capture, not Stop. Verified via reflection that
   the short-circuit returns before touching any manager state.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
David Young and others added 2 commits August 3, 2026 21:44
…otkeys

KeyModifier.getModifierNames() only recognized the legacy
InputEvent.*_MASK bit values (SHIFT=1, META=4, ...), but a Stop-hotkey
modifier inherited from a SikuliX 2.0.x install is stored in the extended
InputEvent.*_DOWN_MASK convention (320 = SHIFT_DOWN_MASK + META_DOWN_MASK).
None of those bits match, so getModifierNames(320) returned "",
HotkeyController composed the KeyStroke "C" instead of "shift meta C",
and keymaster faithfully registered a bare, unmodified 'C' as a global
system hotkey — swallowing the letter C in every application.

Both codebases share the Java Preferences node /org/sikuli/script, so the
stored value migrates across. The latent mismatch was unmasked by
b159a25 (oculix-org#388), which correctly aligned getStopHotkeyModifiers() to read
the key setStopHotkeyModifiers() writes; before that the stored value was
never read at all.

Fix:
- KeyModifier.getModifierNames(): recognize both the legacy *_MASK and
  extended *_DOWN_MASK bit pattern for each modifier. The two ranges are
  disjoint (1/2/4/8/32 vs 64/128/256/512/8192), so this cannot produce a
  false positive.
- HotkeyController.addHotkey(HotkeyListener, int, int): refuse to register
  when a non-zero modifier resolves to no known modifier name, rather than
  silently degrading to an unmodified global hotkey.
- KeyModifierTest covering legacy and extended parsing.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Addresses review feedback from @julienmerconsulting on oculix-org#450.

KeyModifierTest: 4 -> 9 cases, covering every modifier getModifierNames()
recognizes in both conventions — legacy Shift+Meta (5), extended
Shift+Meta (320, the value observed in the wild), extended Shift+Alt (576,
the Windows-side equivalent of the same migration hazard), extended Ctrl,
extended AltGraph, a value mixing both conventions, and a non-modifier bit
(BUTTON1_DOWN_MASK) that must still resolve to no modifiers.

HotkeyControllerTest (new): direct coverage of the refusal safety net —
an unresolvable non-zero modifier must return "" and register nothing.
This is the fix's most important line of defense and previously had no
test of its own.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@julienmerconsulting

Copy link
Copy Markdown
Collaborator

@davidyoung8196504567-sudo — long overdue reply, apologies for the 3-week silence after you pushed the revisions on 04/08.

Honest reason: coming out of four intense months on the v4.0.0 + post-release work, I took actual vacation, and then @uwekoenig's #444 status-bar note turned into a full Region-provenance architectural pass — 24 commits, two external review passes, a new invariant model, a 44-test suite. That shipped yesterday as v4.1.0-rc1. Between the vacation reset and the #444 marathon, your PR did not get the attention it deserved. No excuse, just what happened.

Reviewed now. The diff is clean end to end:

  • HotkeyController.addHotkey — the refusal safety net is exactly the right shape. Non-zero modifier resolving to empty must not fall back to a bare-key global hotkey.
  • KeyModifier.getModifierNames — recognizing both legacy *_MASK and extended *_DOWN_MASK in parallel is the conservative fix. I verified the bit ranges are disjoint (CTRL=2 vs 128, SHIFT=1 vs 64, ALT=8 vs 512, ALTGR=32 vs 8192, META=4 vs 256), so no false positive is possible.
  • Tests — 9 modifier cases + the direct safety-net test using a fresh HotkeyController instance (great call, avoids the native provider entirely). Coverage is exactly on the bug shape.

I moved the base branch to `chore/global-bug-fixes` a few minutes ago (that's where fixes accumulate before syncing to master on the next release), and synced `master` into `chore` so the merge itself will be clean. GitHub cache is still catching up on the display side.

Merging now. Bug #449 closes automatically via the "Fixes #449" line. Your fix will land in the upcoming v4.1.0-rc2 pre-release alongside the #444 work.

Thank you for the follow-through on this one — the root-cause chain in #449, the 2.0.x archaeology confirmation, the revisions delivered in less than 24 hours after review comments. This is exactly the kind of contribution the project needs, and I owe you a faster turnaround next time.

🦎

@julienmerconsulting
julienmerconsulting merged commit de0286f into oculix-org:chore/global-bug-fixes Aug 27, 2026
4 checks passed
julienmerconsulting added a commit that referenced this pull request Aug 27, 2026
Local branch had merged fix/444 (Region provenance) and fix/425 (SubFindRun
guard) ahead of RC2 prep. Origin had PR #450 (KeyModifier dual-convention
parsing for macOS bare 'C' hotkey capture) merged in the meantime. Bring
both sides together so the push succeeds.

Co-authored-by: Claude (Anthropic) <noreply@anthropic.com>
davidyoung8196504567-sudo pushed a commit to davidyoung8196504567-sudo/Oculix that referenced this pull request Sep 4, 2026
The stop/abort hotkey has always been a preference with no way to change
it — the Preferences dialog only exposed the capture hotkey. RaiMan filed
oculix-org#335 for the case where the default collides with something else; there
was no recourse short of editing the Java Preferences store by hand,
which on macOS is not even reachable via the plist (cfprefsd owns it).

Adds a Hotkeys tab holding both hotkeys, and closes two gaps that oculix-org#449
exposed:

- Nothing validated a binding, so a user could bind a hotkey to a bare
  letter through the UI and reproduce oculix-org#449 by hand — the IDE would grab
  that letter globally, in every application. HotkeyBindingValidator now
  refuses a bare printable key, a modifier-only press, or a binding that
  collides with the other hotkey, and OK/Apply stay disabled with an
  inline reason until the combination is usable. Function keys are
  exempt from the modifier requirement: F13 is dedicated hardware, and
  grabbing it costs no application the ability to type.

- Registration failure was silent. HotkeyManager.addHotkey() already
  returned success, and both install paths discarded it, so a conflict
  looked identical to a hotkey that simply never fired. Both now act on
  the result and tell the user which combination could not be claimed —
  the substantive answer to "for conflict cases".

Also:
- STOP_HOTKEY_ENABLED preference, so a user with no free combination can
  switch the hotkey off entirely rather than fighting for one. This is
  the supported route the key==0 mechanism dropped from oculix-org#450 lacked.
- Stop hotkey is baked into the Run and Record tooltips at construction,
  and the capture hotkey into ButtonCapture's, so all three went stale
  after a rebind. They now refresh. The capture case was a pre-existing
  bug.
- Reset-to-defaults restores only the two hotkeys, via new public
  accessors, rather than calling setDefaults() and rewriting every
  preference in the store.

The dialog keeps the deprecated KeyEvent.getModifiers() deliberately —
it yields the legacy InputEvent.*_MASK convention that PreferencesUser
and Key.convertKeyToText() both expect. "Modernising" it to
getModifiersEx() would store extended *_DOWN_MASK values and recreate
the mismatch behind oculix-org#449. Commented at both call sites.

PreferencesWin is JFormDesigner-generated, but the .jfd form is already
stale (it predates _btnMore) and no build step regenerates the class, so
the new tab is hand-built outside the guarded blocks. A round-trip
through the designer would lose work.

Tab indices in initComponentsI18n shift by one: Hotkeys is inserted at 1,
so Text Editing is now 2 and General 3.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davidyoung8196504567-sudo pushed a commit to davidyoung8196504567-sudo/Oculix that referenced this pull request Sep 4, 2026
…startup NPE

Two defects found by actually launching the IDE built from 3e19e015.

1. Startup NPE — installCaptureHotkey() runs from startGUI() long before the
   toolbar exists, so the btnCapture.refreshTooltip() call added in 3e19e015
   dereferenced null, killed the main thread and left the splash screen up
   forever. btnRun/btnRecord were already guarded; btnCapture was not. Now
   guarded consistently: the button reads the binding in its own constructor
   anyway, so only a later rebind needs the refresh.

2. Modifiers vanished from the display. Key.convertKeyToText() renders via
   KeyEvent.getKeyModifiersText(), which understands only the legacy
   InputEvent.*_MASK bits. A stop hotkey stored in the extended *_DOWN_MASK
   convention — 320 for Shift+Cmd, as inherited from SikuliX 2.0.x, see oculix-org#449 —
   rendered as no modifiers at all. The new Hotkeys tab therefore displayed a
   perfectly good Shift+Cmd+C binding as a bare "C", which reads exactly like
   the oculix-org#449 bug had come back. The same wrong text was already reaching the
   Run, Record and Capture tooltips.

   This is the display twin of the oculix-org#449 registration bug. oculix-org#450 fixed
   KeyModifier.getModifierNames() for registration; this path was never
   touched. Fixed here rather than folded into oculix-org#450, which is already under
   review and scoped deliberately narrowly.

   Modifiers are now normalized to the legacy convention before rendering. The
   two conventions occupy disjoint bits (1/2/4/8/32 vs 64/128/256/512/8192),
   so accepting either cannot produce a false positive.

Verified on macOS 26.6: IDE launches, Hotkeys tab renders in second position,
capture shows "^ 2" and stop shows "⌘+⇧ C".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
davidyoung8196504567-sudo pushed a commit to davidyoung8196504567-sudo/Oculix that referenced this pull request Sep 4, 2026
…etect

Testing oculix-org#335 step 8 on macOS showed the warning cannot fire for a genuine
OS-level conflict there, so the previous comment overclaimed.

keymaster's Provider.register() returns void — it discards the OSStatus from
Carbon's RegisterEventHotKey and the BOOL from Win32's RegisterHotKey. Since
HotkeyController.installHotkey() returns the composed key string immediately
after calling it, the boolean the IDE acts on means only "the binding string
parsed", never "the OS accepted it". On macOS and Windows a combination
already owned by another application therefore registers "successfully" and
simply never fires.

Linux is different: LinuxHotkeyManager catches jxgrabkey's
HotkeyConflictException and returns false, so real grab conflicts do reach the
warning there.

The warning still earns its place — it fires on every platform for the
unresolvable-modifier refusal added in oculix-org#450, which is the failure mode that
produced oculix-org#449 — but the comment now says what it actually detects rather than
implying it covers all conflicts everywhere. Making macOS/Windows report real
conflicts would mean patching or replacing the bundled keymaster library,
which is deliberately out of scope.

Comment only; no behaviour change.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
julienmerconsulting pushed a commit that referenced this pull request Sep 7, 2026
api-compile, ide-compile and codeql only fire for pull requests targeting
master (plus release/oculix-rc for the two compile jobs). Since #450 landed,
chore/global-bug-fixes is where feature branches are asked to merge — so PRs
into it get none of those three, and the only check that runs is
ocr-perf-bench, which has no branch filter and matches on paths.

The effect is easy to miss because it looks like passing CI rather than absent
CI: a PR retargeted from master to chore/global-bug-fixes keeps the green ticks
its earlier head already earned, while its new head runs nothing at all.

Adds the branch to the three filters. No job definitions change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018XD7mNUrETDESsPY22Dubv
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.

macOS: bare 'C' key globally captured as Abort hotkey (modifier-mask convention mismatch)

2 participants