From e40d502d43a6b233b7d1600f1ea9217e97639d6b Mon Sep 17 00:00:00 2001 From: Daneel Date: Thu, 26 Feb 2026 11:44:13 +0100 Subject: [PATCH] =?UTF-8?q?ns:=20fix=20VoiceOver=20cursor=20sync=20?= =?UTF-8?q?=E2=80=94=208=20changes=20from=20pipeline=20review?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- ...oundsForRange-for-macOS-Zoom-cursor-.patch | 328 +++--------------- 1 file changed, 55 insertions(+), 273 deletions(-) diff --git a/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch b/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch index 25f621a..2424d7e 100644 --- a/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch +++ b/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch @@ -52,7 +52,6 @@ MRC compatible. + Lisp_Object lastSelectedWindow; + @public + NSRect lastAccessibilityCursorRect; -+ ptrdiff_t lastAccessibilityModiff; + @protected #endif BOOL font_panel_active; @@ -89,61 +88,16 @@ MRC compatible. 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 ()) @@ -401,19 +355,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 +379,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 +390,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 +418,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 +559,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 +679,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 +717,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 +747,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); + } +} + @@ -876,6 +836,7 @@ MRC compatible. + } + + [elements addObject:elem]; ++ [elem release]; /* array retains; balance the alloc */ + } + else + { @@ -1016,185 +977,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