From 083b4a4acdaa69eabc9e1f66dbaf4633e2be6a8b Mon Sep 17 00:00:00 2001 From: Daneel Date: Mon, 2 Mar 2026 10:10:36 +0100 Subject: [PATCH] Fix crash and cursor sync in accessibility patches 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. --- ...tification-dispatch-and-mode-line-el.patch | 14 ++-- ...d-frame-completion-candidates-for-Vo.patch | 77 ++++++++----------- 2 files changed, 42 insertions(+), 49 deletions(-) diff --git a/patches/0003-ns-add-buffer-notification-dispatch-and-mode-line-el.patch b/patches/0003-ns-add-buffer-notification-dispatch-and-mode-line-el.patch index c6ee38a..9e7f305 100644 --- a/patches/0003-ns-add-buffer-notification-dispatch-and-mode-line-el.patch +++ b/patches/0003-ns-add-buffer-notification-dispatch-and-mode-line-el.patch @@ -107,12 +107,14 @@ index 3e1ac74..d3015e2 100644 + = @(ns_ax_text_state_change_selection_move); + moveInfo[@"AXTextSelectionDirection"] = @(direction); + moveInfo[@"AXTextChangeElement"] = self; -+ /* Omit granularity for character moves so VoiceOver does not -+ derive its own speech (it would read the wrong character -+ for evil block-cursor mode). Include it for word/line/ -+ selection so VoiceOver reads the appropriate text. */ -+ if (!isCharMove) -+ moveInfo[@"AXTextSelectionGranularity"] = @(granularity); ++ /* Always include granularity so VoiceOver can advance its browse ++ cursor by the correct unit (character, word, or line). Without ++ granularity, VO leaves its browse cursor at the previous position ++ for character moves, breaking Emacs-cursor → VO-cursor sync. ++ For character moves, the explicit AnnouncementRequested below ++ (High priority) overrides VO's auto-speech, so evil block-cursor ++ mode still reads the correct character. */ ++ moveInfo[@"AXTextSelectionGranularity"] = @(granularity); + + ns_ax_post_notification_with_info ( + self, diff --git a/patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch b/patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch index 6ccce5f..e8f3b0e 100644 --- a/patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch +++ b/patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch @@ -21,7 +21,7 @@ element when a child frame completion closes. etc/NEWS | 18 +- src/nsterm.h | 21 ++ src/nsterm.m | 496 +++++++++++++++++++++++++++++++++++++++---- - 4 files changed, 501 insertions(+), 52 deletions(-) + 4 files changed, 491 insertions(+), 52 deletions(-) diff --git a/doc/emacs/macos.texi b/doc/emacs/macos.texi index 6514dfc..bcf74b3 100644 @@ -427,22 +427,16 @@ index 8d44b5f..29b646d 100644 if (cachedText && granularity == ns_ax_text_selection_granularity_line) { -@@ -9175,7 +9314,14 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b +@@ -9175,7 +9314,8 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b ptrdiff_t currentOverlayStart = 0; ptrdiff_t currentOverlayEnd = 0; + block_input (); specpdl_ref count2 = SPECPDL_INDEX (); -+ /* Register unblock_input as an unwind action so that if any Lisp -+ call below signals (triggering a longjmp through unbind_to), -+ block_input is always paired with an unblock_input. The explicit -+ unblock_input() at the end of the function is still needed for -+ the normal (non-signal) path. */ -+ record_unwind_protect_void (unblock_input); record_unwind_current_buffer (); if (b != current_buffer) set_buffer_internal_1 (b); -@@ -9352,12 +9498,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f +@@ -9352,12 +9492,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f if (!b) return; @@ -472,7 +466,7 @@ index 8d44b5f..29b646d 100644 if (modiff != self.cachedModiff) { self.cachedModiff = modiff; -@@ -9371,6 +9534,7 @@ Text property changes (e.g. face updates from +@@ -9371,6 +9528,7 @@ Text property changes (e.g. face updates from { self.cachedCharsModiff = chars_modiff; [self postTextChangedNotification:point]; @@ -480,7 +474,7 @@ index 8d44b5f..29b646d 100644 } } -@@ -9393,8 +9557,15 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property +@@ -9393,8 +9551,15 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property displayed in the minibuffer. In normal editing buffers, font-lock and other modes change BUF_OVERLAY_MODIFF on every redisplay, triggering O(overlays) work per keystroke. @@ -498,54 +492,51 @@ index 8d44b5f..29b646d 100644 goto skip_overlay_scan; int selected_line = -1; -@@ -9440,7 +9611,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property +@@ -9440,7 +9605,19 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property self.cachedPoint = point; self.cachedMarkActive = markActive; - /* Compute direction. */ + /* Compute direction. -+ When VoiceOver moved the cursor via setAccessibilitySelectedTextRange: -+ (voiceoverSetPoint == YES), use sequential next/previous so VoiceOver -+ continues smooth navigation from its current position. -+ When Emacs moved the cursor independently (voiceoverSetPoint == NO), -+ force discontiguous direction so VoiceOver re-anchors its browse -+ cursor to accessibilitySelectedTextRange; without this, VoiceOver's -+ internal browse position diverges from the Emacs insertion point and -+ subsequent VO+arrow navigation starts from the wrong location. */ ++ voiceoverSetPoint distinguishes who moved the cursor: ++ - YES (VoiceOver via setAccessibilitySelectedTextRange:): ++ keep sequential next/previous so VO tracks smoothly. ++ - NO (Emacs via keyboard command or ELisp): ++ for cross-line jumps that are not C-n/C-p, force ++ discontiguous so VoiceOver re-anchors its browse cursor ++ to accessibilitySelectedTextRange. ++ Character/word moves within a line always stay sequential ++ so VoiceOver tracks C-f/C-b/M-f/M-b naturally. */ + BOOL emacsMovedCursor = !voiceoverSetPoint; + voiceoverSetPoint = NO; /* Consume the flag. */ + NSInteger direction = ns_ax_text_selection_direction_discontiguous; if (point > oldPoint) direction = ns_ax_text_selection_direction_next; -@@ -9488,6 +9670,26 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property +@@ -9488,6 +9664,22 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property granularity = ns_ax_text_selection_granularity_line; } + /* Programmatic jumps that cross a line boundary (]], [[, M-<, + xref, imenu, …) are discontiguous: the cursor teleported to an + arbitrary position, not one sequential step forward/backward. -+ Reporting AXTextSelectionDirectionDiscontiguous causes VoiceOver -+ to re-anchor its rotor browse cursor at the new -+ accessibilitySelectedTextRange rather than advancing linearly -+ from its previous internal position. */ -+ if (!isCtrlNP && granularity == ns_ax_text_selection_granularity_line) -+ direction = ns_ax_text_selection_direction_discontiguous; -+ -+ /* If Emacs moved the cursor (not VoiceOver), force discontiguous -+ so VoiceOver re-anchors its browse cursor to the current -+ accessibilitySelectedTextRange. This covers all Emacs-initiated -+ moves: editing commands, ELisp, isearch, etc. -+ Exception: C-n/C-p (isCtrlNP) already uses next/previous with -+ line granularity; those are already sequential and VoiceOver -+ handles them correctly. */ -+ if (emacsMovedCursor && !isCtrlNP) ++ All three conditions must hold: ++ - emacsMovedCursor: VoiceOver-initiated moves (via ++ setAccessibilitySelectedTextRange:) keep sequential ++ direction so VO can manage its own browse cursor. ++ - !isCtrlNP: C-n/C-p (and arrow up/down, which also bind ++ next-line/previous-line) are sequential line moves. ++ - granularity == line: only cross-line jumps qualify; ++ character and word moves within a line stay sequential ++ so VoiceOver tracks them naturally (C-f/C-b, M-f/M-b). */ ++ if (emacsMovedCursor && !isCtrlNP ++ && granularity == ns_ax_text_selection_granularity_line) + direction = ns_ax_text_selection_direction_discontiguous; + /* Post notifications for focused and non-focused elements. */ if ([self isAccessibilityFocused]) [self postFocusedCursorNotification:point -@@ -9630,6 +9832,17 @@ - (NSRect)accessibilityFrame +@@ -9630,6 +9826,17 @@ - (NSRect)accessibilityFrame if (vis_start >= vis_end) return @[]; @@ -563,7 +554,7 @@ index 8d44b5f..29b646d 100644 /* Symbols are interned once at startup via DEFSYM in syms_of_nsterm; reference them directly here (GC-safe, no repeated obarray lookup). */ -@@ -9750,6 +9963,7 @@ than O(chars). Fall back to pos+1 as safety net. */ +@@ -9750,6 +9957,7 @@ than O(chars). Fall back to pos+1 as safety net. */ pos = span_end; } @@ -571,7 +562,7 @@ index 8d44b5f..29b646d 100644 return [[spans copy] autorelease]; } -@@ -9931,6 +10145,10 @@ - (void)dealloc +@@ -9931,6 +10139,10 @@ - (void)dealloc #endif [accessibilityElements release]; @@ -582,7 +573,7 @@ index 8d44b5f..29b646d 100644 [[self menu] release]; [super dealloc]; } -@@ -11380,6 +11598,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f +@@ -11380,6 +11592,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f windowClosing = NO; processingCompose = NO; @@ -592,7 +583,7 @@ index 8d44b5f..29b646d 100644 scrollbarsNeedingUpdate = 0; fs_state = FULLSCREEN_NONE; fs_before_fs = next_maximized = -1; -@@ -12688,6 +12909,152 @@ - (id)accessibilityFocusedUIElement +@@ -12688,6 +12903,152 @@ - (id)accessibilityFocusedUIElement The existing elements carry cached state (modiff, point) from the previous redisplay cycle. Rebuilding first would create fresh elements with current values, making change detection impossible. */ @@ -672,9 +663,9 @@ index 8d44b5f..29b646d 100644 + This prevents redundant work on every redisplay tick and + also guards against re-entrance: if Lisp calls below + trigger redisplay, the modiff check short-circuits. */ -+ EMACS_INT modiff = BUF_MODIFF (b); + if (!BUFFER_LIVE_P (b)) + return; ++ EMACS_INT modiff = BUF_MODIFF (b); + /* Compare buffer identity using the raw pointer, not a Lisp_Object. + A killed buffer can be GC'd even if we hold a Lisp_Object for it + (EmacsView is not GC-visible). Storing and comparing struct buffer * @@ -745,7 +736,7 @@ index 8d44b5f..29b646d 100644 - (void)postAccessibilityUpdates { NSTRACE ("[EmacsView postAccessibilityUpdates]"); -@@ -12698,11 +13065,64 @@ - (void)postAccessibilityUpdates +@@ -12698,12 +13059,64 @@ - (void)postAccessibilityUpdates /* Re-entrance guard: VoiceOver callbacks during notification posting can trigger redisplay, which calls ns_update_end, which calls us