patches: fix all review issues from Opus core maintainer review

BLOCKERS fixed:
- #1: add missing skip_overlay_scan: label (goto without target)
- #2: move accessibilityLineForIndex: to patch 0002 (used in 0002/0003,
  was defined in 0007 — forward reference crash risk)
- #3: move BOOL singleLineMove to patch 0008 (declared in 0007, unused
  until 0008 — -Werror build failure)

WARNINGS fixed:
- #1: block_input ordering at 5 sites (record_unwind_protect_void must
  come AFTER block_input to avoid unmatched unblock_input on error)
- #2: deduplicate ns_ax_face_is_selected (move to patch 0001, remove
  from patch 0007 where it was duplicated)
- #4: improve childFrameLastBuffer comment (document rename edge case)
- #5: complete ChangeLog for patch 0007 (add 3 missing method entries)
- #6: document lastSelectedWindow/lastRootWindow GC safety in nsterm.h

MINOR fixed:
- #2: wrap ns_zoom_track_completion call in MAC_OS_X_VERSION >= 101000
  guard (consistent with the function definition)
- #3: @cindex Zoom verified present in VoiceOver section (already OK)
- #4: raise line_starts/line_ends bound from 128 to 512 (consistent
  with ns_ax_selected_overlay_text)
- #5: add echo_area_buffer[] lifetime comment referencing xdisp.c

All 9 patches apply cleanly (git apply verified sequentially).
This commit is contained in:
2026-03-03 20:43:56 +01:00
parent f42e799991
commit 97c14a3bd9
8 changed files with 320 additions and 144 deletions

View File

@@ -1,4 +1,4 @@
From 217177caefc709c37ae04732ec595ace903e4cc4 Mon Sep 17 00:00:00 2001
From b87fb2b1824761fe3d91a27afe966eada39c1c45 Mon Sep 17 00:00:00 2001
From: Martin Sukany <martin@sukany.cz>
Date: Mon, 2 Mar 2026 18:39:46 +0100
Subject: [PATCH 8/9] ns: announce overlay completion candidates for VoiceOver
@@ -7,10 +7,13 @@ Completion frameworks such as Vertico, Ivy, and Icomplete render
candidates via overlay before-string/after-string properties. Without
this change VoiceOver cannot read overlay-based completion UIs.
* src/nsterm.m (ns_ax_face_is_selected): New static function; matches
'current', 'selected', 'selection' in face symbol names.
(ns_ax_selected_overlay_text): New function; scan overlay strings in
the window for a line with a selected face; return its text.
* src/nsterm.m (ns_ax_selected_overlay_text): New function; scan
overlay strings in the window for a line with a selected face; return
its text.
(accessibilityStringForRange:, accessibilityAttributedStringForRange:)
(accessibilityRangeForLine:): New NSAccessibility protocol methods.
Moved here from planned patch 0008 to keep the AX protocol interface
complete before notification logic uses it.
(ensureTextCache): Switch cache-validity counter from BUF_CHARS_MODIFF
to BUF_MODIFF. Fold/unfold commands (org-mode, outline-mode,
hideshow-mode) change the 'invisible text property via
@@ -27,11 +30,11 @@ granularity_unknown when the cache is absent), so font-lock passes
cannot trigger O(buffer-size) rebuilds via the notification path.
---
src/nsterm.h | 1 +
src/nsterm.m | 358 ++++++++++++++++++++++++++++++++++++++++++++-------
2 files changed, 316 insertions(+), 43 deletions(-)
src/nsterm.m | 384 ++++++++++++++++++++++++++++++++++++++++-----------
2 files changed, 306 insertions(+), 79 deletions(-)
diff --git a/src/nsterm.h b/src/nsterm.h
index f245675513..a210ceba14 100644
index 4bf79a9adb..72ca210bb0 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -510,6 +510,7 @@ typedef struct ns_ax_visible_run
@@ -43,7 +46,7 @@ index f245675513..a210ceba14 100644
@property (nonatomic, assign) BOOL cachedMarkActive;
@property (nonatomic, copy) NSString *cachedCompletionAnnouncement;
diff --git a/src/nsterm.m b/src/nsterm.m
index a0419bb5df..b9d3a0eb53 100644
index e4e43dd7a3..c9fe93a57b 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7263,11 +7263,154 @@ Accessibility virtual elements (macOS / Cocoa only)
@@ -202,7 +205,7 @@ index a0419bb5df..b9d3a0eb53 100644
static NSString *
ns_ax_buffer_text (struct window *w, ptrdiff_t *out_start,
ns_ax_visible_run **out_runs, NSUInteger *out_nruns)
@@ -7340,7 +7483,7 @@ Accessibility virtual elements (macOS / Cocoa only)
@@ -7343,7 +7486,7 @@ Accessibility virtual elements (macOS / Cocoa only)
/* Extract this visible run's text. Use
Fbuffer_substring_no_properties which correctly handles the
@@ -211,7 +214,7 @@ index a0419bb5df..b9d3a0eb53 100644
include garbage bytes when the run spans the gap position. */
Lisp_Object lstr = Fbuffer_substring_no_properties (
make_fixnum (pos), make_fixnum (run_end));
@@ -7421,7 +7564,7 @@ Mode lines using icon fonts (e.g. nerd-font icons)
@@ -7424,7 +7567,7 @@ Mode lines using icon fonts (e.g. nerd-font icons)
return NSZeroRect;
/* charpos_start and charpos_len are already in buffer charpos
@@ -220,7 +223,40 @@ index a0419bb5df..b9d3a0eb53 100644
charposForAccessibilityIndex which handles invisible text. */
ptrdiff_t cp_start = charpos_start;
ptrdiff_t cp_end = cp_start + charpos_len;
@@ -7896,6 +8039,7 @@ @implementation EmacsAccessibilityBuffer
@@ -7606,31 +7749,7 @@ already on the main queue (e.g., inside postAccessibilityUpdates
freeing the main queue for VoiceOver's dispatch_sync calls. */
/* Return true if FACE (a symbol or list of symbols) looks like a
- "selected item" face. Substring match is intentionally broad ---
- it catches vertico-current, icomplete-selected-match,
- ivy-current-match, company-tooltip-selection, and similar.
- False positives are harmless: this runs only on overlay/child-frame
- strings during completion, never in a hot redisplay path. */
-static bool
-ns_ax_face_is_selected (Lisp_Object face)
-{
- if (SYMBOLP (face) && !NILP (face))
- {
- const char *name = SSDATA (SYMBOL_NAME (face));
- if (strstr (name, "current") || strstr (name, "selected")
- || strstr (name, "selection"))
- return true;
- }
- if (CONSP (face))
- {
- for (Lisp_Object tail = face; CONSP (tail); tail = XCDR (tail))
- if (ns_ax_face_is_selected (XCAR (tail)))
- return true;
- }
- return false;
-}
-
-static inline void
+ static inline void
ns_ax_post_notification (id element,
NSAccessibilityNotificationName name)
{
@@ -7924,6 +8043,7 @@ @implementation EmacsAccessibilityBuffer
@synthesize cachedOverlayModiff;
@synthesize cachedTextStart;
@synthesize cachedModiff;
@@ -228,7 +264,7 @@ index a0419bb5df..b9d3a0eb53 100644
@synthesize cachedPoint;
@synthesize cachedMarkActive;
@synthesize cachedCompletionAnnouncement;
@@ -7993,7 +8137,7 @@ - (void)ensureTextCache
@@ -8021,7 +8141,7 @@ - (void)ensureTextCache
NSTRACE ("EmacsAccessibilityBuffer ensureTextCache");
/* This method is only called from the main thread (AX getters
dispatch_sync to main first). Reads of cachedText/cachedTextModiff
@@ -237,7 +273,7 @@ index a0419bb5df..b9d3a0eb53 100644
write section at the end needs synchronization to protect
against concurrent reads from AX server thread. */
eassert ([NSThread isMainThread]);
@@ -8005,25 +8149,38 @@ - (void)ensureTextCache
@@ -8033,25 +8153,38 @@ - (void)ensureTextCache
if (!b)
return;
@@ -293,7 +329,7 @@ index a0419bb5df..b9d3a0eb53 100644
&& cachedTextStart == BUF_BEGV (b)
&& pt >= cachedTextStart
&& (textLen == 0
@@ -8039,7 +8196,7 @@ included in the cached AX text (it is handled separately via
@@ -8067,7 +8200,7 @@ included in the cached AX text (it is handled separately via
{
[cachedText release];
cachedText = [text retain];
@@ -302,7 +338,7 @@ index a0419bb5df..b9d3a0eb53 100644
cachedTextStart = start;
if (visibleRuns)
@@ -8051,9 +8208,9 @@ included in the cached AX text (it is handled separately via
@@ -8079,9 +8212,9 @@ included in the cached AX text (it is handled separately via
Walk the cached text once, recording the start offset of each
line. Uses NSString lineRangeForRange: --- O(N) in the total
text --- but this loop runs only on cache rebuild, which is
@@ -315,7 +351,7 @@ index a0419bb5df..b9d3a0eb53 100644
enters this code. */
if (lineStartOffsets)
xfree (lineStartOffsets);
@@ -8108,7 +8265,7 @@ - (NSUInteger)accessibilityIndexForCharpos:(ptrdiff_t)charpos
@@ -8136,7 +8269,7 @@ - (NSUInteger)accessibilityIndexForCharpos:(ptrdiff_t)charpos
/* Binary search: runs are sorted by charpos (ascending). Find the
run whose [charpos, charpos+length) range contains the target,
or the nearest run after an invisible gap. O(log n) instead of
@@ -324,7 +360,7 @@ index a0419bb5df..b9d3a0eb53 100644
NSUInteger lo = 0, hi = visibleRunCount;
while (lo < hi)
{
@@ -8157,10 +8314,10 @@ by run length (visible window), not total buffer size. */
@@ -8185,10 +8318,10 @@ by run length (visible window), not total buffer size. */
/* Convert accessibility string index to buffer charpos.
Safe to call from any thread: uses only cachedText (NSString) and
@@ -337,7 +373,7 @@ index a0419bb5df..b9d3a0eb53 100644
@synchronized (self)
{
if (visibleRunCount == 0)
@@ -8202,7 +8359,7 @@ the slow path (composed character sequence walk), which is
@@ -8230,7 +8363,7 @@ the slow path (composed character sequence walk), which is
return cp;
}
}
@@ -346,7 +382,7 @@ index a0419bb5df..b9d3a0eb53 100644
if (lo > 0)
{
ns_ax_visible_run *last = &visibleRuns[visibleRunCount - 1];
@@ -8224,7 +8381,7 @@ the slow path (composed character sequence walk), which is
@@ -8252,7 +8385,7 @@ the slow path (composed character sequence walk), which is
deadlocking the AX server thread. This is prevented by:
1. validWindow checks WINDOW_LIVE_P and BUFFERP before every
@@ -355,58 +391,75 @@ index a0419bb5df..b9d3a0eb53 100644
2. All dispatch_sync blocks run on the main thread where no
concurrent Lisp code can modify state between checks.
3. block_input prevents timer events and process output from
@@ -8570,6 +8727,50 @@ - (NSInteger)accessibilityInsertionPointLineNumber
@@ -8597,26 +8730,26 @@ - (NSInteger)accessibilityInsertionPointLineNumber
return [self lineForAXIndex:point_idx];
}
-- (NSRange)accessibilityRangeForLine:(NSInteger)line
+- (NSString *)accessibilityStringForRange:(NSRange)range
+{
+ if (![NSThread isMainThread])
+ {
{
if (![NSThread isMainThread])
{
- __block NSRange result;
+ __block NSString *result;
+ dispatch_sync (dispatch_get_main_queue (), ^{
dispatch_sync (dispatch_get_main_queue (), ^{
- result = [self accessibilityRangeForLine:line];
+ result = [self accessibilityStringForRange:range];
+ });
+ return result;
+ }
+ [self ensureTextCache];
});
return result;
}
[self ensureTextCache];
- if (!cachedText || line < 0)
- return NSMakeRange (NSNotFound, 0);
-
- NSUInteger len = [cachedText length];
- if (len == 0)
- return (line == 0) ? NSMakeRange (0, 0)
- : NSMakeRange (NSNotFound, 0);
+ if (!cachedText || range.location + range.length > [cachedText length])
+ return @"";
+ return [cachedText substringWithRange:range];
+}
+
- return [self rangeForLine:(NSUInteger)line textLength:len];
+- (NSAttributedString *)accessibilityAttributedStringForRange:(NSRange)range
+{
+ NSString *str = [self accessibilityStringForRange:range];
+ return [[[NSAttributedString alloc] initWithString:str] autorelease];
}
- (NSInteger)accessibilityLineForIndex:(NSInteger)index
@@ -8638,6 +8771,29 @@ - (NSInteger)accessibilityLineForIndex:(NSInteger)index
idx = [cachedText length];
return [self lineForAXIndex:idx];
+
+}
+
+- (NSInteger)accessibilityLineForIndex:(NSInteger)index
+- (NSRange)accessibilityRangeForLine:(NSInteger)line
+{
+ if (![NSThread isMainThread])
+ {
+ __block NSInteger result;
+ __block NSRange result;
+ dispatch_sync (dispatch_get_main_queue (), ^{
+ result = [self accessibilityLineForIndex:index];
+ result = [self accessibilityRangeForLine:line];
+ });
+ return result;
+ }
+ [self ensureTextCache];
+ if (!cachedText || index < 0)
+ return 0;
+ if (!cachedText || line < 0)
+ return NSMakeRange (NSNotFound, 0);
+
+ NSUInteger idx = (NSUInteger) index;
+ if (idx > [cachedText length])
+ idx = [cachedText length];
+ NSUInteger len = [cachedText length];
+ if (len == 0)
+ return (line == 0) ? NSMakeRange (0, 0)
+ : NSMakeRange (NSNotFound, 0);
+
+ return [self lineForAXIndex:idx];
+
+}
+
- (NSRange)accessibilityRangeForLine:(NSInteger)line
{
if (![NSThread isMainThread])
@@ -8792,7 +8993,7 @@ - (NSRect)accessibilityFrame
+ return [self rangeForLine:(NSUInteger)line textLength:len];
}
- (NSRange)accessibilityRangeForIndex:(NSInteger)index
@@ -8840,7 +8996,7 @@ - (NSRect)accessibilityFrame
/* ===================================================================
@@ -415,7 +468,7 @@ index a0419bb5df..b9d3a0eb53 100644
These methods notify VoiceOver of text and selection changes.
Called from the redisplay cycle (postAccessibilityUpdates).
@@ -8807,7 +9008,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
@@ -8855,7 +9011,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
if (point > self.cachedPoint
&& point - self.cachedPoint == 1)
{
@@ -424,7 +477,7 @@ index a0419bb5df..b9d3a0eb53 100644
[self invalidateTextCache];
[self ensureTextCache];
if (cachedText)
@@ -8826,7 +9027,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
@@ -8874,7 +9030,7 @@ - (void)postTextChangedNotification:(ptrdiff_t)point
/* Update cachedPoint here so the selection-move branch does NOT
fire for point changes caused by edits. WebKit and Chromium
never send both ValueChanged and SelectedTextChanged for the
@@ -433,7 +486,7 @@ index a0419bb5df..b9d3a0eb53 100644
self.cachedPoint = point;
NSDictionary *change = @{
@@ -9220,16 +9421,80 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
@@ -9268,16 +9424,80 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
BOOL markActive = !NILP (BVAR (b, mark_active));
/* --- Text changed (edit) --- */
@@ -518,7 +571,7 @@ index a0419bb5df..b9d3a0eb53 100644
{
ptrdiff_t oldPoint = self.cachedPoint;
BOOL oldMarkActive = self.cachedMarkActive;
@@ -9247,8 +9512,15 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
@@ -9295,8 +9515,14 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
bool isCtrlNP = ns_ax_event_is_line_nav_key (&ctrlNP);
/* --- Granularity detection --- */
@@ -531,11 +584,10 @@ index a0419bb5df..b9d3a0eb53 100644
+ to VoiceOver via the AX getter path (accessibilityValue etc.). */
NSInteger granularity = ns_ax_text_selection_granularity_unknown;
- [self ensureTextCache];
+ BOOL singleLineMove = NO;
if (cachedText && oldPoint > 0)
{
NSUInteger tlen = [cachedText length];
@@ -12408,7 +12680,7 @@ - (int) fullscreenState
@@ -12457,7 +12683,7 @@ - (int) fullscreenState
if (WINDOW_LEAF_P (w))
{
@@ -544,7 +596,7 @@ index a0419bb5df..b9d3a0eb53 100644
EmacsAccessibilityBuffer *elem
= [existing objectForKey:[NSValue valueWithPointer:w]];
if (!elem)
@@ -12442,7 +12714,7 @@ - (int) fullscreenState
@@ -12491,7 +12717,7 @@ - (int) fullscreenState
}
else
{
@@ -553,7 +605,7 @@ index a0419bb5df..b9d3a0eb53 100644
Lisp_Object child = w->contents;
while (!NILP (child))
{
@@ -12554,7 +12826,7 @@ - (void)postAccessibilityUpdates
@@ -12603,7 +12829,7 @@ - (void)postAccessibilityUpdates
accessibilityUpdating = YES;
/* Detect window tree change (split, delete, new buffer). Compare
@@ -562,7 +614,7 @@ index a0419bb5df..b9d3a0eb53 100644
Lisp_Object curRoot = FRAME_ROOT_WINDOW (emacsframe);
if (!EQ (curRoot, lastRootWindow))
{
@@ -12563,12 +12835,12 @@ - (void)postAccessibilityUpdates
@@ -12612,12 +12838,12 @@ - (void)postAccessibilityUpdates
}
/* If tree is stale, rebuild FIRST so we don't iterate freed