ns: fix VoiceOver cursor sync — 8 changes from pipeline review

Changes applied (from vo-cursor pipeline review, 7 workers):

1. (Change 8) Add ns_ax_index_for_charpos helper, refactor
   ns_ax_index_for_point as thin wrapper — shared coordinate
   mapping for all accessibility attribute methods.

2. (Change 1) Remove Site A notifications from ns_draw_window_cursor.
   Eliminates duplicate VoiceOver notifications (Site A + Site B both
   fired for same events). Zoom cursor tracking (UAZoomChangeFocus)
   preserved.

3. (Change 7) Remove redundant bare ValueChanged before rich
   userInfo version in postAccessibilityUpdatesForWindow:.

4. (Change 2) Fix typing echo character extraction to use glyph-based
   index (ns_ax_index_for_point) instead of buffer-relative
   (pt - BUF_BEGV - 1).

5. (Change 3) Add AXTextStateChangeType:@2 (SelectionMove) userInfo
   to SelectedTextChanged notification for cursor movement —
   enables VoiceOver line-by-line reading on arrow keys.

6. (Change 4) Fix accessibilityRangeForPosition: to return
   glyph-based index via ns_ax_index_for_charpos instead of
   buffer-relative (charpos - BUF_BEGV).

7. (Change 5) Fix accessibilitySelectedTextRange mark branch to use
   ns_ax_index_for_charpos for both endpoints instead of mixing
   glyph-based point with buffer-relative mark.

8. Remove 10 redundant text methods from EmacsView (Group role
   should not expose text attributes — eliminates coordinate
   system divergence with EmacsAccessibilityBuffer).

9. Fix MRC leak: release EmacsAccessibilityBuffer after addObject:
   in ns_ax_collect_windows.

10. Remove dead lastAccessibilityModiff ivar (was only used by
    removed Site A).

Enum values verified from WebKit AXTextStateChangeIntent.h:
  AXTextStateChangeTypeEdit = 1
  AXTextStateChangeTypeSelectionMove = 2
  AXTextEditTypeTyping = 3
This commit is contained in:
2026-02-26 11:44:13 +01:00
parent 946b138a0a
commit 8849a726a3

View File

@@ -5,14 +5,19 @@ Subject: [PATCH] ns: add macOS Zoom cursor tracking and VoiceOver accessibility
Dual accessibility: UAZoomChangeFocus for Zoom + virtual element tree for
VoiceOver. Notifications target the focused virtual element, not EmacsView.
Full hierarchy plumbing (accessibilityWindow, accessibilityParent, etc.).
MRC compatible.
MRC compatible. Single notification path (post-redisplay only) with
glyph-consistent coordinate system throughout.
---
--- a/src/nsterm.h 2026-02-26 08:46:18.118172281 +0100
+++ b/src/nsterm.h 2026-02-26 09:08:57.204955708 +0100
@@ -455,6 +455,34 @@
diff --git a/src/nsterm.h b/src/nsterm.h
index 7c1ee4c..1f9639c 100644
--- a/src/nsterm.h
+++ b/src/nsterm.h
@@ -453,6 +453,34 @@ enum ns_return_frame_mode
@end
/* ==========================================================================
+/* ==========================================================================
+
+ Accessibility virtual elements (macOS / Cocoa only)
+
+ ========================================================================== */
@@ -39,12 +44,10 @@ MRC compatible.
+#endif /* NS_IMPL_COCOA */
+
+
+/* ==========================================================================
+
The main Emacs view
/* ==========================================================================
========================================================================== */
@@ -471,6 +499,12 @@
The main Emacs view
@@ -471,6 +499,11 @@ enum ns_return_frame_mode
#ifdef NS_IMPL_COCOA
char *old_title;
BOOL maximizing_resize;
@@ -52,12 +55,11 @@ MRC compatible.
+ Lisp_Object lastSelectedWindow;
+ @public
+ NSRect lastAccessibilityCursorRect;
+ ptrdiff_t lastAccessibilityModiff;
+ @protected
#endif
BOOL font_panel_active;
NSFont *font_panel_result;
@@ -528,6 +562,12 @@
@@ -528,6 +561,12 @@ enum ns_return_frame_mode
- (void)windowWillExitFullScreen;
- (void)windowDidExitFullScreen;
- (void)windowDidBecomeKey;
@@ -70,9 +72,11 @@ MRC compatible.
@end
--- a/src/nsterm.m 2026-02-26 08:46:18.124172384 +0100
+++ b/src/nsterm.m 2026-02-26 10:46:54.607568839 +0100
@@ -1104,6 +1104,11 @@
diff --git a/src/nsterm.m b/src/nsterm.m
index 932d209..077f092 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -1104,6 +1104,11 @@ static NSRect constrain_frame_rect(NSRect frameRect, bool isFullscreen)
unblock_input ();
ns_updating_frame = NULL;
@@ -84,66 +88,21 @@ MRC compatible.
}
static void
@@ -3232,6 +3237,82 @@
@@ -3232,6 +3237,37 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
/* Prevent the cursor from being drawn outside the text area. */
r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
+#ifdef NS_IMPL_COCOA
+ /* Accessibility cursor tracking for macOS Zoom and VoiceOver.
+ Only notify AT when drawing the cursor in the active (selected)
+ window. Without this guard, C-x o triggers UAZoomChangeFocus
+ for the old window last, snapping Zoom back. */
+ /* Accessibility: store cursor rect for Zoom and bounds queries.
+ VoiceOver notifications are handled solely by
+ postAccessibilityUpdatesForWindow: (called from ns_update_end)
+ to avoid duplicate notifications and mid-redisplay fragility. */
+ {
+ EmacsView *view = FRAME_NS_VIEW (f);
+ if (view && on_p && active_p)
+ {
+ /* Store cursor rect for accessibilityBoundsForRange: queries. */
+ view->lastAccessibilityCursorRect = r;
+
+ /* Find the focused virtual element — VoiceOver tracks IT,
+ not the EmacsView (AXGroup). Notifications must come from
+ the element VoiceOver is monitoring. */
+ id axTarget = [view accessibilityFocusedUIElement];
+ if (!axTarget)
+ axTarget = view;
+
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (f->selected_window)->contents);
+
+ if (curbuf && BUF_MODIFF (curbuf) != view->lastAccessibilityModiff)
+ {
+ /* Buffer content changed — typing echo. Post ValueChanged
+ with rich userInfo on the FOCUSED ELEMENT.
+ kAXTextStateChangeTypeEdit = 1, kAXTextEditTypeTyping = 3. */
+ view->lastAccessibilityModiff = BUF_MODIFF (curbuf);
+
+ NSString *changedText = @"";
+ ptrdiff_t pt = BUF_PT (curbuf);
+ if (pt > BUF_BEGV (curbuf))
+ {
+ NSRange charRange = NSMakeRange (
+ (NSUInteger)(pt - BUF_BEGV (curbuf) - 1), 1);
+ changedText = [view accessibilityStringForRange:charRange];
+ if (!changedText)
+ changedText = @"";
+ }
+
+ NSDictionary *change = @{
+ @"AXTextEditType": @3,
+ @"AXTextChangeValue": changedText
+ };
+ NSDictionary *userInfo = @{
+ @"AXTextStateChangeType": @1,
+ @"AXTextChangeValues": @[change]
+ };
+ NSAccessibilityPostNotificationWithUserInfo (
+ axTarget, NSAccessibilityValueChangedNotification, userInfo);
+ }
+
+ /* Always notify cursor movement on the focused element. */
+ NSAccessibilityPostNotification (
+ axTarget, NSAccessibilitySelectedTextChangedNotification);
+
+ /* Tell macOS Zoom where the cursor is. UAZoomChangeFocus()
+ expects top-left origin (CG coordinate space). */
+ if (UAZoomEnabled ())
@@ -167,10 +126,12 @@ MRC compatible.
ns_focus (f, NULL, 0);
NSGraphicsContext *ctx = [NSGraphicsContext currentContext];
@@ -6849,6 +6930,646 @@
/* ==========================================================================
@@ -6847,6 +6883,652 @@ - (BOOL)fulfillService: (NSString *)name withArg: (NSString *)arg
}
#endif
+/* ==========================================================================
+
+ Accessibility virtual elements (macOS / Cocoa only)
+
+ ========================================================================== */
@@ -401,19 +362,16 @@ MRC compatible.
+}
+
+
+/* Compute the character index within glyph-extracted text that
+ corresponds to the buffer point position. */
+/* Compute the glyph-based character index for an arbitrary buffer
+ charpos within the visible glyph rows of window W. All accessibility
+ attribute methods must use this (or its wrapper ns_ax_index_for_point)
+ to stay in the same coordinate space as accessibilityValue. */
+static NSUInteger
+ns_ax_index_for_point (struct window *w)
+ns_ax_index_for_charpos (struct window *w, ptrdiff_t charpos)
+{
+ if (!w || !w->current_matrix || !WINDOW_LEAF_P(w))
+ return 0;
+
+ struct buffer *b = XBUFFER(w->contents);
+ if (!b)
+ return 0;
+
+ ptrdiff_t point = BUF_PT(b);
+ struct glyph_matrix *matrix = w->current_matrix;
+ NSUInteger pos = 0;
+
@@ -428,10 +386,10 @@ MRC compatible.
+ ptrdiff_t row_start = MATRIX_ROW_START_CHARPOS (row);
+ ptrdiff_t row_end = MATRIX_ROW_END_CHARPOS (row);
+
+ if (point >= row_start && point < row_end)
+ if (charpos >= row_start && charpos < row_end)
+ {
+ /* Point is within this row. Count visible glyphs whose
+ buffer charpos is before point. */
+ /* Charpos is within this row. Count visible glyphs whose
+ buffer charpos is before the target. */
+ int chars_before = 0;
+ struct glyph *g = row->glyphs[TEXT_AREA];
+ struct glyph *gend = g + row->used[TEXT_AREA];
@@ -439,7 +397,7 @@ MRC compatible.
+ {
+ if (g->type == CHAR_GLYPH && !g->padding_p
+ && g->charpos >= row_start
+ && g->charpos < point)
+ && g->charpos < charpos)
+ {
+ unsigned ch = g->u.ch;
+ if (ch != '\n' && ch != '\r' && ch >= 32)
@@ -467,6 +425,18 @@ MRC compatible.
+ return pos > 0 ? pos - 1 : 0;
+}
+
+/* Convenience wrapper: glyph-based index for buffer point. */
+static NSUInteger
+ns_ax_index_for_point (struct window *w)
+{
+ if (!w || !WINDOW_LEAF_P(w))
+ return 0;
+ struct buffer *b = XBUFFER(w->contents);
+ if (!b)
+ return 0;
+ return ns_ax_index_for_charpos (w, BUF_PT(b));
+}
+
+
+@implementation EmacsAccessibilityElement
+
@@ -596,17 +566,14 @@ MRC compatible.
+ if (NILP(BVAR(b, mark_active)))
+ return NSMakeRange(point_idx, 0);
+
+ /* With active mark, report the selection range. Map mark
+ position to accessibility index using the same glyph-based
+ mapping as point. */
+ /* With active mark, report the selection range. Both endpoints
+ must be mapped to glyph-based indices consistent with
+ accessibilityValue (via ns_ax_index_for_charpos). */
+ ptrdiff_t mark_pos = marker_position (BVAR (b, mark));
+ ptrdiff_t pt_pos = BUF_PT (b);
+ ptrdiff_t begv = BUF_BEGV (b);
+ ptrdiff_t sel_start = (mark_pos < pt_pos) ? mark_pos : pt_pos;
+ ptrdiff_t sel_end = (mark_pos < pt_pos) ? pt_pos : mark_pos;
+ NSUInteger start_idx = (NSUInteger) (sel_start - begv);
+ NSUInteger len = (NSUInteger) (sel_end - sel_start);
+ return NSMakeRange(start_idx, len);
+ NSUInteger mark_idx = ns_ax_index_for_charpos (w, mark_pos);
+ NSUInteger start_idx = MIN (point_idx, mark_idx);
+ NSUInteger end_idx = MAX (point_idx, mark_idx);
+ return NSMakeRange (start_idx, end_idx - start_idx);
+}
+
+- (NSInteger)accessibilityInsertionPointLineNumber
@@ -719,15 +686,10 @@ MRC compatible.
+ glyph_x += glyph->pixel_width;
+ }
+
+ /* Convert buffer charpos to accessibility index. */
+ struct buffer *b = XBUFFER (w->contents);
+ if (!b)
+ return NSMakeRange (0, 0);
+
+ ptrdiff_t idx = best_charpos - BUF_BEGV (b);
+ if (idx < 0) idx = 0;
+
+ return NSMakeRange ((NSUInteger) idx, 1);
+ /* Convert buffer charpos to glyph-based accessibility index,
+ consistent with accessibilityValue coordinate space. */
+ NSUInteger ax_idx = ns_ax_index_for_charpos (w, best_charpos);
+ return NSMakeRange (ax_idx, 1);
+}
+
+- (NSRange)accessibilityVisibleCharacterRange
@@ -762,22 +724,19 @@ MRC compatible.
+ ptrdiff_t modiff = BUF_MODIFF(b);
+ ptrdiff_t point = BUF_PT(b);
+
+ /* Text content changed? */
+ /* Text content changed? Post ValueChanged with typing echo userInfo.
+ Single notification (no bare ValueChanged first) — Chromium pattern.
+ kAXTextStateChangeTypeEdit = 1, kAXTextEditTypeTyping = 3
+ (verified from WebKit AXTextStateChangeIntent.h). */
+ if (modiff != self.cachedModiff)
+ {
+ self.cachedModiff = modiff;
+ NSAccessibilityPostNotification(self,
+ NSAccessibilityValueChangedNotification);
+
+ /* Rich typing echo for VoiceOver.
+ kAXTextStateChangeTypeEdit = 1, kAXTextEditTypeTyping = 3.
+ Must include AXTextChangeValues array for VoiceOver to speak. */
+ NSString *changedText = @"";
+ ptrdiff_t pt = BUF_PT (b);
+ if (pt > BUF_BEGV (b))
+ NSUInteger point_idx = ns_ax_index_for_point (w);
+ if (point_idx > 0)
+ {
+ NSRange charRange = NSMakeRange (
+ (NSUInteger)(pt - BUF_BEGV (b) - 1), 1);
+ NSRange charRange = NSMakeRange (point_idx - 1, 1);
+ changedText = [self accessibilityStringForRange:charRange];
+ if (!changedText)
+ changedText = @"";
@@ -795,12 +754,20 @@ MRC compatible.
+ self, NSAccessibilityValueChangedNotification, userInfo);
+ }
+
+ /* Cursor moved? */
+ /* Cursor moved? Post with userInfo so VoiceOver distinguishes
+ navigation from programmatic selection changes and triggers
+ line-by-line reading.
+ kAXTextStateChangeTypeSelectionMove = 2. */
+ if (point != self.cachedPoint)
+ {
+ self.cachedPoint = point;
+ NSAccessibilityPostNotification(self,
+ NSAccessibilitySelectedTextChangedNotification);
+ NSDictionary *moveInfo = @{
+ @"AXTextStateChangeType": @2
+ };
+ NSAccessibilityPostNotificationWithUserInfo(
+ self,
+ NSAccessibilitySelectedTextChangedNotification,
+ moveInfo);
+ }
+}
+
@@ -809,12 +776,10 @@ MRC compatible.
+#endif /* NS_IMPL_COCOA */
+
+
+/* ==========================================================================
+
EmacsView implementation
/* ==========================================================================
========================================================================== */
@@ -6889,6 +7610,7 @@
EmacsView implementation
@@ -6889,6 +7571,7 @@ - (void)dealloc
[layer release];
#endif
@@ -822,7 +787,7 @@ MRC compatible.
[[self menu] release];
[super dealloc];
}
@@ -8237,6 +8959,18 @@
@@ -8237,6 +8920,18 @@ - (void)windowDidBecomeKey /* for direct calls */
XSETFRAME (event.frame_or_window, emacsframe);
kbd_buffer_store_event (&event);
ns_send_appdefined (-1); // Kick main loop
@@ -841,7 +806,7 @@ MRC compatible.
}
@@ -9474,6 +10208,391 @@
@@ -9474,6 +10169,213 @@ - (int) fullscreenState
return fs_state;
}
@@ -876,6 +841,7 @@ MRC compatible.
+ }
+
+ [elements addObject:elem];
+ [elem release]; /* array retains; balance the alloc */
+ }
+ else
+ {
@@ -1016,185 +982,6 @@ MRC compatible.
+ return [self accessibilityBoundsForRange:range];
+}
+
+/* ---- Text content methods (for Zoom and legacy AT) ---- */
+
+- (id)accessibilityValue
+{
+ if (!emacsframe)
+ return @"";
+
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf)
+ return @"";
+
+ ptrdiff_t start_byte = BUF_BEGV_BYTE (curbuf);
+ ptrdiff_t byte_range = BUF_ZV_BYTE (curbuf) - start_byte;
+ ptrdiff_t range = BUF_ZV (curbuf) - BUF_BEGV (curbuf);
+
+ if (range > 10000)
+ {
+ range = 10000;
+ ptrdiff_t end_byte = buf_charpos_to_bytepos (curbuf,
+ BUF_BEGV (curbuf) + range);
+ byte_range = end_byte - start_byte;
+ }
+
+ Lisp_Object str;
+ if (! NILP (BVAR (curbuf, enable_multibyte_characters)))
+ str = make_uninit_multibyte_string (range, byte_range);
+ else
+ str = make_uninit_string (range);
+ memcpy (SDATA (str), BYTE_POS_ADDR (start_byte), byte_range);
+
+ return [NSString stringWithLispString:str];
+}
+
+- (NSRange)accessibilitySelectedTextRange
+{
+ if (!emacsframe)
+ return NSMakeRange (0, 0);
+
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf)
+ return NSMakeRange (0, 0);
+
+ ptrdiff_t pt = BUF_PT (curbuf) - BUF_BEGV (curbuf);
+ return NSMakeRange ((NSUInteger) pt, 0);
+}
+
+- (NSString *)accessibilityStringForRange:(NSRange)nsrange
+{
+ if (!emacsframe)
+ return @"";
+
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf)
+ return @"";
+
+ ptrdiff_t start = BUF_BEGV (curbuf) + (ptrdiff_t) nsrange.location;
+ ptrdiff_t end = start + (ptrdiff_t) nsrange.length;
+ ptrdiff_t buf_end = BUF_ZV (curbuf);
+
+ if (start < BUF_BEGV (curbuf)) start = BUF_BEGV (curbuf);
+ if (end > buf_end) end = buf_end;
+ if (start >= end) return @"";
+
+ ptrdiff_t start_byte = buf_charpos_to_bytepos (curbuf, start);
+ ptrdiff_t end_byte = buf_charpos_to_bytepos (curbuf, end);
+ ptrdiff_t char_range = end - start;
+ ptrdiff_t brange = end_byte - start_byte;
+
+ Lisp_Object str;
+ if (! NILP (BVAR (curbuf, enable_multibyte_characters)))
+ str = make_uninit_multibyte_string (char_range, brange);
+ else
+ str = make_uninit_string (char_range);
+ memcpy (SDATA (str), BUF_BYTE_ADDRESS (curbuf, start_byte), brange);
+
+ return [NSString stringWithLispString:str];
+}
+
+- (NSInteger)accessibilityNumberOfCharacters
+{
+ if (!emacsframe)
+ return 0;
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf)
+ return 0;
+ ptrdiff_t range = BUF_ZV (curbuf) - BUF_BEGV (curbuf);
+ return (NSInteger) MIN (range, 10000);
+}
+
+- (NSString *)accessibilitySelectedText
+{
+ if (!emacsframe)
+ return @"";
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf || NILP (BVAR (curbuf, mark_active)))
+ return @"";
+ return @"";
+}
+
+- (NSInteger)accessibilityInsertionPointLineNumber
+{
+ if (!emacsframe)
+ return 0;
+ struct window *w = XWINDOW (emacsframe->selected_window);
+ if (!w)
+ return 0;
+ return (NSInteger) (w->cursor.vpos);
+}
+
+- (NSRange)accessibilityVisibleCharacterRange
+{
+ if (!emacsframe)
+ return NSMakeRange (0, 0);
+ struct buffer *curbuf
+ = XBUFFER (XWINDOW (emacsframe->selected_window)->contents);
+ if (!curbuf)
+ return NSMakeRange (0, 0);
+ ptrdiff_t range = BUF_ZV (curbuf) - BUF_BEGV (curbuf);
+ return NSMakeRange (0, (NSUInteger) MIN (range, 10000));
+}
+
+- (NSAttributedString *)accessibilityAttributedStringForRange:(NSRange)range
+{
+ NSString *str = [self accessibilityStringForRange:range];
+ return [[[NSAttributedString alloc] initWithString:str] autorelease];
+}
+
+- (NSInteger)accessibilityLineForIndex:(NSInteger)index
+{
+ if (!emacsframe)
+ return 0;
+ struct window *w = XWINDOW (emacsframe->selected_window);
+ if (!w || !w->current_matrix)
+ return 0;
+ struct buffer *curbuf = XBUFFER (w->contents);
+ if (!curbuf)
+ return 0;
+ ptrdiff_t charpos = BUF_BEGV (curbuf) + (ptrdiff_t) index;
+ struct glyph_matrix *matrix = w->current_matrix;
+ for (int i = 0; i < matrix->nrows; i++)
+ {
+ struct glyph_row *row = matrix->rows + i;
+ if (!row->enabled_p)
+ continue;
+ if (MATRIX_ROW_START_CHARPOS (row) <= charpos
+ && charpos < MATRIX_ROW_END_CHARPOS (row))
+ return (NSInteger) i;
+ }
+ return 0;
+}
+
+- (NSRange)accessibilityRangeForLine:(NSInteger)line
+{
+ if (!emacsframe)
+ return NSMakeRange (0, 0);
+ struct window *w = XWINDOW (emacsframe->selected_window);
+ if (!w || !w->current_matrix)
+ return NSMakeRange (0, 0);
+ struct buffer *curbuf = XBUFFER (w->contents);
+ if (!curbuf)
+ return NSMakeRange (0, 0);
+ struct glyph_matrix *matrix = w->current_matrix;
+ if (line < 0 || line >= matrix->nrows)
+ return NSMakeRange (0, 0);
+ struct glyph_row *row = matrix->rows + line;
+ if (!row->enabled_p)
+ return NSMakeRange (0, 0);
+ ptrdiff_t start = MATRIX_ROW_START_CHARPOS (row) - BUF_BEGV (curbuf);
+ ptrdiff_t end = MATRIX_ROW_END_CHARPOS (row) - BUF_BEGV (curbuf);
+ if (start < 0) start = 0;
+ if (end < start) end = start;
+ return NSMakeRange ((NSUInteger) start, (NSUInteger) (end - start));
+}
+
+/* ---- Legacy parameterized attribute APIs (Zoom uses these) ---- */
+
+- (NSArray *)accessibilityParameterizedAttributeNames
@@ -1233,7 +1020,7 @@ MRC compatible.
@end /* EmacsView */
@@ -9941,6 +11060,14 @@
@@ -9941,6 +10843,14 @@ - (id)accessibilityAttributeValue:(NSString *)attribute
return [super accessibilityAttributeValue:attribute];
}