Files
emacs-doom/patches/0008-ns-announce-child-frame-completion-candidates-for-Vo.patch
Daneel 0f7608326c patches: fix 2 blockers from Opus review
BLOCKER #1: accessibilityUpdating flag exception safety.
A Lisp signal (longjmp) during postAccessibilityUpdates left
the re-entrance flag permanently YES, suppressing all future
AX notifications → VoiceOver goes silent randomly.
Fix: specpdl unwind protection (record_unwind_protect_ptr)
resets the flag on any longjmp. All 3 exit points use unbind_to.

BLOCKER #2: static struct buffer *lastBuffer dangling pointer.
Raw C pointer to buffer struct has no GC protection. After
kill-buffer, the pointer dangles.
Fix: file-scope Lisp_Object lastChildFrameBuffer with staticpro.
EQ comparison instead of pointer equality.

Also: revert accidental em-dash → triple-dash in title bar (0007),
fix README factual error (BUF_OVERLAY_MODIFF cache key).
2026-02-28 18:29:19 +01:00

400 lines
14 KiB
Diff

From 8564e4989f5f358092bd1494c3894a42974ee6e1 Mon Sep 17 00:00:00 2001
From: Martin Sukany <martin@sukany.cz>
Date: Sat, 28 Feb 2026 16:01:29 +0100
Subject: [PATCH 2/2] ns: announce child frame completion candidates for
VoiceOver
Completion frameworks such as Corfu, Company-box, and similar
render candidates in a child frame rather than as overlay strings
in the minibuffer. This patch extends the overlay announcement
support (patch 7/8) to handle child frame popups.
Detect child frames via FRAME_PARENT_FRAME in postAccessibilityUpdates.
Scan the child frame buffer text line by line using Fget_char_property
(which checks both text properties and overlay face properties) to
find the selected candidate. Reuse ns_ax_face_is_selected from
the overlay patch to identify "current", "selected", and
"selection" faces.
Safety:
- record_unwind_current_buffer / set_buffer_internal_1 to switch to
the child frame buffer for Fbuffer_substring_no_properties.
- Re-entrance guard (accessibilityUpdating) before child frame dispatch.
- BUF_MODIFF gating prevents redundant scans.
- WINDOWP, BUFFERP validation for partially initialized frames.
- Buffer size limit (10000 chars) skips non-completion child frames.
When the child frame closes, post FocusedUIElementChangedNotification
on the parent buffer element to restore VoiceOver's character echo
and cursor tracking. The flag childFrameCompletionActive is set by
the child frame handler and cleared on the parent's next accessibility
cycle when no child frame is visible (via FOR_EACH_FRAME).
Announce via AnnouncementRequested to NSApp with High priority.
Use direct UAZoomChangeFocus because the child frame renders
independently --- its ns_update_end runs after the parent's
draw_window_cursor, so the last Zoom call wins.
* src/nsterm.h (EmacsView): Add announceChildFrameCompletion,
childFrameCompletionActive flag.
* src/nsterm.m (ns_ax_selected_child_frame_text): New function.
(EmacsView announceChildFrameCompletion): New method, set parent flag.
(EmacsView postAccessibilityUpdates): Dispatch to child frame handler,
refocus parent buffer element when child frame closes.
---
src/nsterm.h | 2 +
src/nsterm.m | 279 ++++++++++++++++++++++++++++++++++++++++++++++++++-
2 files changed, 278 insertions(+), 3 deletions(-)
diff --git a/src/nsterm.h b/src/nsterm.h
index 5c15639..8b34300 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -594,6 +594,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
NSRect lastAccessibilityCursorRect;
BOOL overlayZoomActive;
NSRect overlayZoomRect;
+ BOOL childFrameCompletionActive;
#endif
BOOL font_panel_active;
NSFont *font_panel_result;
@@ -657,6 +658,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
- (void)rebuildAccessibilityTree;
- (void)invalidateAccessibilityTree;
- (void)postAccessibilityUpdates;
+- (void)announceChildFrameCompletion;
#endif
@end
diff --git a/src/nsterm.m b/src/nsterm.m
index 143e784..da1a319 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -7066,6 +7066,110 @@ ns_ax_selected_overlay_text (struct buffer *b,
}
+/* Scan buffer text of a child frame for the selected completion
+ candidate. Used for frameworks that render candidates in a
+ child frame (e.g. Corfu, Company-box) rather than as overlay
+ strings. Check the effective face (text properties + overlays)
+ at the start of each line via Fget_char_property.
+
+ Returns the candidate text (trimmed) or nil. Sets
+ *OUT_LINE_INDEX to the 0-based line index for Zoom. */
+static NSString *
+ns_ax_selected_child_frame_text (struct buffer *b, Lisp_Object buf_obj,
+ int *out_line_index)
+{
+ *out_line_index = -1;
+ ptrdiff_t beg = BUF_BEGV (b);
+ ptrdiff_t end = BUF_ZV (b);
+
+ if (beg >= end)
+ return nil;
+
+ /* Temporarily switch to the child frame buffer.
+ Fbuffer_substring_no_properties operates on current_buffer,
+ which may be a different buffer (e.g., the parent frame's). */
+ specpdl_ref count = SPECPDL_INDEX ();
+ record_unwind_current_buffer ();
+ set_buffer_internal_1 (b);
+
+ /* Get buffer text as a Lisp string for efficient scanning.
+ The buffer is a small completion popup (typically < 20 lines). */
+ Lisp_Object str
+ = Fbuffer_substring_no_properties (make_fixnum (beg),
+ make_fixnum (end));
+ if (!STRINGP (str) || SCHARS (str) == 0)
+ {
+ unbind_to (count, Qnil);
+ return nil;
+ }
+
+ /* Scan newlines (same pattern as ns_ax_selected_overlay_text).
+ The data pointer is used only in this loop, before Lisp calls. */
+ const unsigned char *data = SDATA (str);
+ ptrdiff_t byte_len = SBYTES (str);
+ ptrdiff_t line_starts[128];
+ ptrdiff_t line_ends[128];
+ int nlines = 0;
+ ptrdiff_t char_pos = 0, byte_pos = 0, lstart = 0;
+
+ while (byte_pos < byte_len && nlines < 128)
+ {
+ if (data[byte_pos] == '\n')
+ {
+ if (char_pos > lstart)
+ {
+ line_starts[nlines] = lstart;
+ line_ends[nlines] = char_pos;
+ nlines++;
+ }
+ lstart = char_pos + 1;
+ }
+ if (STRING_MULTIBYTE (str))
+ byte_pos += BYTES_BY_CHAR_HEAD (data[byte_pos]);
+ else
+ byte_pos++;
+ char_pos++;
+ }
+ if (char_pos > lstart && nlines < 128)
+ {
+ line_starts[nlines] = lstart;
+ line_ends[nlines] = char_pos;
+ nlines++;
+ }
+
+ /* Find the line with a selected face. Use Fget_char_property on
+ the BUFFER (not the string) so overlay faces are included.
+ Offset string positions by beg to get buffer positions. */
+ for (int li = 0; li < nlines; li++)
+ {
+ ptrdiff_t buf_pos = beg + line_starts[li];
+ Lisp_Object face
+ = Fget_char_property (make_fixnum (buf_pos), Qface, buf_obj);
+
+ if (ns_ax_face_is_selected (face))
+ {
+ Lisp_Object line
+ = Fsubstring_no_properties (str,
+ make_fixnum (line_starts[li]),
+ make_fixnum (line_ends[li]));
+ NSString *text = [NSString stringWithLispString:line];
+ text = [text stringByTrimmingCharactersInSet:
+ [NSCharacterSet
+ whitespaceAndNewlineCharacterSet]];
+ if ([text length] > 0)
+ {
+ *out_line_index = li;
+ unbind_to (count, Qnil);
+ return text;
+ }
+ }
+ }
+
+ unbind_to (count, Qnil);
+ return nil;
+}
+
+
/* Build accessibility text for window W, skipping invisible text.
Populates *OUT_START with the buffer start charpos.
Populates *OUT_RUNS with an array of visible runs and *OUT_NRUNS
@@ -12311,6 +12415,122 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,
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. */
+
+/* Announce the selected candidate in a child frame completion popup.
+ Handles Corfu, Company-box, and similar frameworks that render
+ candidates in a separate child frame rather than as overlay strings
+ in the minibuffer. Uses direct UAZoomChangeFocus (not the
+ overlayZoomRect flag) because the child frame's ns_update_end runs
+ after the parent's draw_window_cursor. */
+- (void)announceChildFrameCompletion
+{
+
+
+ /* Validate frame state --- child frames may be partially
+ initialized during creation. */
+ if (!WINDOWP (emacsframe->selected_window))
+ return;
+ struct window *w = XWINDOW (emacsframe->selected_window);
+ if (!BUFFERP (w->contents))
+ return;
+ struct buffer *b = XBUFFER (w->contents);
+
+ /* Only scan when the buffer content has actually changed.
+ 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 (EQ (w->contents, lastChildFrameBuffer)
+ && modiff == lastChildFrameModiff)
+ return;
+ lastChildFrameBuffer = w->contents;
+ lastChildFrameModiff = modiff;
+
+ /* Skip buffers larger than a typical completion popup.
+ This avoids scanning eldoc, which-key, or other child
+ frame buffers that are not completion UIs. */
+ if (BUF_ZV (b) - BUF_BEGV (b) > 10000)
+ return;
+
+ int selected_line = -1;
+ NSString *candidate
+ = ns_ax_selected_child_frame_text (b, w->contents, &selected_line);
+
+ if (!candidate)
+ return;
+
+ /* Deduplicate --- avoid re-announcing the same candidate. */
+ const char *cstr = [candidate UTF8String];
+ if (lastChildFrameCandidate && strcmp (cstr, lastChildFrameCandidate) == 0)
+ return;
+ xfree (lastChildFrameCandidate);
+ lastChildFrameCandidate = xstrdup (cstr);
+
+ NSDictionary *annInfo = @{
+ NSAccessibilityAnnouncementKey: candidate,
+ NSAccessibilityPriorityKey:
+ @(NSAccessibilityPriorityHigh)
+ };
+ ns_ax_post_notification_with_info (
+ NSApp,
+ NSAccessibilityAnnouncementRequestedNotification,
+ annInfo);
+
+ /* Mark the parent as having an active child frame completion.
+ When the child frame closes, the parent's next accessibility
+ cycle will post FocusedUIElementChanged to restore VoiceOver's
+ focus to the buffer text element. */
+ struct frame *parent = FRAME_PARENT_FRAME (emacsframe);
+ if (parent)
+ {
+ EmacsView *parentView = FRAME_NS_VIEW (parent);
+ if (parentView)
+ parentView->childFrameCompletionActive = YES;
+ }
+
+ /* Zoom tracking: focus on the selected row in the child frame.
+ Use direct UAZoomChangeFocus rather than overlayZoomRect because
+ the child frame renders independently of the parent. */
+ if (selected_line >= 0 && UAZoomEnabled ())
+ {
+ int line_h = FRAME_LINE_HEIGHT (emacsframe);
+ int y_off = selected_line * line_h;
+ NSRect r = NSMakeRect (
+ WINDOW_TEXT_TO_FRAME_PIXEL_X (w, 0),
+ WINDOW_TO_FRAME_PIXEL_Y (w, y_off),
+ FRAME_COLUMN_WIDTH (emacsframe),
+ line_h);
+ NSRect winRect = [self convertRect:r toView:nil];
+ NSRect screenRect
+ = [[self window] convertRectToScreen:winRect];
+ CGRect cgRect = NSRectToCGRect (screenRect);
+ CGFloat primaryH
+ = [[[NSScreen screens] firstObject] frame].size.height;
+ cgRect.origin.y
+ = primaryH - cgRect.origin.y - cgRect.size.height;
+ UAZoomChangeFocus (&cgRect, &cgRect,
+ kUAZoomFocusTypeInsertionPoint);
+ }
+}
+
+/* Child frame completion dedup state. File-scope so that
+ lastChildFrameBuffer can be staticpro'd against GC. */
+static Lisp_Object lastChildFrameBuffer;
+static EMACS_INT lastChildFrameModiff;
+static char *lastChildFrameCandidate;
+
+/* Reset the re-entrance guard when unwinding past
+ postAccessibilityUpdates due to a Lisp signal (longjmp).
+ Without this, a signal during Lisp calls (e.g. Fget_char_property
+ in overlay or child frame scanning) would leave
+ accessibilityUpdating = YES permanently, suppressing all future
+ accessibility notifications. */
+static void
+ns_ax_reset_accessibility_updating (void *view)
+{
+ ((EmacsView *)view)->accessibilityUpdating = NO;
+}
+
- (void)postAccessibilityUpdates
{
NSTRACE ("[EmacsView postAccessibilityUpdates]");
@@ -12321,10 +12541,60 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,
/* Re-entrance guard: VoiceOver callbacks during notification posting
can trigger redisplay, which calls ns_update_end, which calls us
- again. Prevent infinite recursion. */
+ again. Prevent infinite recursion. This MUST come before the
+ child frame check --- announceChildFrameCompletion makes Lisp
+ calls that can trigger redisplay. */
if (accessibilityUpdating)
return;
accessibilityUpdating = YES;
+ specpdl_ref axCount = SPECPDL_INDEX ();
+ record_unwind_protect_ptr (ns_ax_reset_accessibility_updating, self);
+
+ /* Child frame completion popup (Corfu, Company-box, etc.).
+ Child frames don't participate in the accessibility tree;
+ announce the selected candidate directly. */
+ if (FRAME_PARENT_FRAME (emacsframe))
+ {
+ [self announceChildFrameCompletion];
+ unbind_to (axCount, Qnil);
+ return;
+ }
+
+ /* If a child frame completion was recently active but no child
+ frame is visible anymore, refocus VoiceOver on the buffer
+ element so character echo and cursor tracking resume.
+ Skip if a child frame still exists (completion still open). */
+ if (childFrameCompletionActive)
+ {
+ Lisp_Object tail, frame;
+ BOOL childStillVisible = NO;
+ FOR_EACH_FRAME (tail, frame)
+ if (FRAME_PARENT_FRAME (XFRAME (frame)) == emacsframe
+ && FRAME_VISIBLE_P (XFRAME (frame)))
+ {
+ childStillVisible = YES;
+ break;
+ }
+
+ if (!childStillVisible)
+ {
+ childFrameCompletionActive = NO;
+ EmacsAccessibilityBuffer *focused = nil;
+ for (id elem in accessibilityElements)
+ if ([elem isKindOfClass:
+ [EmacsAccessibilityBuffer class]]
+ && [(EmacsAccessibilityBuffer *)elem
+ isAccessibilityFocused])
+ {
+ focused = elem;
+ break;
+ }
+ if (focused)
+ ns_ax_post_notification (
+ focused,
+ NSAccessibilityFocusedUIElementChangedNotification);
+ }
+ }
/* Detect window tree change (split, delete, new buffer). Compare
FRAME_ROOT_WINDOW --- if it changed, the tree structure changed. */
@@ -12355,7 +12625,7 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,
NSAccessibilityFocusedUIElementChangedNotification);
lastSelectedWindow = emacsframe->selected_window;
- accessibilityUpdating = NO;
+ unbind_to (axCount, Qnil);
return;
}
@@ -12399,7 +12669,7 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,
NSAccessibilityFocusedUIElementChangedNotification);
}
- accessibilityUpdating = NO;
+ unbind_to (axCount, Qnil);
}
/* ---- Cursor position for Zoom (via accessibilityBoundsForRange:) ----
@@ -14341,6 +14611,9 @@ syms_of_nsterm (void)
DEFSYM (Qns_ax_completion, "completion");
DEFSYM (Qns_ax_completions_highlight, "completions-highlight");
DEFSYM (Qns_ax_backtab, "backtab");
+
+ lastChildFrameBuffer = Qnil;
+ staticpro (&lastChildFrameBuffer);
/* Qmouse_face and Qkeymap are defined in textprop.c / keymap.c. */
Fput (Qalt, Qmodifier_value, make_fixnum (alt_modifier));
Fput (Qhyper, Qmodifier_value, make_fixnum (hyper_modifier));
--
2.43.0