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 e40d502d43

View File

@@ -52,7 +52,6 @@ MRC compatible.
+ Lisp_Object lastSelectedWindow; + Lisp_Object lastSelectedWindow;
+ @public + @public
+ NSRect lastAccessibilityCursorRect; + NSRect lastAccessibilityCursorRect;
+ ptrdiff_t lastAccessibilityModiff;
+ @protected + @protected
#endif #endif
BOOL font_panel_active; BOOL font_panel_active;
@@ -89,61 +88,16 @@ MRC compatible.
r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA)); r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
+#ifdef NS_IMPL_COCOA +#ifdef NS_IMPL_COCOA
+ /* Accessibility cursor tracking for macOS Zoom and VoiceOver. + /* Accessibility: store cursor rect for Zoom and bounds queries.
+ Only notify AT when drawing the cursor in the active (selected) + VoiceOver notifications are handled solely by
+ window. Without this guard, C-x o triggers UAZoomChangeFocus + postAccessibilityUpdatesForWindow: (called from ns_update_end)
+ for the old window last, snapping Zoom back. */ + to avoid duplicate notifications and mid-redisplay fragility. */
+ { + {
+ EmacsView *view = FRAME_NS_VIEW (f); + EmacsView *view = FRAME_NS_VIEW (f);
+ if (view && on_p && active_p) + if (view && on_p && active_p)
+ { + {
+ /* Store cursor rect for accessibilityBoundsForRange: queries. */
+ view->lastAccessibilityCursorRect = r; + 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() + /* Tell macOS Zoom where the cursor is. UAZoomChangeFocus()
+ expects top-left origin (CG coordinate space). */ + expects top-left origin (CG coordinate space). */
+ if (UAZoomEnabled ()) + if (UAZoomEnabled ())
@@ -401,19 +355,16 @@ MRC compatible.
+} +}
+ +
+ +
+/* Compute the character index within glyph-extracted text that +/* Compute the glyph-based character index for an arbitrary buffer
+ corresponds to the buffer point position. */ + 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 +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)) + if (!w || !w->current_matrix || !WINDOW_LEAF_P(w))
+ return 0; + 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; + struct glyph_matrix *matrix = w->current_matrix;
+ NSUInteger pos = 0; + NSUInteger pos = 0;
+ +
@@ -428,10 +379,10 @@ MRC compatible.
+ ptrdiff_t row_start = MATRIX_ROW_START_CHARPOS (row); + ptrdiff_t row_start = MATRIX_ROW_START_CHARPOS (row);
+ ptrdiff_t row_end = MATRIX_ROW_END_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 + /* Charpos is within this row. Count visible glyphs whose
+ buffer charpos is before point. */ + buffer charpos is before the target. */
+ int chars_before = 0; + int chars_before = 0;
+ struct glyph *g = row->glyphs[TEXT_AREA]; + struct glyph *g = row->glyphs[TEXT_AREA];
+ struct glyph *gend = g + row->used[TEXT_AREA]; + struct glyph *gend = g + row->used[TEXT_AREA];
@@ -439,7 +390,7 @@ MRC compatible.
+ { + {
+ if (g->type == CHAR_GLYPH && !g->padding_p + if (g->type == CHAR_GLYPH && !g->padding_p
+ && g->charpos >= row_start + && g->charpos >= row_start
+ && g->charpos < point) + && g->charpos < charpos)
+ { + {
+ unsigned ch = g->u.ch; + unsigned ch = g->u.ch;
+ if (ch != '\n' && ch != '\r' && ch >= 32) + if (ch != '\n' && ch != '\r' && ch >= 32)
@@ -467,6 +418,18 @@ MRC compatible.
+ return pos > 0 ? pos - 1 : 0; + 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 +@implementation EmacsAccessibilityElement
+ +
@@ -596,17 +559,14 @@ MRC compatible.
+ if (NILP(BVAR(b, mark_active))) + if (NILP(BVAR(b, mark_active)))
+ return NSMakeRange(point_idx, 0); + return NSMakeRange(point_idx, 0);
+ +
+ /* With active mark, report the selection range. Map mark + /* With active mark, report the selection range. Both endpoints
+ position to accessibility index using the same glyph-based + must be mapped to glyph-based indices consistent with
+ mapping as point. */ + accessibilityValue (via ns_ax_index_for_charpos). */
+ ptrdiff_t mark_pos = marker_position (BVAR (b, mark)); + ptrdiff_t mark_pos = marker_position (BVAR (b, mark));
+ ptrdiff_t pt_pos = BUF_PT (b); + NSUInteger mark_idx = ns_ax_index_for_charpos (w, mark_pos);
+ ptrdiff_t begv = BUF_BEGV (b); + NSUInteger start_idx = MIN (point_idx, mark_idx);
+ ptrdiff_t sel_start = (mark_pos < pt_pos) ? mark_pos : pt_pos; + NSUInteger end_idx = MAX (point_idx, mark_idx);
+ ptrdiff_t sel_end = (mark_pos < pt_pos) ? pt_pos : mark_pos; + return NSMakeRange (start_idx, end_idx - start_idx);
+ NSUInteger start_idx = (NSUInteger) (sel_start - begv);
+ NSUInteger len = (NSUInteger) (sel_end - sel_start);
+ return NSMakeRange(start_idx, len);
+} +}
+ +
+- (NSInteger)accessibilityInsertionPointLineNumber +- (NSInteger)accessibilityInsertionPointLineNumber
@@ -719,15 +679,10 @@ MRC compatible.
+ glyph_x += glyph->pixel_width; + glyph_x += glyph->pixel_width;
+ } + }
+ +
+ /* Convert buffer charpos to accessibility index. */ + /* Convert buffer charpos to glyph-based accessibility index,
+ struct buffer *b = XBUFFER (w->contents); + consistent with accessibilityValue coordinate space. */
+ if (!b) + NSUInteger ax_idx = ns_ax_index_for_charpos (w, best_charpos);
+ return NSMakeRange (0, 0); + return NSMakeRange (ax_idx, 1);
+
+ ptrdiff_t idx = best_charpos - BUF_BEGV (b);
+ if (idx < 0) idx = 0;
+
+ return NSMakeRange ((NSUInteger) idx, 1);
+} +}
+ +
+- (NSRange)accessibilityVisibleCharacterRange +- (NSRange)accessibilityVisibleCharacterRange
@@ -762,22 +717,19 @@ MRC compatible.
+ ptrdiff_t modiff = BUF_MODIFF(b); + ptrdiff_t modiff = BUF_MODIFF(b);
+ ptrdiff_t point = BUF_PT(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) + if (modiff != self.cachedModiff)
+ { + {
+ self.cachedModiff = modiff; + 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 = @""; + NSString *changedText = @"";
+ ptrdiff_t pt = BUF_PT (b); + NSUInteger point_idx = ns_ax_index_for_point (w);
+ if (pt > BUF_BEGV (b)) + if (point_idx > 0)
+ { + {
+ NSRange charRange = NSMakeRange ( + NSRange charRange = NSMakeRange (point_idx - 1, 1);
+ (NSUInteger)(pt - BUF_BEGV (b) - 1), 1);
+ changedText = [self accessibilityStringForRange:charRange]; + changedText = [self accessibilityStringForRange:charRange];
+ if (!changedText) + if (!changedText)
+ changedText = @""; + changedText = @"";
@@ -795,12 +747,20 @@ MRC compatible.
+ self, NSAccessibilityValueChangedNotification, userInfo); + 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) + if (point != self.cachedPoint)
+ { + {
+ self.cachedPoint = point; + self.cachedPoint = point;
+ NSAccessibilityPostNotification(self, + NSDictionary *moveInfo = @{
+ NSAccessibilitySelectedTextChangedNotification); + @"AXTextStateChangeType": @2
+ };
+ NSAccessibilityPostNotificationWithUserInfo(
+ self,
+ NSAccessibilitySelectedTextChangedNotification,
+ moveInfo);
+ } + }
+} +}
+ +
@@ -876,6 +836,7 @@ MRC compatible.
+ } + }
+ +
+ [elements addObject:elem]; + [elements addObject:elem];
+ [elem release]; /* array retains; balance the alloc */
+ } + }
+ else + else
+ { + {
@@ -1016,185 +977,6 @@ MRC compatible.
+ return [self accessibilityBoundsForRange:range]; + 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) ---- */ +/* ---- Legacy parameterized attribute APIs (Zoom uses these) ---- */
+ +
+- (NSArray *)accessibilityParameterizedAttributeNames +- (NSArray *)accessibilityParameterizedAttributeNames