patches: address all non-blocking review findings

Fix remaining issues identified in core maintainer review:
- 0000: correct [PATCH 0/8] to [PATCH] (standalone Zoom series)
- 0006: add paragraph break in macos.texi VoiceOver section
- 0001/0005: shorten separator comment lines to 79 chars
- 0007/0008: genericize third-party package names in commit messages
- 0001: fix block_input ordering (block before registering unwind)
- 0000/0007: document intentional face_is_selected duplication
This commit is contained in:
2026-03-03 12:48:31 +01:00
parent 9bee2a1987
commit 61c23aed2c
12 changed files with 202 additions and 126 deletions

View File

@@ -1,13 +1,13 @@
From 1e3d3919fd41e4480a02190fb89bee1ef8107d62 Mon Sep 17 00:00:00 2001
From: Daneel <daneel@sukany.cz>
From: Martin Sukany <martin@sukany.cz>
Date: Mon, 2 Mar 2026 18:49:13 +0100
Subject: [PATCH 8/8] ns: announce child frame completion candidates for
VoiceOver
Child frame popups (Corfu, Company-mode child frames) render completion
candidates in a separate frame whose buffer is not accessible via the
minibuffer overlay path. This patch scans child frame buffers for
selected candidates and announces them via VoiceOver.
Child-frame completion frameworks render candidates in a separate
frame whose buffer is not accessible via the minibuffer overlay path
(e.g., Corfu, Company-box). This patch scans child frame buffers
for selected candidates and announces them via VoiceOver.
* src/nsterm.h (EmacsView): Add childFrameLastBuffer, childFrameLastModiff,
childFrameLastCandidate, childFrameCompletionActive, lastEchoCharsModiff
@@ -34,7 +34,7 @@ area announcements.
etc/NEWS | 25 ++-
src/nsterm.h | 21 ++
src/nsterm.m | 514 +++++++++++++++++++++++++++++++++++++++----
4 files changed, 510 insertions(+), 64 deletions(-)
4 files changed, 510 insertions(+), 68 deletions(-)
diff --git a/doc/emacs/macos.texi b/doc/emacs/macos.texi
index 8d4a7825d8..03a657f970 100644
@@ -167,7 +167,7 @@ diff --git a/src/nsterm.m b/src/nsterm.m
index 8ef344d9fe..1acb64630a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1275,7 +1275,13 @@ If a completion candidate is selected (overlay or child frame),
@@ -1278,7 +1278,13 @@ If a completion candidate is selected (overlay or child frame),
static void
ns_zoom_track_completion (struct frame *f, EmacsView *view)
{
@@ -182,7 +182,7 @@ index 8ef344d9fe..1acb64630a 100644
return;
if (!WINDOWP (f->selected_window))
return;
@@ -1393,7 +1399,7 @@ so the visual offset is (ov_line + 1) * line_h from
@@ -1396,7 +1402,7 @@ so the visual offset is (ov_line + 1) * line_h from
(zoomCursorUpdated is NO). */
#if defined (MAC_OS_X_VERSION_MIN_REQUIRED) \
&& MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
@@ -191,7 +191,7 @@ index 8ef344d9fe..1acb64630a 100644
&& ns_zoom_enabled_p ()
&& !NSIsEmptyRect (view->lastCursorRect))
{
@@ -3571,7 +3577,7 @@ EmacsView pixels (AppKit, flipped, top-left origin)
@@ -3574,7 +3580,7 @@ EmacsView pixels (AppKit, flipped, top-left origin)
#if defined (MAC_OS_X_VERSION_MIN_REQUIRED) \
&& MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
@@ -200,7 +200,7 @@ index 8ef344d9fe..1acb64630a 100644
{
NSRect windowRect = [view convertRect:r toView:nil];
NSRect screenRect
@@ -7407,6 +7413,117 @@ visual line index for Zoom (skip whitespace-only lines
@@ -7413,6 +7419,117 @@ visual line index for Zoom (skip whitespace-only lines
return nil;
}
@@ -318,7 +318,7 @@ index 8ef344d9fe..1acb64630a 100644
/* 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
@@ -7440,9 +7557,13 @@ visual line index for Zoom (skip whitespace-only lines
@@ -7446,9 +7563,13 @@ visual line index for Zoom (skip whitespace-only lines
return @"";
specpdl_ref count = SPECPDL_INDEX ();
@@ -326,14 +326,13 @@ index 8ef344d9fe..1acb64630a 100644
+ if anything between SPECPDL_INDEX and block_input were to throw,
+ the unwind handler would call unblock_input without a matching
+ block_input, corrupting the input-blocking reference count. */
+ block_input ();
block_input ();
record_unwind_current_buffer ();
record_unwind_protect_void (unblock_input);
- block_input ();
if (b != current_buffer)
set_buffer_internal_1 (b);
@@ -8605,6 +8726,11 @@ - (void)setAccessibilitySelectedTextRange:(NSRange)range
@@ -8611,6 +8732,11 @@ - (void)setAccessibilitySelectedTextRange:(NSRange)range
[self ensureTextCache];
@@ -345,7 +344,7 @@ index 8ef344d9fe..1acb64630a 100644
specpdl_ref count = SPECPDL_INDEX ();
record_unwind_current_buffer ();
/* Ensure block_input is always matched by unblock_input even if
@@ -9053,20 +9179,38 @@ - (void)postFocusedCursorNotification:(ptrdiff_t)point
@@ -9059,20 +9185,38 @@ - (void)postFocusedCursorNotification:(ptrdiff_t)point
&& granularity
== ns_ax_text_selection_granularity_character);
@@ -394,7 +393,7 @@ index 8ef344d9fe..1acb64630a 100644
ns_ax_post_notification_with_info (
self,
NSAccessibilitySelectedTextChangedNotification,
@@ -9166,12 +9310,17 @@ user expectation ("w" jumps to next word and reads it). */
@@ -9172,12 +9316,17 @@ user expectation ("w" jumps to next word and reads it). */
}
}
@@ -407,7 +406,7 @@ index 8ef344d9fe..1acb64630a 100644
+ - C-n/C-p: SelectedTextChanged carries granularity=line, but
+ VoiceOver processes those keystrokes specially and may not
+ produce speech; the explicit announcement is the reliable path.
+ - Discontiguous jumps (]], M-<, xref, imenu, ): granularity=line
+ - Discontiguous jumps (]], M-<, xref, imenu, ...): granularity=line
+ in the notification is omitted (see above) so VoiceOver will
+ not announce automatically; this explicit announcement fills
+ the gap.
@@ -417,7 +416,7 @@ index 8ef344d9fe..1acb64630a 100644
if (cachedText
&& granularity == ns_ax_text_selection_granularity_line)
{
@@ -9236,6 +9385,11 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b
@@ -9242,6 +9391,11 @@ - (void)postCompletionAnnouncementForBuffer:(struct buffer *)b
block_input ();
specpdl_ref count2 = SPECPDL_INDEX ();
@@ -429,7 +428,7 @@ index 8ef344d9fe..1acb64630a 100644
record_unwind_protect_void (unblock_input);
record_unwind_current_buffer ();
if (b != current_buffer)
@@ -9412,12 +9566,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
@@ -9418,12 +9572,29 @@ - (void)postAccessibilityNotificationsForFrame:(struct frame *)f
if (!b)
return;
@@ -437,7 +436,7 @@ index 8ef344d9fe..1acb64630a 100644
+ postEchoAreaAnnouncementIfNeeded (called from postAccessibilityUpdates
+ before this per-element loop) so that they are never lost to a
+ concurrent tree rebuild. For the inactive minibuffer (minibuf_level
+ == 0), skip normal cursor and completion processing there is no
+ == 0), skip normal cursor and completion processing --- there is no
+ meaningful cursor to track. */
+ if (MINI_WINDOW_P (w) && minibuf_level == 0)
+ return;
@@ -459,7 +458,7 @@ index 8ef344d9fe..1acb64630a 100644
if (modiff != self.cachedModiff)
{
self.cachedModiff = modiff;
@@ -9431,6 +9602,7 @@ Text property changes (e.g. face updates from
@@ -9437,6 +9608,7 @@ Text property changes (e.g. face updates from
{
self.cachedCharsModiff = chars_modiff;
[self postTextChangedNotification:point];
@@ -467,7 +466,7 @@ index 8ef344d9fe..1acb64630a 100644
}
}
@@ -9453,8 +9625,15 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9459,8 +9631,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.
@@ -485,7 +484,7 @@ index 8ef344d9fe..1acb64630a 100644
goto skip_overlay_scan;
int selected_line = -1;
@@ -9500,7 +9679,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9506,7 +9685,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
self.cachedPoint = point;
self.cachedMarkActive = markActive;
@@ -505,7 +504,7 @@ index 8ef344d9fe..1acb64630a 100644
NSInteger direction = ns_ax_text_selection_direction_discontiguous;
if (point > oldPoint)
direction = ns_ax_text_selection_direction_next;
@@ -9512,6 +9702,7 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9518,6 +9708,7 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
/* --- Granularity detection --- */
NSInteger granularity = ns_ax_text_selection_granularity_unknown;
@@ -513,7 +512,7 @@ index 8ef344d9fe..1acb64630a 100644
[self ensureTextCache];
if (cachedText && oldPoint > 0)
{
@@ -9526,7 +9717,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9532,7 +9723,18 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
NSRange newLine = [cachedText lineRangeForRange:
NSMakeRange (newIdx, 0)];
if (oldLine.location != newLine.location)
@@ -522,7 +521,7 @@ index 8ef344d9fe..1acb64630a 100644
+ granularity = ns_ax_text_selection_granularity_line;
+ /* Detect single adjacent-line move while oldLine/newLine
+ are in scope. Any command that steps exactly one line ---
+ C-n/C-p, evil j/k, outline-next-heading, etc. --- is
+ C-n/C-p, outline-next-heading, etc. --- is
+ sequential. Multi-line teleports (]], M-<, xref, ...) are
+ not adjacent and will be marked discontiguous below.
+ Detected structurally: no package-specific code needed. */
@@ -533,7 +532,7 @@ index 8ef344d9fe..1acb64630a 100644
else
{
NSUInteger dist = (newIdx > oldIdx
@@ -9548,34 +9750,23 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
@@ -9554,38 +9756,23 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
granularity = ns_ax_text_selection_granularity_line;
}
@@ -545,13 +544,19 @@ index 8ef344d9fe..1acb64630a 100644
- 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;
-
- /* Until voiceoverSetPoint tracking is added, treat all cursor
- moves as Emacs-initiated (refined in a later patch). */
- BOOL emacsMovedCursor = YES;
-
+
+ /* Multi-line teleports are discontiguous; single adjacent-line
+ steps stay sequential. */
+ if (!isCtrlNP && !singleLineMove
+ && granularity == ns_ax_text_selection_granularity_line)
direction = ns_ax_text_selection_direction_discontiguous;
+ 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
@@ -581,7 +586,7 @@ index 8ef344d9fe..1acb64630a 100644
{
NSWindow *win = [self.emacsView window];
if (win)
@@ -9734,6 +9925,13 @@ - (NSRect)accessibilityFrame
@@ -9740,6 +9931,13 @@ - (NSRect)accessibilityFrame
if (vis_start >= vis_end)
return @[];
@@ -595,7 +600,7 @@ index 8ef344d9fe..1acb64630a 100644
block_input ();
specpdl_ref blk_count = SPECPDL_INDEX ();
record_unwind_protect_void (unblock_input);
@@ -10040,6 +10238,10 @@ - (void)dealloc
@@ -10046,6 +10244,10 @@ - (void)dealloc
#endif
[accessibilityElements release];
@@ -606,7 +611,7 @@ index 8ef344d9fe..1acb64630a 100644
[[self menu] release];
[super dealloc];
}
@@ -11489,6 +11691,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
@@ -11495,6 +11697,9 @@ - (instancetype) initFrameFromEmacs: (struct frame *)f
windowClosing = NO;
processingCompose = NO;
@@ -616,7 +621,7 @@ index 8ef344d9fe..1acb64630a 100644
scrollbarsNeedingUpdate = 0;
fs_state = FULLSCREEN_NONE;
fs_before_fs = next_maximized = -1;
@@ -12797,6 +13002,161 @@ - (id)accessibilityFocusedUIElement
@@ -12803,6 +13008,161 @@ - (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. */
@@ -778,7 +783,7 @@ index 8ef344d9fe..1acb64630a 100644
- (void)postAccessibilityUpdates
{
NSTRACE ("[EmacsView postAccessibilityUpdates]");
@@ -12807,11 +13167,69 @@ - (void)postAccessibilityUpdates
@@ -12813,11 +13173,69 @@ - (void)postAccessibilityUpdates
/* Re-entrance guard: VoiceOver callbacks during notification posting
can trigger redisplay, which calls ns_update_end, which calls us