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.
This commit is contained in:
@@ -107,11 +107,13 @@ 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)
|
||||
+ /* 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 (
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user