When AXSelectedTextChanged is posted from the parent EmacsView (NSView)
with UIElementsKey pointing to the EmacsAXBuffer element, VoiceOver calls
accessibilityLineForIndex: on the VIEW rather than on the focused element.
In specialised buffers (org-agenda, org-super-agenda) where line geometry
differs from plain text, the view returns an incorrect range and VoiceOver
reads only the first word at the cursor (e.g. 'La' or 'Liga') instead of
the full line.
Plain text buffers were unaffected because the fallback geometry happened
to be correct for simple line layouts.
Fix: post AXSelectedTextChanged on self (the EmacsAXBuffer element)
instead of on self.emacsView. This causes VoiceOver to call
accessibilityLineForIndex: on the element that owns the selection, which
returns the correct line range in all buffer types. Remove UIElementsKey
(unnecessary when posting from the element itself).
This aligns with the pre-review code (51f5944) which always posted
AX notifications directly on the focused element.
For discontiguous moves (teleports, org-agenda items separated by blank
lines, multi-line jumps), AXSelectedTextChanged was sent with
AXTextSelectionDirection=discontiguous. VoiceOver interprets an
explicit discontiguous direction as 're-anchor only' and reads only the
word at the cursor, ignoring VoiceOver's own line-browse mode.
The pre-review code (51f5944) omitted direction/granularity for all
moves and let VoiceOver determine what to read from its navigation state.
This correctly reads the full line when the VoiceOver rotor is in line
mode, which is the typical setting for text navigation.
Fix: omit AXTextSelectionDirection and AXTextSelectionGranularity from
AXSelectedTextChanged when direction=discontiguous. Include them only
for sequential moves (direction=next/previous), where the explicit hint
ensures VoiceOver reads the correct unit without an extra state query.
This fixes:
- org-agenda / org-super-agenda j/k: items separated by blank lines
cause singleLineMove=NO (non-adjacent AX indices), so direction was
discontiguous -> only first word read.
- Any other navigation that crosses blank or invisible lines.
Sequential moves (C-n/C-p, single adjacent j/k) still include
direction + granularity=line for reliable full-line reads.
oldLine and newLine were declared inside 'if (cachedText && oldPoint > 0)'
block but singleLineMove block referenced them from outside that scope.
Fix: declare singleLineMove = NO before the granularity detection block;
compute adjFwd/adjBwd inside the 'if (oldLine.location != newLine.location)'
branch where both variables are in scope.
Four regressions introduced during review-based refactoring:
1. ZOOM FOCUS JUMPING (P0008 fix in P0000 scope):
ns_accessibility_enabled guard was added to ns_zoom_track_completion,
ns_update_end fallback, and ns_draw_window_cursor. Zoom works
independently of VoiceOver; ns_accessibility_enabled is only set when
a screen reader (VoiceOver) activates the AT layer. Users who use
Zoom without VoiceOver got no cursor tracking at all.
Fix: remove ns_accessibility_enabled from all three Zoom call sites;
guard only with ns_zoom_enabled_p() as in the original.
2. j/k (ANY SINGLE-STEP LINE COMMAND) READS ONLY FIRST WORD:
The code only treated C-n/C-p (isCtrlNP) as sequential line moves.
All other line-movement commands (evil j/k, outline-next-heading,
org-next-visible-heading, etc.) were classified as 'discontiguous'
jumps, causing VoiceOver to re-anchor and read only a word.
Fix: detect single-step moves structurally via NSString line-range
adjacency (NSMaxRange(oldLine) == newLine.location for forward,
NSMaxRange(newLine) == oldLine.location for backward). Any command
that moves exactly one line is sequential --- no command-name
whitelisting needed, no package-specific code.
3. ORG FOLD/UNFOLD NOT REFRESHING VOICEOVER (P0007):
BUF_CHARS_MODIFF misses text-property changes such as 'invisible
used by org-fold-core (org >= 29), outline-mode, hideshow-mode.
Fix: use BUF_MODIFF; cost is acceptable (rebuild only on VoiceOver
queries at human interaction speed, not at redisplay speed).
4. C-n/C-p DROPPED LINE-READ (P0005):
FocusedUIElementChanged posted for ALL emacsMovedCursor moves raced
with AXSelectedTextChanged(granularity=line) and caused VoiceOver
to drop the line-read. Fix: skip FocusedUIElementChanged for
sequential moves (isCtrlNP or singleLineMove).
REGRESSION: fold/unfold in org-mode, outline-mode and hideshow-mode did
not refresh VoiceOver text because ensureTextCache used BUF_CHARS_MODIFF
which is NOT bumped by (put-text-property ... 'invisible), the mechanism
used by modern org-fold-core (org >= 29) and outline-mode to hide text.
VoiceOver would continue reading folded content as if visible, or miss
newly unfolded content entirely, because the text cache was considered
valid despite the visible-text having changed.
Revert ensureTextCache to BUF_MODIFF with an explanatory comment:
- BUF_CHARS_MODIFF is bumped only on character insertions/deletions, not
text-property changes. Fold/unfold uses text properties for visibility.
- BUF_OVERLAY_MODIFF alone is also insufficient: org >= 29 uses text
properties, not overlays, for folding. Also hl-line-mode bumps
BUF_OVERLAY_MODIFF every post-command-hook --- same per-keystroke cost
as BUF_MODIFF, with none of its correctness guarantee.
- BUF_MODIFF cost is acceptable: ensureTextCache is called only when
VoiceOver queries AX properties (human interaction speed, not redisplay
speed). Rebuild cost is O(visible-buffer-text).
Also retain C-n/C-p line-read fix from previous commit (7a0b4f6):
FocusedUIElementChanged excluded for sequential isCtrlNP moves.
When Emacs moves the cursor (emacsMovedCursor=YES), we post
FocusedUIElementChanged on the NSWindow to re-anchor VoiceOver's
browse cursor. For C-n/C-p this notification races with
AXSelectedTextChanged(granularity=line) and causes VoiceOver to
drop the line-read speech.
Arrow key movement works because VoiceOver intercepts those as AX
selection changes (setAccessibilitySelectedTextRange:), making
voiceoverSetPoint=YES and emacsMovedCursor=NO, so no
FocusedUIElementChanged is posted.
Fix: skip FocusedUIElementChanged for sequential C-n/C-p moves
(isCtrlNP). AXSelectedTextChanged with direction=next/previous +
granularity=line is sufficient for VoiceOver to read the new line.
FocusedUIElementChanged is only needed for discontiguous jumps
(]], M-<, isearch, xref etc.) where VoiceOver must re-anchor.
Also merge duplicate comment blocks and fix two compile errors
from a64d24c that Martin caught during testing.
Two bugs introduced during rebase/amend:
1. Stray 'unbind_to (count, Qnil)' in ns_focus (P0000):
A hunk was misplaced into ns_focus where 'count' is not declared.
The comment and unbind_to belonged at the end of ns_zoom_track_completion,
which already has a correct unbind_to. Remove the duplicate from ns_focus.
2. 'voiceoverSetPoint = NO' in EmacsView::initFrameFromEmacs: (P0008):
voiceoverSetPoint is a BOOL ivar of EmacsAXBuffer, not EmacsView.
Setting it in EmacsView's init method causes 'undeclared identifier'.
ObjC BOOL ivars zero-initialize to NO automatically. Remove the line.
voiceoverSetPoint is consumed/set in EmacsAXBuffer methods only.
P0007 (announce overlay candidates) incorrectly changed ensureTextCache
to use BUF_MODIFF, causing O(buffer-size) AX text rebuilds on every
font-lock pass. Reverted to BUF_CHARS_MODIFF throughout.
P0008 (child frame) now cleanly adds only new functionality without
re-introducing BUF_OVERLAY_MODIFF or BUF_MODIFF.
All review blockers and major issues addressed:
- P0000: unbind_to on fall-through path
- P0001: block_input in ns_ax_buffer_text
- P0003: block_input in postCompletionAnnouncementForBuffer; [trims release]
- P0004: block_input in ns_ax_scan_interactive_spans; mojibake ---
- P0006: texinfo semicolons -> periods
- P0007: BUF_CHARS_MODIFF throughout ensureTextCache (no oscillation)
- P0008: childFrameLastBuffer=BVAR(b,name); no BUF_OVERLAY_MODIFF in
ensureTextCache; voiceoverSetPoint init; cachedOverlayModiffForText removed
git am passes all 9 patches on Linux git 2.43.0.
block_input protection moved from P0008 to their respective origin
patches for independent compilability (GNU Emacs requirement):
- P0001 (ns_ax_buffer_text): now has block_input + record_unwind
- P0003 (postCompletionAnnouncementForBuffer): now has block_input
- P0004 (ns_ax_scan_interactive_spans): now has block_input
P0008 now only adds its own new functionality (child frame completion
announcements, echo area announcements) without duplicating block_input
from earlier patches.
All 9 patches apply cleanly with git am on Linux git 2.43.0.
All 9 patches now apply cleanly with git am on Linux (git 2.43.0).
Root cause of previous failures: hunk offsets were systematically wrong
by 7-40 lines; macOS git fuzzy-matched them, Linux did not.
Patches regenerated via git format-patch after applying all changes.
Fixes applied:
- P0000: unbind_to on no-candidate fall-through path; hunk regenerated
- P0001: block_input + record_unwind_protect_void in ns_ax_buffer_text
- P0003: [trims release] MRC memory leak; block_input already present
- P0004: mojibake comment (--- not UTF-8 em-dash)
- P0006: texinfo dangling semicolons -> periods in GNUstep paragraph
- P0007: em-dash fixes removed (content was already --- from P0004/P0005)
- P0008: childFrameLastBuffer -> BVAR(b,name) for GC safety;
BUF_OVERLAY_MODIFF removed from ensureTextCache (hl-line-mode O(N)
rebuild regression); block_input in ns_ax_buffer_text (P0001 scope);
voiceoverSetPoint and childFrameLastBuffer explicit init in
initFrameFromEmacs:; cachedOverlayModiffForText ivar removed
Bug 1 (VO cursor not following Emacs cursor):
- Remove FocusedUIElementChangedNotification on emacsView (was a no-op:
VO re-queried the same element)
- For Emacs-initiated char/word moves, keep natural next/previous
direction instead of forcing discontiguous; SelectedTextChanged with
direction=next advances VO browse cursor sequentially
- Only force discontiguous for line-boundary crossings and large jumps
Bug 2 (word double-read with punctuation):
- Root cause was FocusedUIElementChanged causing VO re-anchor speech
on top of the explicit word announcement
- Removing FocusedUIElementChanged eliminates the duplicate speech
- Add emacsInitiated parameter to postFocusedCursorNotification;
omit AXTextSelectionGranularity for Emacs-initiated moves so VO
does not auto-speak (only explicit announcements provide speech)
- isWordMove now triggers on emacsInitiated flag (Emacs-initiated
word moves always get explicit announcement)
- Issue 1: Add explicit ApplicationServices import for UAZoomEnabled/
UAZoomChangeFocus (was implicit via Carbon.h, now explicit)
- Issue 2: Rename FOR_EACH_FRAME variable 'frames' -> 'frame' (plural
was misleading; matches Emacs convention)
- Issue 3: Move unblock_input before ObjC calls in
postCompletionAnnouncementForBuffer: to avoid holding block_input
during @synchronized operations
- Issue 4: Fix DEFVAR_BOOL doc and Texinfo: initial value is nil,
not t; auto-detection sets it at startup
- Issue 5: Replace magic 10000 with NS_AX_MAX_COMPLETION_BUFFER_CHARS
constant with explanatory comment
- Issue 6: Add comment to lineStartOffsets loop explaining it is gated
on BUF_CHARS_MODIFF and never runs on the hot path
- Issue 8: Rewrite all 9 commit messages to GNU ChangeLog format with
'* file (symbol): description' entries
- Issue 9: Break 81-char @interface line in nsterm.h
- Issue 10: Add WINDOWP/BUFFERP guards before dereferencing
cf->selected_window and cw->contents in ns_zoom_find_child_frame_candidate
- Issue 11: Fix @pxref -> @xref at sentence start in macos.texi
charposForAccessibilityIndex: walked composed character sequences
from the start of a visible run to the target AX index. For a run
covering an entire ASCII buffer, this is O(cursor_position): moving
to line 10,000 requires ~500,000 iterations per call.
The method is called on every SelectedTextChanged notification
response (accessibilityBoundsForRange: from the AX server for cursor
tracking), making cursor movement O(position) in large files.
Fix: when ax_length == length for a run (all characters are single
AX index units — true for all ASCII/Latin text), the charpos offset
is simply ax_idx - run.ax_start. O(1) instead of O(position).
Multi-byte runs (emoji, CJK, non-BMP) fall back to the sequence walk,
bounded by run length (visible window size), not total buffer size.
Root cause: the 50ms rate limit broke child-frame (Corfu) tracking.
When the Corfu child frame redraws, its ns_update_end fires first and
resets the rate-limit timer. When the parent frame's ns_update_end
fires immediately after, the timer has not expired, so
ns_zoom_track_completion returns early without scanning child frames.
Zoom focus stays on the first candidate.
Fix: remove the rate limit; add a FRAME_PARENT_FRAME(f) guard instead.
Child frames have no completion children to scan; their parent's
ns_update_end does the scan via FOR_EACH_FRAME. Returning early on
child-frame calls avoids the redundant scan and leaves the timer
problem moot. Overhead without the rate limit is ~40 Lisp evaluations
per redisplay (~5-20 µs), acceptable given ns_zoom_enabled_p() already
caches the UAZoomEnabled() IPC call.
Fget_char_property with a buffer as OBJECT checks text properties
only. Corfu highlights the selected candidate (corfu-current) via
an overlay, not a text property, so the scan always returned -1 and
Zoom focus stayed on the first line.
Pass cf->selected_window instead of cw->contents so that overlays
are included in the property lookup. Vertico uses text properties
and is unaffected; child-frame completion frameworks that use overlays
(Corfu, Company-box) now track correctly.
Changes:
- EmacsApp gets ns_update_accessibility_state and
ns_accessibility_did_change: methods (patch 0005)
- At startup: UAZoomEnabled() + AXIsProcessTrustedWithOptions()
determine initial ns_accessibility_enabled state
- com.apple.accessibility.api distributed notification updates it
whenever any AT connects or disconnects
- All Zoom call sites (UAZoomChangeFocus) now gated by
ns_accessibility_enabled in addition to ns_zoom_enabled_p()
- ns-accessibility-enabled docstring updated to describe auto-detect
Result: zero config needed; zero overhead when no AT is active;
single variable overrides auto-detection when needed.
500ms (2 Hz) was too aggressive — Zoom focus stopped updating during
keyboard navigation in Vertico/Corfu lists. 50ms (20 Hz) tracks
fast arrow-key navigation while still avoiding per-frame overhead.
UAZoomEnabled() is already cached so the main cost is the overlay
scan, which is cheap.
The Qnil initialization was in patch 0000 (Zoom) but the ivar
declaration is in patch 0008 (child frame tracking). Moved the
init to patch 0008 so each patch compiles independently.
Three fixes:
1. Patch 0000 now compiles standalone: replaced forward declaration
of ns_ax_face_is_selected (defined in VoiceOver patches) with
self-contained ns_zoom_face_is_selected in the Zoom patch.
2. ns_accessibility_enabled defaults to nil: eliminates ALL VoiceOver
overhead (text cache rebuild, AX notifications, Mach IPC to AX
server) when VoiceOver is not in use. Zero per-redisplay cost.
Enable with (setq ns-accessibility-enabled t).
3. UAZoomEnabled() cached for 1s + ns_zoom_track_completion rate-
limited to 2Hz: eliminates 150-600µs/frame of IPC overhead.
Root cause (per Opus analysis): UAZoomEnabled() is a synchronous
Mach IPC roundtrip to macOS Accessibility server, called 3x per
redisplay cycle. At 60fps = 180 IPC roundtrips/second blocking the
main thread. Combined with Emacs's inherent O(position) redisplay
cost, this compounded into progressive choppy behavior.
Fix 1: ns_zoom_enabled_p() caches UAZoomEnabled() for 1 second.
Fix 2: ns_zoom_track_completion() rate-limited to 2 Hz.
Also includes BUF_CHARS_MODIFF fix (patch 0009) for VoiceOver cache.
BUF_CHARS_MODIFF fix — the core performance regression:
ensureTextCache checked BUF_MODIFF which font-lock bumps on every
redisplay. Each cursor movement in a large file triggered full buffer
rebuild. Now uses BUF_CHARS_MODIFF (changes only on char insert/delete).
Performance issue: editing large files (>~10KB, >2000 lines) caused
progressive slowdown regardless of VoiceOver status.
Root causes:
1. ns_zoom_find_overlay_candidate_line: called Foverlays_in on the
entire visible buffer range on every redisplay when UAZoomEnabled().
In files with many overlays (font-lock, hl-line, show-paren etc.)
this was O(overlays) Lisp work per keystroke.
2. postAccessibilityNotificationsForFrame: when ns-accessibility-enabled
is non-nil, checked BUF_OVERLAY_MODIFF every redisplay. font-lock
bumps this on every redraw, triggering ns_ax_selected_overlay_text
(another O(overlays) scan) for non-minibuffer windows.
Fix: Both scans now guard with MINI_WINDOW_P check. Overlay completion
frameworks (Vertico, Icomplete, Ivy) only display candidates in
minibuffer windows --- no completion framework puts selected-face
overlays in normal editing buffers. For non-minibuffer windows both
functions return immediately with zero Lisp calls.
Additionally: ns_zoom_find_child_frame_candidate is skipped when
f->child_frame_list is nil (no child frames = no Corfu popup).
Zoom patch 0000 now tracks completion candidates:
- Overlay: Vertico, Icomplete, Ivy (face heuristic on before-string)
- Child frame: Corfu, Company-box (scan buffer text for selected face)
Also fixes duplicate lastCursorRect ivar when applied with VoiceOver.
Zoom (0000) declares lastCursorRect @public in EmacsView.
VoiceOver (0005) was re-declaring it, causing 'duplicate member'
compiler error when both applied together. Removed the duplicate.
VoiceOver patches 0001-0008 now apply cleanly on top of Zoom patch
0000. The full set (git am patches/000*.patch) works without
conflicts. Patch 0005 (integration) merges Zoom fallback and
VoiceOver postAccessibilityUpdates in ns_update_end.
Fixes from Opus maintainer review:
1. [BLOCKER] Zoom code completely removed from ALL intermediate patches
(0005-0007 no longer have UAZoom/overlayZoom at any commit point)
2. [BLOCKER] Unified cursor rect ivar: lastCursorRect (was split
between lastZoomCursorRect and lastAccessibilityCursorRect)
3. [HIGH] Child frame static vars moved to EmacsView ivars
(childFrameLastCandidate/Buffer/Modiff — no cross-frame interference)
4. [HIGH] intern_c_string replaced with Qbefore_string/Qafter_string
5. [MEDIUM] Zoom fallback gated by zoomCursorUpdated flag (no double call)
Major changes:
1. Zoom separated into standalone patch 0000
- UAZoomChangeFocus in ns_draw_window_cursor
- Fallback in ns_update_end for window-switch tracking
- No overlayZoomActive (source of split/switch/move bug)
2. VoiceOver patches 0001-0008 are now Zoom-free
- All UAZoom*, overlayZoom*, kUAZoomFocus references removed
- lastAccessibilityCursorRect kept for VoiceOver bounds queries
- Commit messages cleaned of Zoom references
3. README.txt and TESTING.txt rewritten for new structure
Addresses reviewer (Stéphane Marks) feedback:
- Keep Zoom patch separate from VoiceOver work
- Design discussion needed for non-Zoom patches
- Performance: ns-accessibility-enabled=nil for zero overhead
