Repository navigation
macOS: Introduce AccessibilityContext, improve existing AX APIs, add accessibilityRange(forLine:) - #11196
Open
MrMage wants to merge 1 commit into
Open
macOS: Introduce AccessibilityContext, improve existing AX APIs, add accessibilityRange(forLine:)#11196MrMage wants to merge 1 commit into
accessibilityRange(forLine:)#11196MrMage wants to merge 1 commit into
Conversation
Add a new AccessibilityContext type (Screen.zig) that captures the full terminal text and viewport byte offsets in a single PinMap pass. This replaces the two separate ghostty_surface_read_text calls (screen + viewport extents) with one context that provides both, halving the work. The PinMap is needed because accessibilityVisibleCharacterRange must report the viewport as a range within the full text — without it, the previous implementation simply returned the range of the entire text. Swift-side changes: - Replace cachedScreenContents + cachedVisibleContents with a single cachedScreenTextInfo backed by the new context - Fix accessibilityNumberOfCharacters: return UTF-16 length instead of grapheme cluster count (NSRange operates on UTF-16 offsets) - Fix accessibilityVisibleCharacterRange: return viewport range instead of full text range - Fix accessibilityLine(for:): proper UTF-16 index conversion with defensive clamping for out-of-range values - Add accessibilityRange(forLine:): new override enabling VoiceOver line navigation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This comment was marked as low quality.
This comment was marked as low quality.
This comment has been minimized.
This comment has been minimized.
This comment was marked as low quality.
This comment was marked as low quality.
Member
Do not ping maintainers hoping to expedite review. It won't happen any faster, and just serves to annoy people - more than you think since many people receive "full feeds" of the activity on the Ghostty repo. |
3 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is the first in a series of PRs improving terminal accessibility on macOS, effectively splitting #10992 over multiple, smaller PRs. See #9930 and #9932 for motivation.
It introduces an
AccessibilityContexttype on the Zig side and uses it to back the existing accessibility APIs inSurfaceView. That in itself is not valuable yet (and in fact, theAccessibilityContextgeneration via PinMap is arguably more complex than the previous approach), but it's a necessary foundation for future improvements (again, see #10992 for what's to come). Large portions of the Accessibility API (includingaccessibilityVisibleCharacterRange()) will require us to build the PinMap anyway, and with this approach we can at least centralize that logic and data in one place.However, this PR also provides some immediate value: it provides a proper implementation of
accessibilityVisibleCharacterRange()and fixes two UTF-16 vs. UTF-8 mismatch issues inaccessibilityNumberOfCharacters()andaccessibilityLine(for:). It also introduces the inverse of that method,accessibilityRange(forLine:).Also see #10992 for a small helper tool that one can use to probe the accessibility APIs for verification.
Zig core:
AccessibilityContext(Screen.zig,Surface.zig)A new
AccessibilityContextstruct holds the full terminal text (scrollback + active area) along with the byte offsets delimiting the visible viewport. It is built from a PinMap — the same mechanism used byselectionString— to accurately determine where the viewport sits within the full text. The PinMap is necessary becauseaccessibilityVisibleCharacterRange()needs to report the viewport as a character range within the full text; the previous implementation had no way to do this and simply returned the range of the entire text. Without the PinMap, the alternatives would be fragile — fetching the viewport text separately and searching for it within the full text could match the wrong occurrence, and making two separate Zig calls leaves no reliable way to correlate their offsets. The PinMap maps every byte in the output text back to its terminal Pin, so we can precisely identify where the viewport starts and ends. The context owns its data (no Pins or page pointers) so it can be used after releasing the terminal mutex.Swift: unified cache and AX method fixes (
SurfaceView_AppKit.swift)The two separate
CachedValue<String>properties (cachedScreenContents,cachedVisibleContents) are replaced by a singleCachedValue<ScreenTextInfo>. TheScreenTextInfostruct holds the full text and the viewport range as aRange<String.Index>.accessibilityVisibleCharacterRange()converts this range toNSRangeonly when needed.Known limitations
accessibilityLine(for:)andaccessibilityRange(forLine:)count\n-delimited lines, not visual screen rows. Soft-wrapped lines are treated as a single line. Accurate visual-line support requires the grid mapping infrastructure planned for a follow-up PR.Outlook
Subsequent PRs will build on
AccessibilityContextto add:viewport_cells,grid_to_offset) foraccessibilityFrame(for:)andaccessibilityRange(for:)(bounds/position queries).content_changednotification pipeline so VoiceOver stays in sync with terminal output without polling — gated on whether an accessibility client is actually connected, to avoid unnecessary work.accessibilityInsertionPointLineNumber()-- Depends on whether this is actually valuable, and/or whether we would need to change the semantics ofaccessibilityLine(for:)andaccessibilityRange(forLine:)to refer to newlines as rendered rather than as line break characters.