From 1e8e84239b0aa0f016a2c42b2057f4c93db4f6ed Mon Sep 17 00:00:00 2001 From: Gabriele Cirulli <886011+gabrielecirulli@users.noreply.github.com> Date: Fri, 10 Jul 2026 21:36:00 +0200 Subject: [PATCH 1/4] feat(hints): show hints in any Electron app without whitelisting it Hints previously appeared inside an Electron app only if its bundle ID was on a built-in list or added to additional_electron_bundles by hand, because that list decided whether neru set AXManualAccessibility to wake the app's accessibility tree. Unlisted Electron apps showed no hints. neru now sets AXManualAccessibility on every focused app whenever hints are enabled. The attribute wakes Electron and Chromium trees and is a harmless no-op on apps that do not implement it, with no window side effect, so it needs no whitelist and no cross-process tree-walk probe to decide when to apply it. AXEnhancedUserInterface, which exposes Chromium/Firefox web-page content but can move windows under tiling window managers, stays gated. It is set only on Chromium/Firefox browsers, and only when hints.additional_ax_support.enable is on. Electron apps no longer receive it on their own. Changes: - electron.go: replace the three per-family Ensure* functions, the depth-10 tree-walk probe, and the retry loop with one EnsureAppAccessibility that sets manual always and enhanced only when asked. Each successful set is cached per pid (keyed with the bundle id to survive pid reuse); a failed set is retried on a later focus and logged once so a real failure is visible without spamming the log on every focus of an app that does not support the attribute. - lifecycle.go: run the enablement whenever hints are enabled rather than only when the setting is on, and derive the enhanced flag from the Chromium/Firefox lists behind the setting. - enable_test.go: cover the cache, retry-on-failure, log-once, enhanced gating, and pid reuse with a fake attribute setter and a log observer. - electron_test.go: drop the tests for the deleted Electron-only helpers. - docs and default config: Electron works out of the box, the setting now describes browser web-content hints, the troubleshooting log lines match the code, and a tiling-window-manager caveat covers the enhanced attribute. The Electron and Chromium bundle lists stay: tree.go still uses them to prune noisy web DOM trees while scanning. Co-Authored-By: Claude Opus 4.8 --- configs/default-config.toml | 2 +- docs/CONFIGURATION.md | 19 +- docs/TROUBLESHOOTING.md | 47 ++- internal/app/lifecycle.go | 64 +--- internal/core/infra/electron/electron.go | 292 +++++------------- internal/core/infra/electron/electron_test.go | 106 ------- internal/core/infra/electron/enable_test.go | 206 ++++++++++++ 7 files changed, 333 insertions(+), 403 deletions(-) create mode 100644 internal/core/infra/electron/enable_test.go diff --git a/configs/default-config.toml b/configs/default-config.toml index 085862011..a2eba9c9c 100644 --- a/configs/default-config.toml +++ b/configs/default-config.toml @@ -101,7 +101,7 @@ visible_check_enabled = false "Right" = "action move_mouse_relative --dx=10 --dy=0" [hints.additional_ax_support] -enable = false # Enable enhanced AX for Electron/Chromium/Firefox/WebKit apps +enable = false # Chromium/Firefox web-page hints via AXEnhancedUserInterface. Electron works without it additional_electron_bundles = [] additional_chromium_bundles = [] additional_firefox_bundles = [] diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index f45d03479..d10242a5d 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -476,15 +476,15 @@ width = 320 ### Additional AX Support -Framework-specific accessibility improvements for Electron, Chromium, Firefox, and WebKit apps: +Makes web-page content inside browsers hintable. Electron apps (VS Code, Slack, and any other) are woken automatically on focus and do not need this setting. -| Option | Type | Default | Description | -| ----------------------------- | ----- | ------- | ---------------------------- | -| `enable` | bool | `false` | Enable additional AX support | -| `additional_electron_bundles` | array | `[]` | Bundle IDs of Electron apps | -| `additional_chromium_bundles` | array | `[]` | Bundle IDs of Chromium apps | -| `additional_firefox_bundles` | array | `[]` | Bundle IDs of Firefox apps | -| `additional_webkit_bundles` | array | `[]` | Bundle IDs of WebKit apps | +| Option | Type | Default | Description | +| ----------------------------- | ----- | ------- | ---------------------------------------------------------------------------------------------------------- | +| `enable` | bool | `false` | Set `AXEnhancedUserInterface` on Chromium and Firefox browsers so hints reach their web-page content | +| `additional_electron_bundles` | array | `[]` | Extra Electron bundle IDs, so neru recognises their web view and prunes the noisy DOM tree while scanning | +| `additional_chromium_bundles` | array | `[]` | Extra Chromium browser bundle IDs that should get web-content hints | +| `additional_firefox_bundles` | array | `[]` | Extra Firefox browser bundle IDs that should get web-content hints | +| `additional_webkit_bundles` | array | `[]` | Extra WebKit browser bundle IDs, so neru prunes their web DOM tree while scanning | ```toml [hints.additional_ax_support] @@ -497,6 +497,9 @@ additional_webkit_bundles = [] Find bundle IDs: `osascript -e 'id of app "Safari"'` +> [!WARNING] +> `enable` sets the `AXEnhancedUserInterface` attribute on Chromium and Firefox browsers. Under a tiling or window-snapping manager (yabai, Rectangle, Magnet, Amethyst), turning this attribute on can make macOS relayout or move the browser window. neru sets it only while `enable` is on, and only on apps it treats as Chromium or Firefox browsers. Native apps never receive it, and Electron apps receive it only if you list them in one of the browser arrays yourself. Leave `enable` off if the window-move side effect bothers you. + > [!TIP] > To support installed PWA apps, add a wildcard bundle ID to the appropriate browser array. For example, to support Brave-installed PWAs, add `"com.brave.Browser.app.*"` to `additional_chromium_bundles`: > diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index 7a7f83368..aa05fb109 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -122,9 +122,9 @@ neru hints # CLI works? log_level = "debug" ``` -### Electron/Chromium/Firefox issues +### Browser web content has no hints -**Enable additional AX support:** +Chromium and Firefox keep their web-page accessibility tree asleep until an app asks for it. Turn on the enhanced attribute so hints reach page content (Electron apps do not need this): ```toml [hints.additional_ax_support] @@ -163,35 +163,30 @@ tail -f ~/Library/Logs/neru/app.log ### Hints don't appear in Electron apps -**Electron apps need additional AX support.** +Electron apps (VS Code, Slack, Discord, and any other) are woken automatically the moment they gain focus, with no configuration. neru sets `AXManualAccessibility` on every focused app, which wakes the Electron accessibility tree. -**Solution:** - -Edit `~/.config/neru/config.toml`: +If hints still do not appear, turn on debug logging and confirm the wake happened: ```toml -[hints.additional_ax_support] -enable = true - -# If your app isn't auto-detected, add it: -additional_electron_bundles = [ - "com.your.electronapp", -] +[logging] +log_level = "debug" ``` -Restart Neru: +Restart Neru and watch the log: ```bash pkill neru && neru launch +tail -f ~/Library/Logs/neru/app.log ``` -**Check logs for:** +**Check the log for:** ``` -App requires Electron support -Enabled AXManualAccessibility for: com.your.app +manual accessibility set ``` +If instead you see `manual accessibility set failed`, macOS refused the attribute. For an Electron app that usually means neru is missing macOS Accessibility permission, so check that first. Ordinary native apps that do not implement the attribute log the same line harmlessly. If neither line appears for the app, confirm the app was frontmost when you opened hints. + ### Hints don't appear in Chrome/Firefox content **Browser needs additional AX support.** @@ -434,14 +429,12 @@ pkill -9 neru ### VS Code: Hints don't appear in editor -**Electron AX support needed.** - -**Solution:** +VS Code is an Electron app and is woken automatically on focus, so hints work on its buttons, tabs, and sidebar without configuration. The editor surface itself exposes a thin accessibility tree, so hints can be sparse inside the editing area. For that surface, switch VS Code to the Vision strategy, which detects elements from the rendered window instead of the AX tree: ```toml -[hints.additional_ax_support] -enable = true -# VS Code is auto-detected +[[hints.app_configs]] +bundle_id = "com.microsoft.VSCode" +strategy = "vision" ``` ### Adobe apps: Hints misaligned or missing @@ -632,9 +625,13 @@ grep "com.apple.Safari" ~/Library/Logs/neru/app.log ### Common log messages -**"App requires Electron support"** - Electron app detected, needs AX support enabled +**"manual accessibility set"** - neru woke an app's accessibility tree by setting AXManualAccessibility on it + +**"manual accessibility set failed"** - macOS refused AXManualAccessibility; harmless for a native app that does not implement it, a permission problem for an app that should + +**"enhanced accessibility set for web content"** - neru set AXEnhancedUserInterface on a Chromium/Firefox browser to expose its web page -**"Enabled AXManualAccessibility"** - Electron support activated successfully +**"enhanced accessibility set failed"** - macOS refused AXEnhancedUserInterface for a browser; its web-page content may not be hintable **"Hints mode activated"** - Hint overlay is active; includes hint count when available diff --git a/internal/app/lifecycle.go b/internal/app/lifecycle.go index f89906cdc..36cb644de 100644 --- a/internal/app/lifecycle.go +++ b/internal/app/lifecycle.go @@ -423,66 +423,22 @@ func (a *App) handleAppActivation(bundleID string) { } if cfg.Hints.Enabled { - if cfg.Hints.AdditionalAXSupport.Enable { - a.handleAdditionalAccessibility(bundleID, cfg) - } + a.handleAdditionalAccessibility(bundleID, cfg) } } -// handleAdditionalAccessibility configures accessibility support for Electron/Chromium/Firefox applications. +// handleAdditionalAccessibility wakes the focused application's accessibility +// tree so hints can read it. AXManualAccessibility is set on every focused app. +// The window-moving AXEnhancedUserInterface is set only for Chromium/Firefox +// browsers, and only when web-content hint support is enabled. func (a *App) handleAdditionalAccessibility(bundleID string, cfg *config.Config) { - config := cfg.Hints.AdditionalAXSupport - - isElectron := electron.ShouldEnableElectronSupport(bundleID, config.AdditionalElectronBundles) - isChromium := electron.ShouldEnableChromiumSupport(bundleID, config.AdditionalChromiumBundles) - isFirefox := electron.ShouldEnableFirefoxSupport(bundleID, config.AdditionalFirefoxBundles) - - if !isElectron && !isChromium && !isFirefox { - return - } + axCfg := cfg.Hints.AdditionalAXSupport - go func() { - // Apps may need time to initialize their accessibility tree after launch. - // We retry a few times to ensure the accessibility attributes are successfully set. - // Use exponential backoff to minimize latency for fast-booting apps while - // still accommodating slow-booting ones. - const ( - maxRetries = 5 - initialDelay = 100 * time.Millisecond - backoffFactor = 2 - ) - - delay := initialDelay - for range maxRetries { - allSuccess := true - - if isElectron { - if !electron.EnsureElectronAccessibility(bundleID, a.logger) { - allSuccess = false - } - } + useEnhanced := axCfg.Enable && + (electron.ShouldEnableChromiumSupport(bundleID, axCfg.AdditionalChromiumBundles) || + electron.ShouldEnableFirefoxSupport(bundleID, axCfg.AdditionalFirefoxBundles)) - if isChromium { - if !electron.EnsureChromiumAccessibility(bundleID, a.logger) { - allSuccess = false - } - } - - if isFirefox { - if !electron.EnsureFirefoxAccessibility(bundleID, a.logger) { - allSuccess = false - } - } - - if allSuccess { - return - } - - // Wait before retrying - time.Sleep(delay) - delay *= backoffFactor - } - }() + go electron.EnsureAppAccessibility(bundleID, useEnhanced, a.logger) } // printStartupInfo displays startup information including registered hotkeys. diff --git a/internal/core/infra/electron/electron.go b/internal/core/infra/electron/electron.go index 04345c76d..f986884e8 100644 --- a/internal/core/infra/electron/electron.go +++ b/internal/core/infra/electron/electron.go @@ -3,7 +3,6 @@ package electron import ( "strings" "sync" - "time" "go.uber.org/zap" @@ -12,65 +11,49 @@ import ( ) const ( - electronAttributeName = "AXManualAccessibility" + manualAttributeName = "AXManualAccessibility" enhancedAttributeName = "AXEnhancedUserInterface" ) -var ( - electronPIDsMu sync.Mutex - electronEnabledPIDs = make(map[int]struct{}) - chromiumPIDsMu sync.Mutex - chromiumEnabledPIDs = make(map[int]struct{}) - firefoxPIDsMu sync.Mutex - firefoxEnabledPIDs = make(map[int]struct{}) -) - -const ( - accessibilityRetryCount = 10 - accessibilityRetryDelay = 100 * time.Millisecond - maxAccessibilityDepth = 10 -) - -// EnsureElectronAccessibility ensures Electron accessibility is enabled for the provided bundle ID. -func EnsureElectronAccessibility(bundleID string, logger *zap.Logger) bool { - return ensureAccessibility( - bundleID, - &electronPIDsMu, - electronEnabledPIDs, - logger, - true, - ) -} - -// EnsureChromiumAccessibility ensures Chromium accessibility is enabled for the provided bundle ID. -func EnsureChromiumAccessibility(bundleID string, logger *zap.Logger) bool { - return ensureAccessibility( - bundleID, - &chromiumPIDsMu, - chromiumEnabledPIDs, - logger, - false, - ) +// setAttribute is the platform accessibility setter. It is a package variable +// so tests can substitute a fake for the cross-process AX call. +var setAttribute = platformSetApplicationAttribute + +// axState records which accessibility attributes have been set on a running +// application, keyed by pid. bundle guards against pid reuse: when the OS hands +// a retired pid to a different application, the mismatched bundle resets the +// record so the new application has its attributes set too. The *Failed flags +// remember that a set already failed and was logged, so a permanently +// unsupported app is retried on later focus without repeating the log line. +type axState struct { + bundle string + manual bool + manualFailed bool + enhanced bool + enhancedFailed bool } -// EnsureFirefoxAccessibility ensures Firefox accessibility is enabled for the provided bundle ID. -func EnsureFirefoxAccessibility(bundleID string, logger *zap.Logger) bool { - return ensureAccessibility( - bundleID, - &firefoxPIDsMu, - firefoxEnabledPIDs, - logger, - false, - ) -} +var ( + enabledPIDsMu sync.Mutex + enabledPIDs = make(map[int]axState) +) -func ensureAccessibility( - bundleID string, - pidsMu *sync.Mutex, - enabledPIDs map[int]struct{}, - logger *zap.Logger, - isElectron bool, -) bool { +// EnsureAppAccessibility sets the accessibility attributes that make an +// application's hint targets readable. +// +// AXManualAccessibility is set on every application. It wakes Electron and +// Chromium accessibility trees, works for any app without a bundle whitelist, +// and is a harmless no-op on applications that do not implement it (the set +// fails silently with no window side effect). +// +// AXEnhancedUserInterface is set only when useEnhanced is true, which the +// caller restricts to Chromium/Firefox browsers with web-content hints turned +// on. It exposes browser web-area content but can relayout or move windows +// under tiling window managers, so it stays off every other application. +// +// A successful set is cached per pid, so re-focusing an already-woken app does +// no further work. A failed set is not cached, so a later focus retries it. +func EnsureAppAccessibility(bundleID string, useEnhanced bool, logger *zap.Logger) { if logger == nil { logger = zap.NewNop() } @@ -81,170 +64,78 @@ func ensureAccessibility( if app == nil { logger.Debug("Application not found for bundle ID", zap.String("bundle_id", bundleID)) - return false + return } info, infoErr := app.Info() if infoErr != nil { - return false + return } pid := info.PID() - if pid <= 0 { - return false - } - - pidsMu.Lock() - - _, already := enabledPIDs[pid] - - pidsMu.Unlock() - - if already { - return true + return } - if hasUsableAccessibilityTree(app, logger) { - markPIDEnabled(pidsMu, enabledPIDs, pid) + ensurePIDAccessibility(pid, bundleID, useEnhanced, logger) +} - return true +// ensurePIDAccessibility applies and caches the accessibility attributes for a +// resolved pid. It is separated from the application lookup so the caching and +// gating rules can be tested without a live accessibility tree. +func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger *zap.Logger) { + enabledPIDsMu.Lock() + state := enabledPIDs[pid] + if !strings.EqualFold(state.bundle, bundleID) { + state = axState{bundle: bundleID} } + enabledPIDsMu.Unlock() - if isElectron { - success := platformSetApplicationAttribute(pid, electronAttributeName, true) + if !state.manual { + if setAttribute(pid, manualAttributeName, true) { + state.manual = true + state.manualFailed = false - if !success { logger.Debug( - "Failed to set AXManualAccessibility", - zap.Int("pid", pid), + "manual accessibility set", zap.String("bundle_id", bundleID), + zap.Int("pid", pid), ) - } - } + } else if !state.manualFailed { + state.manualFailed = true - if waitForAccessibility(app, logger) { - markPIDEnabled(pidsMu, enabledPIDs, pid) - - return true - } - - success := platformSetApplicationAttribute(pid, enhancedAttributeName, true) - - if !success { - logger.Debug( - "Failed to enable AXEnhancedUserInterface", - zap.Int("pid", pid), - zap.String("bundle_id", bundleID), - ) - - return false - } - - if waitForAccessibility(app, logger) { - markPIDEnabled(pidsMu, enabledPIDs, pid) - - return true - } - - logger.Warn( - "Accessibility could not be enabled", - zap.Int("pid", pid), - zap.String("bundle_id", bundleID), - ) - - return false -} - -func waitForAccessibility(app *accessibility.Element, logger *zap.Logger) bool { - for range accessibilityRetryCount { - if hasUsableAccessibilityTree(app, logger) { - return true + logger.Debug( + "manual accessibility set failed", + zap.String("bundle_id", bundleID), + zap.Int("pid", pid), + ) } - - time.Sleep(accessibilityRetryDelay) - } - - return false -} - -func hasUsableAccessibilityTree(root *accessibility.Element, logger *zap.Logger) bool { - if root == nil { - return false - } - - type entry struct { - el *accessibility.Element - depth int } - queue := []entry{{root, 0}} - - for len(queue) > 0 { - cur := queue[0] - queue = queue[1:] - - if cur.el == nil { - continue - } - - info, err := cur.el.Info() - if err != nil || info == nil { - continue - } - - role := info.Role() - - switch role { - case "AXWebArea", "AXScrollArea": - logger.Info("Found usable accessibility tree", zap.String("role", role)) - - return true - } + if useEnhanced && !state.enhanced { + if setAttribute(pid, enhancedAttributeName, true) { + state.enhanced = true + state.enhancedFailed = false - if cur.depth >= maxAccessibilityDepth { - continue - } - - children, err := cur.el.Children(role) - if err != nil { - continue - } + logger.Debug( + "enhanced accessibility set for web content", + zap.String("bundle_id", bundleID), + zap.Int("pid", pid), + ) + } else if !state.enhancedFailed { + state.enhancedFailed = true - for _, child := range children { - queue = append(queue, entry{child, cur.depth + 1}) + logger.Debug( + "enhanced accessibility set failed", + zap.String("bundle_id", bundleID), + zap.Int("pid", pid), + ) } } - return false -} - -func markPIDEnabled( - pidsMu *sync.Mutex, - enabledPIDs map[int]struct{}, - pid int, -) { - pidsMu.Lock() - defer pidsMu.Unlock() - - enabledPIDs[pid] = struct{}{} -} - -// ShouldEnableElectronSupport determines if the provided bundle identifier -// should have Electron accessibility manually toggled based on defaults and -// user-specified overrides. -func ShouldEnableElectronSupport(bundleID string, additionalBundles []string) bool { - if bundleID == "" { - return false - } - - if config.MatchesAdditionalBundle(bundleID, additionalBundles) { - return true - } - - result := IsLikelyElectronBundle(bundleID) - - return result + enabledPIDsMu.Lock() + enabledPIDs[pid] = state + enabledPIDsMu.Unlock() } // ShouldEnableChromiumSupport determines if Chromium accessibility should be enabled for the provided bundle. @@ -279,23 +170,6 @@ func ShouldEnableFirefoxSupport(bundleID string, additionalBundles []string) boo return result } -// IsLikelyElectronBundle returns true if the provided bundle identifier -// matches a known Electron signature. -func IsLikelyElectronBundle(bundleID string) bool { - lower := strings.ToLower(strings.TrimSpace(bundleID)) - if lower == "" { - return false - } - - for _, exact := range config.KnownElectronBundles { - if strings.EqualFold(strings.TrimSpace(exact), lower) { - return true - } - } - - return false -} - // IsLikelyChromiumBundle returns true if the provided bundle identifier matches a known Chromium signature. // This helps identify applications that would benefit from AXEnhancedUserInterface accessibility improvements. func IsLikelyChromiumBundle(bundleID string) bool { diff --git a/internal/core/infra/electron/electron_test.go b/internal/core/infra/electron/electron_test.go index 6ae0570ed..c6177437a 100644 --- a/internal/core/infra/electron/electron_test.go +++ b/internal/core/infra/electron/electron_test.go @@ -16,54 +16,6 @@ const ( bundleIDMyAppUpper = "COM.EXAMPLE.MYAPP" ) -func TestIsLikelyElectronBundle(t *testing.T) { - tests := []struct { - name string - bundleID string - want bool - }{ - { - name: "known electron bundle", - bundleID: "com.microsoft.VSCode", - want: true, - }, - { - name: "case insensitive match", - bundleID: "COM.MICROSOFT.VSCODE", - want: true, - }, - { - name: testNameUnknownBundle, - bundleID: bundleIDSafari, - want: false, - }, - { - name: testNameEmptyBundle, - bundleID: "", - want: false, - }, - { - name: "whitespace bundle", - bundleID: " ", - want: false, - }, - } - - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - got := electron.IsLikelyElectronBundle(testCase.bundleID) - if got != testCase.want { - t.Errorf( - "IsLikelyElectronBundle(%q) = %v, want %v", - testCase.bundleID, - got, - testCase.want, - ) - } - }) - } -} - func TestIsLikelyChromiumBundle(t *testing.T) { tests := []struct { name string @@ -112,64 +64,6 @@ func TestIsLikelyChromiumBundle(t *testing.T) { } } -func TestShouldEnableElectronSupport(t *testing.T) { - tests := []struct { - name string - bundleID string - additionalBundles []string - want bool - }{ - { - name: "known electron bundle", - bundleID: "com.microsoft.VSCode", - additionalBundles: []string{}, - want: true, - }, - { - name: testNameAdditionalBundle, - bundleID: bundleIDMyApp, - additionalBundles: []string{bundleIDMyApp}, - want: true, - }, - { - name: testNameUnknownBundle, - bundleID: bundleIDSafari, - additionalBundles: []string{}, - want: false, - }, - { - name: testNameEmptyBundle, - bundleID: "", - additionalBundles: []string{}, - want: false, - }, - { - name: testNameCaseInsensitiveAdditionalBundle, - bundleID: bundleIDMyApp, - additionalBundles: []string{bundleIDMyAppUpper}, - want: true, - }, - } - - for _, testCase := range tests { - t.Run(testCase.name, func(t *testing.T) { - got := electron.ShouldEnableElectronSupport( - testCase.bundleID, - testCase.additionalBundles, - ) - if got != testCase.want { - t.Errorf( - "ShouldEnableElectronSupport(%q, %v) = %v, want %v", - testCase.bundleID, - testCase.additionalBundles, - got, - testCase.want, - ) - } - }) - } -} - func TestShouldEnableChromiumSupport(t *testing.T) { tests := []struct { name string diff --git a/internal/core/infra/electron/enable_test.go b/internal/core/infra/electron/enable_test.go new file mode 100644 index 000000000..c739a982f --- /dev/null +++ b/internal/core/infra/electron/enable_test.go @@ -0,0 +1,206 @@ +package electron + +import ( + "testing" + + "go.uber.org/zap" + "go.uber.org/zap/zaptest/observer" +) + +// fakeSetter stands in for the cross-process AX attribute setter. It records +// which attributes it was asked to set and returns a programmed result per +// attribute (a missing entry means the set fails). +type fakeSetter struct { + calls []string + results map[string]bool +} + +func (f *fakeSetter) set(_ int, attribute string, _ bool) bool { + f.calls = append(f.calls, attribute) + + return f.results[attribute] +} + +func (f *fakeSetter) countOf(attribute string) int { + count := 0 + + for _, called := range f.calls { + if called == attribute { + count++ + } + } + + return count +} + +// newEnableTest swaps in a fake setter, clears the pid cache, and returns the +// fake plus a log observer. Everything is restored when the test ends. +func newEnableTest(t *testing.T, results map[string]bool) (*fakeSetter, *observer.ObservedLogs, *zap.Logger) { + t.Helper() + + fake := &fakeSetter{results: results} + + prev := setAttribute + setAttribute = fake.set + + t.Cleanup(func() { + setAttribute = prev + + enabledPIDsMu.Lock() + enabledPIDs = make(map[int]axState) + enabledPIDsMu.Unlock() + }) + + enabledPIDsMu.Lock() + enabledPIDs = make(map[int]axState) + enabledPIDsMu.Unlock() + + core, logs := observer.New(zap.DebugLevel) + + return fake, logs, zap.New(core) +} + +func stateForPID(pid int) axState { + enabledPIDsMu.Lock() + defer enabledPIDsMu.Unlock() + + return enabledPIDs[pid] +} + +func TestEnsurePIDManualSetOnceAndCached(t *testing.T) { + fake, logs, logger := newEnableTest(t, map[string]bool{manualAttributeName: true}) + + ensurePIDAccessibility(100, "com.example.app", false, logger) + ensurePIDAccessibility(100, "com.example.app", false, logger) + + if got := fake.countOf(manualAttributeName); got != 1 { + t.Fatalf("manual set attempts = %d, want 1 (cached after success)", got) + } + + if got := fake.countOf(enhancedAttributeName); got != 0 { + t.Fatalf("enhanced set attempts = %d, want 0 (not requested)", got) + } + + state := stateForPID(100) + if !state.manual || state.enhanced { + t.Fatalf("state = %+v, want manual only", state) + } + + if got := logs.FilterMessage("manual accessibility set").Len(); got != 1 { + t.Fatalf("manual success logs = %d, want 1", got) + } +} + +func TestEnsurePIDManualFailureRetriesAndLogsOnce(t *testing.T) { + fake, logs, logger := newEnableTest(t, map[string]bool{manualAttributeName: false}) + + ensurePIDAccessibility(100, "com.example.app", false, logger) + ensurePIDAccessibility(100, "com.example.app", false, logger) + + if got := fake.countOf(manualAttributeName); got != 2 { + t.Fatalf("manual set attempts = %d, want 2 (a failed set is retried)", got) + } + + state := stateForPID(100) + if state.manual { + t.Fatalf("state.manual = true, want false after a failed set") + } + + if !state.manualFailed { + t.Fatalf("state.manualFailed = false, want true") + } + + if got := logs.FilterMessage("manual accessibility set failed").Len(); got != 1 { + t.Fatalf("manual failure logs = %d, want 1 (logged once, not per focus)", got) + } +} + +func TestEnsurePIDEnhancedOnlyWhenRequested(t *testing.T) { + results := map[string]bool{manualAttributeName: true, enhancedAttributeName: true} + + fake, logs, logger := newEnableTest(t, results) + + ensurePIDAccessibility(100, "com.brave.Browser", false, logger) + + if got := fake.countOf(enhancedAttributeName); got != 0 { + t.Fatalf("enhanced set attempts = %d, want 0 when useEnhanced is false", got) + } + + if state := stateForPID(100); state.enhanced { + t.Fatalf("state.enhanced = true, want false when useEnhanced is false") + } + + if got := logs.FilterMessage("enhanced accessibility set for web content").Len(); got != 0 { + t.Fatalf("enhanced logs = %d, want 0", got) + } +} + +func TestEnsurePIDEnhancedSetWhenRequested(t *testing.T) { + results := map[string]bool{manualAttributeName: true, enhancedAttributeName: true} + + fake, logs, logger := newEnableTest(t, results) + + ensurePIDAccessibility(100, "com.brave.Browser", true, logger) + ensurePIDAccessibility(100, "com.brave.Browser", true, logger) + + if got := fake.countOf(enhancedAttributeName); got != 1 { + t.Fatalf("enhanced set attempts = %d, want 1 (cached after success)", got) + } + + state := stateForPID(100) + if !state.manual || !state.enhanced { + t.Fatalf("state = %+v, want manual and enhanced", state) + } + + if got := logs.FilterMessage("enhanced accessibility set for web content").Len(); got != 1 { + t.Fatalf("enhanced success logs = %d, want 1", got) + } +} + +func TestEnsurePIDEnhancedFailureRetriesAndLogsOnce(t *testing.T) { + results := map[string]bool{manualAttributeName: true, enhancedAttributeName: false} + + fake, logs, logger := newEnableTest(t, results) + + ensurePIDAccessibility(100, "com.brave.Browser", true, logger) + ensurePIDAccessibility(100, "com.brave.Browser", true, logger) + + if got := fake.countOf(enhancedAttributeName); got != 2 { + t.Fatalf("enhanced set attempts = %d, want 2 (a failed set is retried)", got) + } + + state := stateForPID(100) + if state.enhanced || !state.enhancedFailed { + t.Fatalf("state = %+v, want enhanced false and enhancedFailed true", state) + } + + if got := logs.FilterMessage("enhanced accessibility set failed").Len(); got != 1 { + t.Fatalf("enhanced failure logs = %d, want 1", got) + } +} + +func TestEnsurePIDReuseResetsState(t *testing.T) { + fake, _, logger := newEnableTest(t, map[string]bool{manualAttributeName: true}) + + ensurePIDAccessibility(100, "com.first.app", false, logger) + ensurePIDAccessibility(100, "com.second.app", false, logger) + + if got := fake.countOf(manualAttributeName); got != 2 { + t.Fatalf("manual set attempts = %d, want 2 (pid reused by a different bundle)", got) + } + + if state := stateForPID(100); state.bundle != "com.second.app" { + t.Fatalf("state.bundle = %q, want com.second.app", state.bundle) + } +} + +func TestEnsurePIDSameBundleCaseInsensitiveKeepsCache(t *testing.T) { + fake, _, logger := newEnableTest(t, map[string]bool{manualAttributeName: true}) + + ensurePIDAccessibility(100, "com.example.App", false, logger) + ensurePIDAccessibility(100, "COM.EXAMPLE.APP", false, logger) + + if got := fake.countOf(manualAttributeName); got != 1 { + t.Fatalf("manual set attempts = %d, want 1 (same bundle, different case)", got) + } +} From 5208426053f6dde8d8509db6c133d3e39f783331 Mon Sep 17 00:00:00 2001 From: Gabriele Cirulli <886011+gabrielecirulli@users.noreply.github.com> Date: Mon, 13 Jul 2026 12:25:28 +0200 Subject: [PATCH 2/4] Improve comments --- internal/app/lifecycle.go | 11 +++++++---- internal/core/infra/electron/electron.go | 5 ++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/internal/app/lifecycle.go b/internal/app/lifecycle.go index 36cb644de..5b702ccb0 100644 --- a/internal/app/lifecycle.go +++ b/internal/app/lifecycle.go @@ -427,10 +427,13 @@ func (a *App) handleAppActivation(bundleID string) { } } -// handleAdditionalAccessibility wakes the focused application's accessibility -// tree so hints can read it. AXManualAccessibility is set on every focused app. -// The window-moving AXEnhancedUserInterface is set only for Chromium/Firefox -// browsers, and only when web-content hint support is enabled. +// handleAdditionalAccessibility wakes the accessibility tree of browsers and +// Electron-based apps. It sets `AXManualAccessibility` on every focused app +// (no-op on non-chromium-based apps). +// +// It also sets `AXEnhancedUserInterface` on Chromium/Firefox browsers when +// web-content hints are enabled. This flag can have side effects on window +// layout under tiling window managers, so it is not set for other apps. func (a *App) handleAdditionalAccessibility(bundleID string, cfg *config.Config) { axCfg := cfg.Hints.AdditionalAXSupport diff --git a/internal/core/infra/electron/electron.go b/internal/core/infra/electron/electron.go index f986884e8..a5a2fd743 100644 --- a/internal/core/infra/electron/electron.go +++ b/internal/core/infra/electron/electron.go @@ -42,9 +42,8 @@ var ( // application's hint targets readable. // // AXManualAccessibility is set on every application. It wakes Electron and -// Chromium accessibility trees, works for any app without a bundle whitelist, -// and is a harmless no-op on applications that do not implement it (the set -// fails silently with no window side effect). +// Chromium accessibility trees, and is a harmless no-op on applications that do +// not implement it. // // AXEnhancedUserInterface is set only when useEnhanced is true, which the // caller restricts to Chromium/Firefox browsers with web-content hints turned From f3d1b87f67f94426703a763012de8ca450d65706 Mon Sep 17 00:00:00 2001 From: Gabriele Cirulli <886011+gabrielecirulli@users.noreply.github.com> Date: Mon, 13 Jul 2026 13:24:35 +0200 Subject: [PATCH 3/4] fix(hints): retry accessibility setup and clean up stale AX state Addresses review feedback on the accessibility enablement. - Retry with exponential backoff when an app is focused before its accessibility tree is ready, so hints appear in a slow-launching app without a manual refocus. The retry lives in EnsureAppAccessibility, and its burst is confined to an app's first encounter, so ordinary native apps (which do not take the attribute) are not re-probed on every focus. - Clear AXEnhancedUserInterface on the next focus of a browser after hints.additional_ax_support is turned off, so its tiling-window side effect does not outlast the setting. - Drop an app's cached accessibility state when it terminates, so a process that later reuses the retired pid has its attributes set again instead of inheriting stale per-process flags. This also bounds cache growth. Tests cover readiness reporting, first-encounter retry gating, the enhanced teardown, and the terminate-time cache reset. Co-Authored-By: Claude Opus 4.8 --- docs/TROUBLESHOOTING.md | 2 + internal/app/lifecycle.go | 23 +++- internal/core/infra/electron/electron.go | 140 ++++++++++++++++---- internal/core/infra/electron/enable_test.go | 121 ++++++++++++++++- 4 files changed, 249 insertions(+), 37 deletions(-) diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index aa05fb109..d0f814237 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -633,6 +633,8 @@ grep "com.apple.Safari" ~/Library/Logs/neru/app.log **"enhanced accessibility set failed"** - macOS refused AXEnhancedUserInterface for a browser; its web-page content may not be hintable +**"enhanced accessibility cleared"** - neru removed AXEnhancedUserInterface from a browser after web-content hint support was turned off, so its tiling-window side effect does not persist + **"Hints mode activated"** - Hint overlay is active; includes hint count when available **"Clickable element collection was slow"** - Accessibility scanning completed but took longer than expected diff --git a/internal/app/lifecycle.go b/internal/app/lifecycle.go index 5b702ccb0..9b251bbad 100644 --- a/internal/app/lifecycle.go +++ b/internal/app/lifecycle.go @@ -195,6 +195,13 @@ func (a *App) setupAppWatcherCallbacks() { a.handleAppActivation(bundleID) }) + // Drop an app's cached accessibility state when it quits, so a later process + // reusing its pid has the attributes set again instead of inheriting stale + // per-process flags. + a.appWatcher.OnTerminate(func(_, bundleID string) { + electron.ForgetAppAccessibility(bundleID) + }) + // Watch for display parameter changes (monitor unplug/plug, resolution changes) a.appWatcher.OnScreenParametersChanged(func() { a.handleScreenParametersChange() @@ -427,13 +434,17 @@ func (a *App) handleAppActivation(bundleID string) { } } -// handleAdditionalAccessibility wakes the accessibility tree of browsers and -// Electron-based apps. It sets `AXManualAccessibility` on every focused app -// (no-op on non-chromium-based apps). +// handleAdditionalAccessibility wakes the focused application's accessibility +// tree so hints can read it. It sets `AXManualAccessibility` on any focused app +// (a no-op on apps that do not implement it), as well as +// `AXEnhancedUserInterface` on Chromium and Firefox browsers when +// `hints.additional_ax_support` is enabled. The latter is necessary in order to +// expose web-area content but can cause windows to move and prevent tiling +// window managers from working correctly, so it stays off every other app. // -// It also sets `AXEnhancedUserInterface` on Chromium/Firefox browsers when -// web-content hints are enabled. This flag can have side effects on window -// layout under tiling window managers, so it is not set for other apps. +// The work runs on a background goroutine because EnsureAppAccessibility +// retries a freshly launched app with backoff and would otherwise block the +// activation handler while it waits. func (a *App) handleAdditionalAccessibility(bundleID string, cfg *config.Config) { axCfg := cfg.Hints.AdditionalAXSupport diff --git a/internal/core/infra/electron/electron.go b/internal/core/infra/electron/electron.go index a5a2fd743..6713e5508 100644 --- a/internal/core/infra/electron/electron.go +++ b/internal/core/infra/electron/electron.go @@ -3,6 +3,7 @@ package electron import ( "strings" "sync" + "time" "go.uber.org/zap" @@ -19,12 +20,11 @@ const ( // so tests can substitute a fake for the cross-process AX call. var setAttribute = platformSetApplicationAttribute -// axState records which accessibility attributes have been set on a running -// application, keyed by pid. bundle guards against pid reuse: when the OS hands -// a retired pid to a different application, the mismatched bundle resets the -// record so the new application has its attributes set too. The *Failed flags -// remember that a set already failed and was logged, so a permanently -// unsupported app is retried on later focus without repeating the log line. +// axState records which accessibility attributes have been set on one running +// application. bundle is the application's bundle id, used to detect pid reuse +// (see enabledPIDs). The *Failed flags remember that a set already failed and +// was logged, so an application that does not support an attribute is retried on +// later focus without repeating the log line. type axState struct { bundle string manual bool @@ -33,6 +33,13 @@ type axState struct { enhancedFailed bool } +// enabledPIDs caches the accessibility state of running applications, keyed by +// pid. An entry is dropped when its application terminates (see +// ForgetAppAccessibility), so a later process that reuses the retired pid starts +// from a clean slate. The axState.bundle field covers the window before that +// termination is processed: if the OS hands the pid to a different application +// first, the mismatched bundle resets the record so the new application has its +// attributes set too. var ( enabledPIDsMu sync.Mutex enabledPIDs = make(map[int]axState) @@ -41,17 +48,25 @@ var ( // EnsureAppAccessibility sets the accessibility attributes that make an // application's hint targets readable. // -// AXManualAccessibility is set on every application. It wakes Electron and +// `AXManualAccessibility` is set on every application. It wakes Electron and // Chromium accessibility trees, and is a harmless no-op on applications that do // not implement it. // -// AXEnhancedUserInterface is set only when useEnhanced is true, which the +// `AXEnhancedUserInterface` is set only when useEnhanced is true, which the // caller restricts to Chromium/Firefox browsers with web-content hints turned // on. It exposes browser web-area content but can relayout or move windows // under tiling window managers, so it stays off every other application. // // A successful set is cached per pid, so re-focusing an already-woken app does // no further work. A failed set is not cached, so a later focus retries it. +// +// A freshly launched app may not have its accessibility tree ready the moment +// it first gains focus, so the attributes are set with exponential-backoff +// retries. The retry burst is confined to an app's first encounter: an app +// already handled (woken, or one that does not take the attribute) returns +// after a single attempt, so ordinary native apps are not put through the whole +// burst on every focus. The retries sleep between attempts, so callers run this +// on a goroutine. func EnsureAppAccessibility(bundleID string, useEnhanced bool, logger *zap.Logger) { if logger == nil { logger = zap.NewNop() @@ -59,33 +74,66 @@ func EnsureAppAccessibility(bundleID string, useEnhanced bool, logger *zap.Logge logger = logger.Named("electron") + const ( + maxAttempts = 5 + initialDelay = 100 * time.Millisecond + backoffFactor = 2 + ) + + ready, retry := ensureAppAccessibilityOnce(bundleID, useEnhanced, logger) + if ready || !retry { + return + } + + delay := initialDelay + + for attempt := 1; attempt < maxAttempts; attempt++ { + time.Sleep(delay) + delay *= backoffFactor + + if ready, _ := ensureAppAccessibilityOnce(bundleID, useEnhanced, logger); ready { + return + } + } +} + +// ensureAppAccessibilityOnce resolves the application for bundleID and applies +// its accessibility attributes once. It reports whether every wanted attribute +// is in place (ready) and whether a backoff retry may still help (retry): true +// while the app is not yet resolvable or is seen for the first time, and false +// once it has been handled. +func ensureAppAccessibilityOnce(bundleID string, useEnhanced bool, logger *zap.Logger) (ready, retry bool) { app := accessibility.ApplicationByBundleID(bundleID) if app == nil { logger.Debug("Application not found for bundle ID", zap.String("bundle_id", bundleID)) - return + return false, true } info, infoErr := app.Info() if infoErr != nil { - return + return false, true } pid := info.PID() if pid <= 0 { - return + return false, true } - ensurePIDAccessibility(pid, bundleID, useEnhanced, logger) + return ensurePIDAccessibility(pid, bundleID, useEnhanced, logger) } // ensurePIDAccessibility applies and caches the accessibility attributes for a // resolved pid. It is separated from the application lookup so the caching and -// gating rules can be tested without a live accessibility tree. -func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger *zap.Logger) { +// gating rules can be tested without a live accessibility tree. It reports +// whether every wanted attribute is in place (ready) and whether this is the +// first time the process behind the pid is seen (retry), which the caller uses +// to limit the backoff retry burst to freshly launched apps. +func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger *zap.Logger) (ready, retry bool) { enabledPIDsMu.Lock() state := enabledPIDs[pid] - if !strings.EqualFold(state.bundle, bundleID) { + firstEncounter := !strings.EqualFold(state.bundle, bundleID) + if firstEncounter { state = axState{bundle: bundleID} } enabledPIDsMu.Unlock() @@ -111,21 +159,36 @@ func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger * } } - if useEnhanced && !state.enhanced { - if setAttribute(pid, enhancedAttributeName, true) { - state.enhanced = true - state.enhancedFailed = false - - logger.Debug( - "enhanced accessibility set for web content", - zap.String("bundle_id", bundleID), - zap.Int("pid", pid), - ) - } else if !state.enhancedFailed { - state.enhancedFailed = true + if useEnhanced { + if !state.enhanced { + if setAttribute(pid, enhancedAttributeName, true) { + state.enhanced = true + state.enhancedFailed = false + + logger.Debug( + "enhanced accessibility set for web content", + zap.String("bundle_id", bundleID), + zap.Int("pid", pid), + ) + } else if !state.enhancedFailed { + state.enhancedFailed = true + + logger.Debug( + "enhanced accessibility set failed", + zap.String("bundle_id", bundleID), + zap.Int("pid", pid), + ) + } + } + } else if state.enhanced { + // AXEnhancedUserInterface can relayout or move windows under tiling + // window managers, so when web-content hints are off it is cleared to + // keep that side effect from outlasting the setting. + if setAttribute(pid, enhancedAttributeName, false) { + state.enhanced = false logger.Debug( - "enhanced accessibility set failed", + "enhanced accessibility cleared", zap.String("bundle_id", bundleID), zap.Int("pid", pid), ) @@ -135,6 +198,27 @@ func ensurePIDAccessibility(pid int, bundleID string, useEnhanced bool, logger * enabledPIDsMu.Lock() enabledPIDs[pid] = state enabledPIDsMu.Unlock() + + return state.manual && (!useEnhanced || state.enhanced), firstEncounter +} + +// ForgetAppAccessibility drops the cached accessibility state for every pid +// belonging to the given bundle id. It is called when an application terminates +// so a later process that reuses a retired pid, or a fresh instance of the same +// app, starts from a clean slate and has its attributes set again. +func ForgetAppAccessibility(bundleID string) { + if bundleID == "" { + return + } + + enabledPIDsMu.Lock() + defer enabledPIDsMu.Unlock() + + for pid, state := range enabledPIDs { + if strings.EqualFold(state.bundle, bundleID) { + delete(enabledPIDs, pid) + } + } } // ShouldEnableChromiumSupport determines if Chromium accessibility should be enabled for the provided bundle. diff --git a/internal/core/infra/electron/enable_test.go b/internal/core/infra/electron/enable_test.go index c739a982f..57598c3f2 100644 --- a/internal/core/infra/electron/enable_test.go +++ b/internal/core/infra/electron/enable_test.go @@ -8,15 +8,17 @@ import ( ) // fakeSetter stands in for the cross-process AX attribute setter. It records -// which attributes it was asked to set and returns a programmed result per -// attribute (a missing entry means the set fails). +// which attributes it was asked to set, and with which value, and returns a +// programmed result per attribute (a missing entry means the set fails). type fakeSetter struct { calls []string + values []bool results map[string]bool } -func (f *fakeSetter) set(_ int, attribute string, _ bool) bool { +func (f *fakeSetter) set(_ int, attribute string, value bool) bool { f.calls = append(f.calls, attribute) + f.values = append(f.values, value) return f.results[attribute] } @@ -33,6 +35,18 @@ func (f *fakeSetter) countOf(attribute string) int { return count } +// lastValueOf returns the value of the most recent set of the given attribute, +// and whether the attribute was set at all. +func (f *fakeSetter) lastValueOf(attribute string) (value, found bool) { + for i := len(f.calls) - 1; i >= 0; i-- { + if f.calls[i] == attribute { + return f.values[i], true + } + } + + return false, false +} + // newEnableTest swaps in a fake setter, clears the pid cache, and returns the // fake plus a log observer. Everything is restored when the test ends. func newEnableTest(t *testing.T, results map[string]bool) (*fakeSetter, *observer.ObservedLogs, *zap.Logger) { @@ -204,3 +218,104 @@ func TestEnsurePIDSameBundleCaseInsensitiveKeepsCache(t *testing.T) { t.Fatalf("manual set attempts = %d, want 1 (same bundle, different case)", got) } } + +func TestEnsurePIDReportsReadiness(t *testing.T) { + fake, _, logger := newEnableTest(t, map[string]bool{ + manualAttributeName: true, + enhancedAttributeName: false, + }) + + // Manual only: ready as soon as the manual set succeeds. + if ready, _ := ensurePIDAccessibility(100, "com.example.app", false, logger); !ready { + t.Fatal("manual-only ready = false, want true after the manual set succeeds") + } + + // Enhanced wanted but its set fails: not ready, so the caller retries. + if ready, _ := ensurePIDAccessibility(200, "com.brave.Browser", true, logger); ready { + t.Fatal("ready = true, want false while the enhanced set keeps failing") + } + + fake.results[enhancedAttributeName] = true + + if ready, _ := ensurePIDAccessibility(200, "com.brave.Browser", true, logger); !ready { + t.Fatal("ready = false, want true once every wanted attribute is set") + } +} + +func TestEnsurePIDFirstEncounterGuidesRetry(t *testing.T) { + _, _, logger := newEnableTest(t, map[string]bool{manualAttributeName: false}) + + // First time this pid is seen: worth a backoff retry in case the app is + // still bringing its accessibility tree up. + if _, retry := ensurePIDAccessibility(100, "com.example.app", false, logger); !retry { + t.Fatal("retry = false on first encounter, want true so a slow launch is retried") + } + + // Seen before and still failing: a known app, so no retry burst on a later + // focus. + if _, retry := ensurePIDAccessibility(100, "com.example.app", false, logger); retry { + t.Fatal("retry = true on a later focus, want false so native apps are not re-probed") + } + + // A different process reusing the pid is a fresh encounter again. + if _, retry := ensurePIDAccessibility(100, "com.other.app", false, logger); !retry { + t.Fatal("retry = false after pid reuse, want true for the new process") + } +} + +func TestEnsurePIDEnhancedClearedWhenDisabled(t *testing.T) { + results := map[string]bool{manualAttributeName: true, enhancedAttributeName: true} + + fake, logs, logger := newEnableTest(t, results) + + // Enhanced is on for a browser, then web-content hints are turned off. + ensurePIDAccessibility(100, "com.brave.Browser", true, logger) + ensurePIDAccessibility(100, "com.brave.Browser", false, logger) + + if value, found := fake.lastValueOf(enhancedAttributeName); !found || value { + t.Fatalf("last enhanced set = (value=%v, found=%v), want (false, true) to clear it", value, found) + } + + if state := stateForPID(100); state.enhanced { + t.Fatal("state.enhanced = true, want false after the attribute is cleared") + } + + if got := logs.FilterMessage("enhanced accessibility cleared").Len(); got != 1 { + t.Fatalf("enhanced cleared logs = %d, want 1", got) + } + + // Re-focusing with hints still off does not clear again. + ensurePIDAccessibility(100, "com.brave.Browser", false, logger) + + if got := fake.countOf(enhancedAttributeName); got != 2 { + t.Fatalf("enhanced set attempts = %d, want 2 (one to set, one to clear)", got) + } +} + +func TestForgetAppAccessibilityResetsState(t *testing.T) { + fake, _, logger := newEnableTest(t, map[string]bool{manualAttributeName: true}) + + ensurePIDAccessibility(100, "com.example.App", false, logger) + + // An empty bundle id must not wipe unrelated entries. + ForgetAppAccessibility("") + + if state := stateForPID(100); state.bundle == "" { + t.Fatal("state was cleared by an empty-bundle forget, want it kept") + } + + // The app terminates; its cached state is dropped (bundle match is + // case-insensitive, like the pid-reuse guard). + ForgetAppAccessibility("COM.EXAMPLE.APP") + + if state := stateForPID(100); state.bundle != "" { + t.Fatalf("state = %+v, want empty after the app is forgotten", state) + } + + // A new process reuses the retired pid: the attribute is set again. + ensurePIDAccessibility(100, "com.example.App", false, logger) + + if got := fake.countOf(manualAttributeName); got != 2 { + t.Fatalf("manual set attempts = %d, want 2 (cache cleared on terminate, so the reused pid is set again)", got) + } +} From 0d90304865feb6c537b3b0da9000146524050ac6 Mon Sep 17 00:00:00 2001 From: Gabriele Cirulli <886011+gabrielecirulli@users.noreply.github.com> Date: Fri, 10 Jul 2026 22:13:15 +0200 Subject: [PATCH 4/4] refactor(config): rename hints.additional_ax_support to hints.web_content_hints The [hints.additional_ax_support] section only controls whether hints reach web-page content inside Chromium and Firefox browsers. Now that Electron apps are woken without any setting, the old name no longer describes what the section does. Renaming it to [hints.web_content_hints] makes the setting self-describing, and its on/off field is renamed from enable to enabled to match the boolean naming used elsewhere in the config (grid.enabled, systray.enabled). The four bundle lists keep their names. Because neru's TOML loader ignores unknown keys, a config still using the old section or the old field name would silently lose its settings. To prevent that, the loader rejects both through the existing configuration-validation error path, the same one that shows the "Configuration Validation Failed" dialog, with a message naming the new key. Changes: - config.go: rename the AdditionalAXSupport type to WebContentHints and its Enable field to Enabled, updating the toml/json tags to web_content_hints/enabled. - config_defaults.go, lifecycle.go, tree.go: update every reader. - service.go: reject both the removed section and the removed enable field during load with a CodeInvalidConfig error that points at the new key. - service_test.go: cover the rejection of the old section and the old field, and a successful load of the new names. - configs and docs: rename the section and field everywhere they appear. Co-Authored-By: Claude Opus 4.8 --- configs/default-config.toml | 4 +- configs/hints-only-config.toml | 4 +- docs/CONFIGURATION.md | 14 ++-- docs/TROUBLESHOOTING.md | 4 +- internal/app/lifecycle.go | 6 +- internal/config/config.go | 8 +- internal/config/config_defaults.go | 4 +- internal/config/service.go | 39 ++++++++++ internal/config/service_test.go | 92 +++++++++++++++++++++++ internal/core/infra/accessibility/tree.go | 6 +- 10 files changed, 156 insertions(+), 25 deletions(-) diff --git a/configs/default-config.toml b/configs/default-config.toml index a2eba9c9c..ecaa94f1f 100644 --- a/configs/default-config.toml +++ b/configs/default-config.toml @@ -100,8 +100,8 @@ visible_check_enabled = false "Left" = "action move_mouse_relative --dx=-10 --dy=0" "Right" = "action move_mouse_relative --dx=10 --dy=0" -[hints.additional_ax_support] -enable = false # Chromium/Firefox web-page hints via AXEnhancedUserInterface. Electron works without it +[hints.web_content_hints] +enabled = false # Chromium/Firefox web-page hints via AXEnhancedUserInterface. Electron works without it additional_electron_bundles = [] additional_chromium_bundles = [] additional_firefox_bundles = [] diff --git a/configs/hints-only-config.toml b/configs/hints-only-config.toml index 32d512ec5..16cbdfe02 100644 --- a/configs/hints-only-config.toml +++ b/configs/hints-only-config.toml @@ -51,8 +51,8 @@ ignore_clickable_check = false # "Shift+L" = "__disabled__" # } -[hints.additional_ax_support] -enable = true +[hints.web_content_hints] +enabled = true additional_electron_bundles = [] additional_chromium_bundles = [] diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index d10242a5d..35872e760 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -474,21 +474,21 @@ y_offset = 24 width = 320 ``` -### Additional AX Support +### Web Content Hints Makes web-page content inside browsers hintable. Electron apps (VS Code, Slack, and any other) are woken automatically on focus and do not need this setting. | Option | Type | Default | Description | | ----------------------------- | ----- | ------- | ---------------------------------------------------------------------------------------------------------- | -| `enable` | bool | `false` | Set `AXEnhancedUserInterface` on Chromium and Firefox browsers so hints reach their web-page content | +| `enabled` | bool | `false` | Set `AXEnhancedUserInterface` on Chromium and Firefox browsers so hints reach their web-page content | | `additional_electron_bundles` | array | `[]` | Extra Electron bundle IDs, so neru recognises their web view and prunes the noisy DOM tree while scanning | | `additional_chromium_bundles` | array | `[]` | Extra Chromium browser bundle IDs that should get web-content hints | | `additional_firefox_bundles` | array | `[]` | Extra Firefox browser bundle IDs that should get web-content hints | | `additional_webkit_bundles` | array | `[]` | Extra WebKit browser bundle IDs, so neru prunes their web DOM tree while scanning | ```toml -[hints.additional_ax_support] -enable = false +[hints.web_content_hints] +enabled = false additional_electron_bundles = [] additional_chromium_bundles = [] additional_firefox_bundles = [] @@ -498,14 +498,14 @@ additional_webkit_bundles = [] Find bundle IDs: `osascript -e 'id of app "Safari"'` > [!WARNING] -> `enable` sets the `AXEnhancedUserInterface` attribute on Chromium and Firefox browsers. Under a tiling or window-snapping manager (yabai, Rectangle, Magnet, Amethyst), turning this attribute on can make macOS relayout or move the browser window. neru sets it only while `enable` is on, and only on apps it treats as Chromium or Firefox browsers. Native apps never receive it, and Electron apps receive it only if you list them in one of the browser arrays yourself. Leave `enable` off if the window-move side effect bothers you. +> `enabled` sets the `AXEnhancedUserInterface` attribute on Chromium and Firefox browsers. Under a tiling or window-snapping manager (yabai, Rectangle, Magnet, Amethyst), turning this attribute on can make macOS relayout or move the browser window. neru sets it only while `enabled` is on, and only on apps it treats as Chromium or Firefox browsers. Native apps never receive it, and Electron apps receive it only if you list them in one of the browser arrays yourself. Leave `enabled` off if the window-move side effect bothers you. > [!TIP] > To support installed PWA apps, add a wildcard bundle ID to the appropriate browser array. For example, to support Brave-installed PWAs, add `"com.brave.Browser.app.*"` to `additional_chromium_bundles`: > > ```toml -> [hints.additional_ax_support] -> enable = true +> [hints.web_content_hints] +> enabled = true > additional_chromium_bundles = ["com.brave.Browser.app.*"] > ``` > diff --git a/docs/TROUBLESHOOTING.md b/docs/TROUBLESHOOTING.md index d0f814237..942e7c25e 100644 --- a/docs/TROUBLESHOOTING.md +++ b/docs/TROUBLESHOOTING.md @@ -127,8 +127,8 @@ log_level = "debug" Chromium and Firefox keep their web-page accessibility tree asleep until an app asks for it. Turn on the enhanced attribute so hints reach page content (Electron apps do not need this): ```toml -[hints.additional_ax_support] -enable = true +[hints.web_content_hints] +enabled = true ``` ### Menubar/Dock hints missing diff --git a/internal/app/lifecycle.go b/internal/app/lifecycle.go index 9b251bbad..c9ef48f52 100644 --- a/internal/app/lifecycle.go +++ b/internal/app/lifecycle.go @@ -438,7 +438,7 @@ func (a *App) handleAppActivation(bundleID string) { // tree so hints can read it. It sets `AXManualAccessibility` on any focused app // (a no-op on apps that do not implement it), as well as // `AXEnhancedUserInterface` on Chromium and Firefox browsers when -// `hints.additional_ax_support` is enabled. The latter is necessary in order to +// `hints.web_content_hints` is enabled. The latter is necessary in order to // expose web-area content but can cause windows to move and prevent tiling // window managers from working correctly, so it stays off every other app. // @@ -446,9 +446,9 @@ func (a *App) handleAppActivation(bundleID string) { // retries a freshly launched app with backoff and would otherwise block the // activation handler while it waits. func (a *App) handleAdditionalAccessibility(bundleID string, cfg *config.Config) { - axCfg := cfg.Hints.AdditionalAXSupport + axCfg := cfg.Hints.WebContentHints - useEnhanced := axCfg.Enable && + useEnhanced := axCfg.Enabled && (electron.ShouldEnableChromiumSupport(bundleID, axCfg.AdditionalChromiumBundles) || electron.ShouldEnableFirefoxSupport(bundleID, axCfg.AdditionalFirefoxBundles)) diff --git a/internal/config/config.go b/internal/config/config.go index 9a8887b60..f4f661bee 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -722,7 +722,7 @@ type HintsConfig struct { AppConfigs []AppConfig `json:"appConfigs" toml:"app_configs"` - AdditionalAXSupport AdditionalAXSupport `json:"additionalAxSupport" toml:"additional_ax_support"` + WebContentHints WebContentHints `json:"webContentHints" toml:"web_content_hints"` Hotkeys map[string]StringOrStringArray `json:"hotkeys" toml:"-"` } @@ -919,9 +919,9 @@ type HeldRepeatConfig struct { Interval int `json:"interval" toml:"interval_ms"` // Interval between subsequent repeats (ms) } -// AdditionalAXSupport defines accessibility support for specific application frameworks. -type AdditionalAXSupport struct { - Enable bool `json:"enable" toml:"enable"` +// WebContentHints configures accessibility for hints inside browser web-page content. +type WebContentHints struct { + Enabled bool `json:"enabled" toml:"enabled"` AdditionalElectronBundles []string `json:"additionalElectronBundles" toml:"additional_electron_bundles"` AdditionalChromiumBundles []string `json:"additionalChromiumBundles" toml:"additional_chromium_bundles"` AdditionalFirefoxBundles []string `json:"additionalFirefoxBundles" toml:"additional_firefox_bundles"` diff --git a/internal/config/config_defaults.go b/internal/config/config_defaults.go index 25f167368..dd1a9a750 100644 --- a/internal/config/config_defaults.go +++ b/internal/config/config_defaults.go @@ -437,8 +437,8 @@ func newDefaultConfig() *Config { AppConfigs: []AppConfig{}, - AdditionalAXSupport: AdditionalAXSupport{ - Enable: false, + WebContentHints: WebContentHints{ + Enabled: false, AdditionalElectronBundles: []string{}, AdditionalChromiumBundles: []string{}, AdditionalFirefoxBundles: []string{}, diff --git a/internal/config/service.go b/internal/config/service.go index 44fed17db..e46bf5dc2 100644 --- a/internal/config/service.go +++ b/internal/config/service.go @@ -313,6 +313,45 @@ func (s *Service) LoadWithValidation(path string) *LoadResult { return configResult } + // [hints.additional_ax_support] is no longer a valid section; it was replaced + // by [hints.web_content_hints]. The loader ignores unknown keys, so a config + // still using the old name would silently lose its settings. Reject it and + // point at the new key. + if hints, ok := raw["hints"].(map[string]any); ok { + if _, present := hints["additional_ax_support"]; present { + configResult.ValidationError = derrors.New( + derrors.CodeInvalidConfig, + "[hints.additional_ax_support] has been renamed to [hints.web_content_hints]. "+ + "Rename that section in your config to keep browser web-content hints.", + ) + configResult.Config = DefaultConfig() + + s.logger.Warn("Config uses the renamed key hints.additional_ax_support", + zap.Error(configResult.ValidationError)) + + return configResult + } + + // web_content_hints.enable was renamed to web_content_hints.enabled. The + // loader ignores unknown keys, so the old field would silently disable + // web-content hints. Reject it and point at the new key. + if webContentHints, ok := hints["web_content_hints"].(map[string]any); ok { + if _, present := webContentHints["enable"]; present { + configResult.ValidationError = derrors.New( + derrors.CodeInvalidConfig, + "[hints.web_content_hints] enable has been renamed to enabled. "+ + "Rename that key in your config to keep browser web-content hints.", + ) + configResult.Config = DefaultConfig() + + s.logger.Warn("Config uses the renamed key hints.web_content_hints.enable", + zap.Error(configResult.ValidationError)) + + return configResult + } + } + } + // Decode into typed config struct (separate pass for validation) _, err := toml.DecodeFile(configResult.ConfigPath, configResult.Config) if err != nil { diff --git a/internal/config/service_test.go b/internal/config/service_test.go index bafc2aa78..b9d9303e3 100644 --- a/internal/config/service_test.go +++ b/internal/config/service_test.go @@ -277,3 +277,95 @@ bundle_id = "com.example.app" }) } } + +func TestService_LoadWithValidation_RenamedWebContentHintsKey(t *testing.T) { + service := config.NewService(config.DefaultConfig(), "", zap.NewNop(), nil) + + t.Run("old additional_ax_support key is rejected with guidance", func(t *testing.T) { + configPath := writeTempToml(t, ` +[hints.additional_ax_support] +enable = true +`) + + result := service.LoadWithValidation(configPath) + + if result.ValidationError == nil { + t.Fatal("LoadWithValidation() expected a ValidationError for the renamed key, got nil") + } + + errMsg := result.ValidationError.Error() + if !strings.Contains(errMsg, "additional_ax_support") || + !strings.Contains(errMsg, "web_content_hints") { + t.Errorf("error = %q, want it to name both the old and new keys", errMsg) + } + }) + + t.Run("new web_content_hints key loads", func(t *testing.T) { + configPath := writeTempToml(t, ` +[hints.web_content_hints] +enabled = true +additional_chromium_bundles = ["com.example.browser"] +`) + + result := service.LoadWithValidation(configPath) + + if result.ValidationError != nil { + t.Fatalf("LoadWithValidation() unexpected ValidationError: %v", result.ValidationError) + } + + if !result.Config.Hints.WebContentHints.Enabled { + t.Error("web_content_hints.enabled did not load as true") + } + + got := result.Config.Hints.WebContentHints.AdditionalChromiumBundles + if len(got) != 1 || got[0] != "com.example.browser" { + t.Errorf("additional_chromium_bundles = %v, want [com.example.browser]", got) + } + }) + + t.Run("old web_content_hints.enable field is rejected with guidance", func(t *testing.T) { + configPath := writeTempToml(t, ` +[hints.web_content_hints] +enable = true +`) + + result := service.LoadWithValidation(configPath) + + if result.ValidationError == nil { + t.Fatal("LoadWithValidation() expected a ValidationError for the renamed field, got nil") + } + + errMsg := result.ValidationError.Error() + if !strings.Contains(errMsg, "enable") || !strings.Contains(errMsg, "enabled") { + t.Errorf("error = %q, want it to name both the old and new field", errMsg) + } + }) + + t.Run("empty old section is still rejected", func(t *testing.T) { + configPath := writeTempToml(t, "[hints.additional_ax_support]\n") + + result := service.LoadWithValidation(configPath) + + if result.ValidationError == nil { + t.Fatal("expected the bare old section to be rejected, got nil") + } + + if !strings.Contains(result.ValidationError.Error(), "web_content_hints") { + t.Errorf("error = %q, want it to name the new key", result.ValidationError.Error()) + } + }) + + t.Run("non-table hints does not misfire the guard", func(t *testing.T) { + configPath := writeTempToml(t, "hints = 5\n") + + // A non-table [hints] must skip the rename guard without panicking. The + // typed decode reports the real type error; the guard must not claim it + // fired. + result := service.LoadWithValidation(configPath) + + if result.ValidationError != nil && + strings.Contains(result.ValidationError.Error(), "has been renamed") { + t.Errorf("rename guard misfired on a non-table hints: %v", result.ValidationError) + } + }) +} diff --git a/internal/core/infra/accessibility/tree.go b/internal/core/infra/accessibility/tree.go index 6aabb8e4a..be880bd28 100644 --- a/internal/core/infra/accessibility/tree.go +++ b/internal/core/infra/accessibility/tree.go @@ -954,12 +954,12 @@ func isUserConfiguredChromiumElectron(bundleID string, configProvider config.Pro return false } - chromiumBundles := cfg.Hints.AdditionalAXSupport.AdditionalChromiumBundles + chromiumBundles := cfg.Hints.WebContentHints.AdditionalChromiumBundles if config.MatchesAdditionalBundle(bundleID, chromiumBundles) { return true } - electronBundles := cfg.Hints.AdditionalAXSupport.AdditionalElectronBundles + electronBundles := cfg.Hints.WebContentHints.AdditionalElectronBundles return config.MatchesAdditionalBundle(bundleID, electronBundles) } @@ -988,6 +988,6 @@ func isWebKit(bundleID string, configProvider config.Provider) bool { return config.MatchesAdditionalBundle( bundleID, - cfg.Hints.AdditionalAXSupport.AdditionalWebKitBundles, + cfg.Hints.WebContentHints.AdditionalWebKitBundles, ) }