Skip to content

fix(focus): prime Obsidian's Electron accessibility tree so suggestions work (#791) - #825

Open
francescorinaudo wants to merge 1 commit into
FuJacob:mainfrom
francescorinaudo:fix/obsidian-electron-priming
Open

fix(focus): prime Obsidian's Electron accessibility tree so suggestions work (#791)#825
francescorinaudo wants to merge 1 commit into
FuJacob:mainfrom
francescorinaudo:fix/obsidian-electron-priming

Conversation

@francescorinaudo

@francescorinaudo francescorinaudo commented Sep 7, 2026

Copy link
Copy Markdown

Summary

Obsidian (md.obsidian) was missing from BrowserAppDetector.electronEditorBundleIdentifiers, so Cotabby never set AXManualAccessibility on it. Electron builds its accessibility tree lazily; without that switch Obsidian's window exposes a single bare AXGroup, the system-wide focused-element query returns nil, and the focus tracker logs "No focused Accessibility element" on every tick. Adding the bundle id to the allowlist turns on the same priming, app-scoped focus fallback, descendant search and web-content caret handling that fixed VS Code in #671.

Evidence: a read-only AX probe of Obsidian 1.12.7 (Electron 39.8.3 / Chromium 142) shows the application element advertising AXManualAccessibility (settable, value 0) and AXEnhancedUserInterface, with the focused window holding one AXGroup and three buttons, i.e. the dormant renderer view. Obsidian worked in 0.3.3 (#276) and broke in 0.6.0 when the Electron allowlist gate was introduced.

Validation

xcodebuild test -project Cotabby.xcodeproj -scheme Cotabby -destination 'platform=macOS' \
  -skip-testing:CotabbyTests/FoundationModelDriftEvalTests CODE_SIGNING_ALLOWED=NO \
  -derivedDataPath "$HOME/Library/Caches/myCotabby/DerivedData"
# ** TEST SUCCEEDED **  Executed 1785 tests, with 6 tests skipped and 0 failures
  • The four new assertions (BrowserAppDetectorTests, WebContentFieldDetectorTests) failed before the one-line allowlist change and pass after it.
  • xcodebuild build of the Cotabby Dev scheme: succeeded (3 pre-existing warnings, 0 errors).
  • swiftlint lint --config .swiftlint.yml --quiet (SwiftLint 0.65.1): exit 0, no warnings on the changed files; repo-wide count unchanged versus main (0 → 0).
  • End-to-end on macOS 26.6.2, Obsidian 1.12.7 (Electron 39.8.3): with a Debug build of this branch (-cotabby-debug) and Obsidian frontmost, the log shows CHROME-PRIME enabled web accessibility for Obsidian, then CHROME-FOCUS-PROBE resolved via hit-test for Obsidian, then Focus snapshot changed: app=Obsidian capability=Supported with caret=derived (and exact) on every resolve, i.e. the inline ghost-text path, not the popup fallback. Ghost text appears in the editor and can be accepted. On main the same session logs only No focused Accessibility element for Obsidian.

Linked issues

Fixes #791
Refs #616 (ghost-text placement inside Obsidian is a separate geometry problem and is not addressed here)
Refs #626, #671

Risk / rollout notes

  • Behavior change is limited to md.obsidian: Cotabby now sets AXManualAccessibility on Obsidian's application element, exactly as it already does for Chromium browsers, ClickUp and VS Code. No settings, schema or pbxproj changes.
  • Obsidian bundles that ship under other ids (Insider builds use the same md.obsidian) are covered; matching is case-insensitive and exact, so no other md.* app is affected.
  • Placement ([Bug] Suggestion Display in Obsidian #616) may still route to the popup card where CodeMirror's caret geometry is only .estimated; that is the existing CompletionRenderModePolicy behaviour and is left for a follow-up.

Summary by CodeRabbit

  • Bug Fixes

    • Improved focus detection in Obsidian and VSCodium editor fields.
    • Obsidian editor content is now recognized as web content even when DOM metadata is unavailable.
  • Tests

    • Added coverage for Obsidian and case-insensitive application identifiers.
    • Added verification for Obsidian’s accessibility priming behavior.

Greptile Summary

Adds Obsidian to the existing Electron-editor allowlist so Cotabby primes its dormant web accessibility tree and can resolve its CodeMirror editing surface.

  • Recognizes md.obsidian case-insensitively as a covered Electron editor.
  • Applies the existing accessibility-priming and web-content detection paths to Obsidian.
  • Adds focused tests for bundle detection, priming eligibility, and web-content classification.

Confidence Score: 5/5

The PR appears safe to merge, with no outstanding or newly introduced actionable issues.

The current diff narrowly extends an existing Electron recovery path to Obsidian and adds aligned tests; there have been no changes since the previous review and no repository-rule violations remain.

Important Files Changed

Filename Overview
Cotabby/Support/Focus/Applications/BrowserAppDetector.swift Adds Obsidian's bundle identifier to the narrow Electron-editor accessibility-priming allowlist.
CotabbyTests/Support/Accessibility/WebContentFieldDetectorTests.swift Verifies that Obsidian fields are classified as web content without DOM-reflection attributes.
CotabbyTests/Support/Focus/Applications/BrowserAppDetectorTests.swift Covers Obsidian detection, case-insensitive matching, and accessibility-priming eligibility.

Reviews (2): Last reviewed commit: "fix(focus): prime Obsidian's Electron AX..." | Re-trigger Greptile

Context used:

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 361d6262-a430-4ece-a211-0c809b92be13

📥 Commits

Reviewing files that changed from the base of the PR and between ac2699e and 1a738a9.

📒 Files selected for processing (3)
  • Cotabby/Support/Focus/Applications/BrowserAppDetector.swift
  • CotabbyTests/Support/Accessibility/WebContentFieldDetectorTests.swift
  • CotabbyTests/Support/Focus/Applications/BrowserAppDetectorTests.swift

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.


📝 Walkthrough

Walkthrough

Browser detection now recognizes Obsidian as an Electron editor. Obsidian activates web accessibility priming, and tests cover case-insensitive bundle matching, priming, and web-content classification.

Changes

Obsidian Electron support

Layer / File(s) Summary
Electron allowlist and validation
Cotabby/Support/Focus/Applications/BrowserAppDetector.swift, CotabbyTests/Support/Focus/Applications/BrowserAppDetectorTests.swift, CotabbyTests/Support/Accessibility/WebContentFieldDetectorTests.swift
The Electron editor allowlist includes md.obsidian. Detection is case-insensitive, Obsidian requires web accessibility priming, and web-content classification works without DOM attributes. The comments document Obsidian’s CodeMirror 6 accessibility behavior.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 853ee

Obsidian is now recognized as an Electron editor so its accessibility tree is primed and suggestions can operate in its editor. The targeted behavior and regression coverage are in place, with no remaining merge-readiness risk identified.

Suggested reviewers: fujacob

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Out of Scope Changes check ⚠️ Warning The PR objectives are limited to Obsidian, but the production allowlist also adds VSCodium (com.vscodium). The provided issue does not require this change. Remove the VSCodium allowlist change and related test updates, or provide a linked issue and objective that explicitly require VSCodium support.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly identifies the Obsidian accessibility-tree fix and matches the primary change.
Linked Issues check ✅ Passed The PR adds Obsidian to the Electron accessibility allowlist and adds regression coverage. These changes directly address the focus and suggestion-detection failure reported in issue [#791].
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

…Jacob#791)

Obsidian (md.obsidian) was missing from BrowserAppDetector's Electron-editor
allowlist, so Cotabby never set AXManualAccessibility on it and its CodeMirror
editor stayed invisible to the focus tracker ("No focused Accessibility element").
A read-only AX probe of Obsidian 1.12.7 / Electron 39 confirmed the app element
advertises the attribute. Same shape as the VS Code fix (FuJacob#671).

Tests: BrowserAppDetectorTests and WebContentFieldDetectorTests gained md.obsidian
cases (red before, green after); full suite 1785 passed / 6 skipped / 0 failed.
@francescorinaudo
francescorinaudo force-pushed the fix/obsidian-electron-priming branch from 1a738a9 to 853eef4 Compare September 7, 2026 13:29
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.

[Bug] obsidian incompatibility with cotabby

1 participant