BUF_MODIFF was needed for fold/unfold correctness (org-mode), but
ensureTextCache was called from postAccessibilityNotificationsForFrame:
on every cursor move, causing O(buffer-size) rebuild on every font-lock
pass.
Fix: remove [self ensureTextCache] from the granularity-detection branch
of the notification path. The granularity detection now uses cachedText
directly, falling back to granularity_unknown when absent (safe: VoiceOver
makes its own determination). ensureTextCache is called exclusively from
AX getters (human interaction speed). Font-lock passes no longer trigger
any cache rebuild.
The BUF_MODIFF validity in ensureTextCache is retained (correct for
fold/unfold). Comment updated to accurately describe the calling pattern.
All 9 patches apply cleanly on fresh base (git apply verified).
- Blocker #1: add BOOL emacsMovedCursor = YES in patch 0005 so series
compiles standalone; patch 0008 replaces it with !voiceoverSetPoint
- Blocker #3: change all Daneel authorship to Martin Sukany
- Zoom gate: remove ns_accessibility_enabled guards from Zoom code
paths (0005 no longer adds them; 0008 retains the clarifying comment)
- eassert: remove redundant BUFFER_LIVE_P eassert with contradictory
comment in patch 0008
- macos.texi: integrate orphan 'Block-style cursors' paragraph as
@item in the Known Limitations list
- cindex: restore @cindex Zoom, cursor tracking (macOS) removed in 0008
- ChangeLog 0002: list only functions actually added in that patch
- ChangeLog 0008: accurate description (remove wrong BUF_CHARS_MODIFF
claim for ensureTextCache; ns_ax_buffer_text block_input was in 0001)
- git apply --check: all 9 patches apply cleanly on fresh base
Remaining open issue: BUF_MODIFF regression in patch 0007 (ensureTextCache
O(N) rebuild per font-lock pass) requires design decision before
submission.
The block_input/record_unwind_protect_void ordering fix and its
explanatory comment belong in 0001 (ns_ax_buffer_text), which is
where the pattern is first introduced. Having the fragile pattern
visible across patches 0001-0007 invites review comments.
Remove the now-redundant block_input reordering hunk from 0008;
0001 already establishes the correct order with the comment.
0008 was already fixing the block_input/record_unwind_protect_void
ordering with a detailed comment. My earlier change to 0001 broke
0008's context match. Let the series fix it in the right place.
- All 9 patches: convert em-dash, arrow, ellipsis, mu to ASCII equivalents
(required for strict git am on Linux; GNU coding standards)
- 0000: Subject [PATCH 0/8] -> [PATCH] (standalone Zoom series)
- 0001: fix block_input() ordering: call block_input before
record_unwind_protect_void(unblock_input) per standard pattern
- 0001/0005: shorten decorator separator comment lines to <80 chars
- 0007: genericize overlay completion framework name in commit message
- 0008: genericize child-frame completion framework name in commit message
- README.txt: fix stale 'default t' to 'default nil' with auto-detection note
- TESTING.txt: fix stale 'defaults to t' to 'defaults to nil'
M-REM-1 - Stack array bound comment:
ns_ax_selected_child_frame_text uses line_starts/line_ends[128].
Added comment documenting the 128-line bound and the 10,000-character
upstream guard that makes silent truncation safe in practice.
M-REM-2 - Remove dead BUFFER_LIVE_P check:
The second BUFFER_LIVE_P check in announceChildFrameCompletion (after
updating childFrameLastBuffer/Modiff) is unreachable: no code between
the entry check and this point can kill the buffer. Replaced with
eassert so DEBUG builds catch violations while releasing dead code.
M-REM-3 - @interface declaration for postEchoAreaAnnouncementIfNeeded:
Added declaration to the #ifdef NS_IMPL_COCOA accessibility block in
nsterm.h alongside existing postAccessibilityUpdates and
announceChildFrameCompletion declarations. postAccessibilityUpdates
is called from ns_update_end; all three methods should be declared.
L-REM-4 - Commit message class name:
Changed (EmacsAXBuffer) to (EmacsAccessibilityBuffer) in the subject
line. EmacsAXBuffer does not exist; EmacsAccessibilityBuffer is the
actual class that received the voiceoverSetPoint ivar.
NEWS - Remove third-party package names:
Replaced 'Vertico, Icomplete, Ivy for overlay candidates; Corfu,
Company-box for child frame popups' with generic descriptions per GNU
Emacs NEWS convention (third-party package names avoided in NEWS).
C1 - block_input ordering in ns_ax_buffer_text:
block_input() now called before record_unwind_protect_void(unblock_input).
Previously the unwind handler could have been called without a matching
block_input, corrupting the input-blocking reference count.
C2 - unbind_to missing in patch 0004:
unbind_to(blk_count, Qnil) moved from patch 0008 to patch 0004 so that
ns_ax_scan_interactive_spans has a complete block_input/unbind_to pair
when patches 0000-0004 are applied independently.
H1 - Zoom patch forward dependency on VoiceOver:
Removed forward declaration 'static bool ns_ax_face_is_selected' and
the delegation from ns_zoom_face_is_selected. Restored standalone
implementation of ns_zoom_face_is_selected in the Zoom patch so patch
0000 compiles and links independently of the VoiceOver patches.
H2 - ns_accessibility_enabled removal undocumented:
Added comment to ns_zoom_track_completion explaining that Zoom cursor
tracking is gated only on ns_zoom_enabled_p(), not ns_accessibility_enabled.
Users running Zoom without VoiceOver must still get completion tracking.
M5 - childFrameLastBuffer GC safety undocumented:
Added comment at the assignment site explaining why BVAR(b, name) (an
interned symbol reachable from obarray) is GC-safe without staticpro.
M6 - FOR_EACH_FRAME without block_input:
Added block_input/unblock_input around the FOR_EACH_FRAME loop in
postAccessibilityUpdates that checks for visible child frames.
Vframe_list must not be modified by timers or process sentinels
during iteration.
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
VoiceOver's browse cursor was not following Emacs cursor moves because
FocusedUIElementChangedNotification was posted on the buffer element
(a custom NSObject subclass), which VoiceOver ignores for browse cursor
re-anchoring.
Three changes:
1. Post FocusedUIElementChanged on the NSWindow instead of self, causing
VoiceOver to call accessibilityFocusedUIElement on the window and
re-anchor its browse cursor at the returned buffer element.
2. Post NSAccessibilityLayoutChangedNotification on self.emacsView with
NSAccessibilityUIElementsKey pointing to the buffer element, causing
VoiceOver to re-examine the element's properties including
accessibilitySelectedTextRange.
3. Post SelectedTextChangedNotification from self.emacsView (an NSView)
instead of self (NSObject), with NSAccessibilityUIElementsKey, so
VoiceOver processes text-change notifications from a view-based
element in the hierarchy.
The notification was posted on self.emacsView (the NSView container).
VoiceOver called accessibilityFocusedUIElement on the view, got back
the same buffer element, and did not re-anchor its browse cursor.
Fix: post on self (the buffer element itself). When VoiceOver receives
NSAccessibilityFocusedUIElementChangedNotification from an element that
is already focused, it silently re-queries accessibilitySelectedTextRange
and re-anchors its browse cursor without re-announcing the element name.
Covers all emacsMovedCursor granularities (arrow keys, M-f/M-b, C-n/C-p
via the separate isCtrlNP path).
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)
Removing the evil-mode EQ conditions left the if() without its
closing paren. Fix:
if (EQ (cmd, Qns_ax_next_line)
|| EQ (cmd, Qns_ax_dired_next_line)) <- add ')'
Same for previous_line. Verified: all 9 patches apply and build.
Commit 7609922 changed patch 0003 comment from
'for evil block-cursor mode' to 'for block-cursor mode'.
Patch 0008 had a '-' (removal) line still referencing 'for evil
block-cursor mode', which git am could not find in the file ->
'patch does not apply'.
Verified locally: git am now applies all 9 patches cleanly on f0dbe25.
Adding 4 lines in patch 0003 (punctuation trim fix) shifted all
subsequent nsterm.m positions by 4. Update the @@ -NNNN offsets
for all 14 nsterm.m hunks at line >= 9060 in patch 0008.
patch 0002: Do not activate mark in setAccessibilitySelectedTextRange.
VoiceOver range.length is an internal word-boundary hint, not a text
selection. Activating the mark made accessibilitySelectedTextRange
return a non-zero length, causing VoiceOver to position its browse
cursor at the END of the selection instead of the START.
patch 0003: Fix word announcement double-read and punctuation.
- Explicit word announcement only for Emacs-initiated (discontiguous)
moves; VO-initiated (sequential) word navigation relies on VO
auto-speech from the granularity=word notification, preventing
double-read.
- Strip trailing/leading punctuation from word announcements using
punctuationCharacterSet so 'Ahoj,' is announced as 'Ahoj'.
patch 0008: Post FocusedUIElementChangedNotification on the EmacsView
(not just on line-granularity moves) for all Emacs-initiated cursor
movements. Posting on the view causes VoiceOver to re-query
accessibilityFocusedUIElement and re-anchor its browse cursor at the
current accessibilitySelectedTextRange. Also fixes a pre-existing
off-by-one in the macos.texi hunk header.
Bug 1 (crash): postEchoAreaAnnouncementIfNeeded called Fbuffer_string()
without block_input, allowing timer events to interleave with Lisp
calls and corrupt buffer state. Added block_input/unblock_input via
record_unwind_protect_void. Also removed unpaired unblock_input()
in postCompletionAnnouncementForBuffer (patch 0003) that became a
double-unblock after patch 0008 added block_input + unwind protect.
Bug 2 (cursor sync): VoiceOver browse cursor did not follow Emacs
keyboard cursor because SelectedTextChanged with discontiguous
direction alone is not sufficient for VoiceOver to re-anchor. Added
NSAccessibilityFocusedUIElementChangedNotification post when Emacs
moves the cursor across a line boundary, forcing VoiceOver to re-query
accessibilitySelectedTextRange.
Bug 3 (word off-by-one): Evil w (next-word) read the previous word
instead of the destination word. VO auto-speech from
SelectedTextChanged with direction=next+granularity=word reads the
traversed word, not the arrived-at word. Added explicit word
announcement (like char moves) that reads the word AT the new cursor
position using whitespace-delimited word boundary scan.
BUF_MODIFF(b) dereferences the struct buffer pointer unconditionally.
If the buffer was killed, this accesses freed memory and crashes.
Check BUFFER_LIVE_P first.
Use precise Python line-index swap instead of Edit tool to avoid
accidentally replacing other patch content.
Patch 0008 modifies postFocusedCursorNotification to add
'&& direction != discontiguous' to the isCharMove guard, which IS
the cursor sync fix. Changing patch 0003 independently broke
patch 0008's context match. Restore patch 0003 to original.
Patch 0008: Move BUFFER_LIVE_P check before BUF_MODIFF dereference
in announceChildFrameCompletion. Accessing BUF_MODIFF on a killed
buffer is a null/garbage dereference.
Patch 0003: Always include AXTextSelectionGranularity in
postFocusedCursorNotification, including for character-granularity
moves. Without granularity, VoiceOver leaves its browse cursor at
the previous position on C-f/C-b/arrow moves. The explicit
AnnouncementRequested (High priority) still overrides VO speech
for evil block-cursor correctness.
- Correct three pre-existing DEFVAR_BOOL doc strings (ns-use-native-
fullscreen, ns-use-mwheel-acceleration, ns-use-mwheel-momentum) that
were accidentally modified by an earlier rebase; restore original text
- Break @property line in nsterm.h to 79 chars
- Replace em-dash with --- and break 80-char comment line in patch 0004
