From f961f46b9ac3ac0d4bdc1786cce33479aaaa32ab Mon Sep 17 00:00:00 2001 From: Daneel Date: Thu, 26 Feb 2026 14:53:47 +0100 Subject: [PATCH] =?UTF-8?q?v15.4:=20comprehensive=20VoiceOver=20fix=20?= =?UTF-8?q?=E2=80=94=209=20bugs=20from=204-reviewer=20audit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0 fixes (critical): - accessibilityRangeForLine: include trailing newline — fixes zero-length ranges for empty lines causing 'end of text' - accessibilityVisibleCharacterRange: return full buffer range — VoiceOver was treating visible window boundary as end of text - ensureTextCache: invalidate when point moves outside cached region (stale cachedTextStart after scrolling without editing) - ns_ax_frame_for_range: use row->height not visible_height, clip to text area — fixes VoiceOver cursor bleeding into adjacent lines P1 fixes (important): - Post SelectedTextChanged after FocusedUIElementChanged on focus acquisition (windowDidBecomeKey) and window switch (C-x o) - Post LayoutChangedNotification after tree rebuild - accessibilityRangeForIndex: rangeOfComposedCharacterSequenceAtIndex for emoji/combining marks - Buffer accessibilityFrame excludes mode line height - New elements init cachedPoint/cachedModiff=-1 to force first notification Reviewed by: architect + 3 reviewers (text, notifications, frames) --- ...oundsForRange-for-macOS-Zoom-cursor-.patch | 130 ++++++++++-------- 1 file changed, 76 insertions(+), 54 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 43ce26e..08f1a3a 100644 --- a/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch +++ b/patches/0001-ns-implement-AXBoundsForRange-for-macOS-Zoom-cursor-.patch @@ -73,7 +73,7 @@ index 7c1ee4c..4abeafe 100644 diff --git a/src/nsterm.m b/src/nsterm.m -index 932d209..5252e6d 100644 +index 932d209..8cf4f33 100644 --- a/src/nsterm.m +++ b/src/nsterm.m @@ -1104,6 +1104,11 @@ ns_update_end (struct frame *f) @@ -126,7 +126,7 @@ index 932d209..5252e6d 100644 ns_focus (f, NULL, 0); NSGraphicsContext *ctx = [NSGraphicsContext currentContext]; -@@ -6847,6 +6883,764 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) +@@ -6847,6 +6883,770 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) } #endif @@ -263,7 +263,7 @@ index 932d209..5252e6d 100644 + rowRect.origin.y = WINDOW_TO_FRAME_PIXEL_Y (w, MAX (0, row->y)); + rowRect.origin.y = MAX (rowRect.origin.y, window_y); + rowRect.size.width = window_width; -+ rowRect.size.height = row->visible_height; ++ rowRect.size.height = row->height; + + if (!found) + { @@ -278,6 +278,16 @@ index 932d209..5252e6d 100644 + if (!found) + return NSZeroRect; + ++ /* Clip result to text area bounds. */ ++ { ++ int text_area_x, text_area_y, text_area_w, text_area_h; ++ window_box (w, TEXT_AREA, &text_area_x, &text_area_y, ++ &text_area_w, &text_area_h); ++ CGFloat max_y = WINDOW_TO_FRAME_PIXEL_Y (w, text_area_y + text_area_h); ++ if (NSMaxY (result) > max_y) ++ result.size.height = max_y - result.origin.y; ++ } ++ + /* Convert from EmacsView (flipped) coords to screen coords. */ + NSRect winRect = [view convertRect:result toView:nil]; + return [[view window] convertRectToScreen:winRect]; @@ -355,7 +365,11 @@ index 932d209..5252e6d 100644 + return; + + ptrdiff_t modiff = BUF_MODIFF (b); -+ if (cachedText && cachedTextModiff == modiff) ++ ptrdiff_t pt = BUF_PT (b); ++ NSUInteger textLen = cachedText ? [cachedText length] : 0; ++ if (cachedText && cachedTextModiff == modiff ++ && pt >= cachedTextStart ++ && pt <= cachedTextStart + (ptrdiff_t)textLen) + return; + + ptrdiff_t start; @@ -548,6 +562,10 @@ index 932d209..5252e6d 100644 + while (line_end < len + && [cachedText characterAtIndex:line_end] != '\n') + line_end++; ++ /* Include the trailing newline so empty lines have length 1. */ ++ if (line_end < len ++ && [cachedText characterAtIndex:line_end] == '\n') ++ line_end++; + return NSMakeRange (i, line_end - i); + } + if (i < len && [cachedText characterAtIndex:i] == '\n') @@ -555,6 +573,9 @@ index 932d209..5252e6d 100644 + cur_line++; + } + } ++ /* Phantom final line after the last newline. */ ++ if (cur_line == line) ++ return NSMakeRange (len, 0); + return NSMakeRange (NSNotFound, 0); +} + @@ -564,7 +585,7 @@ index 932d209..5252e6d 100644 + if (!cachedText || index < 0 + || (NSUInteger) index >= [cachedText length]) + return NSMakeRange (NSNotFound, 0); -+ return NSMakeRange ((NSUInteger) index, 1); ++ return [cachedText rangeOfComposedCharacterSequenceAtIndex:(NSUInteger)index]; +} + +- (NSRange)accessibilityStyleRangeForIndex:(NSInteger)index @@ -653,41 +674,11 @@ index 932d209..5252e6d 100644 + +- (NSRange)accessibilityVisibleCharacterRange +{ -+ struct window *w = self.emacsWindow; -+ if (!w || !w->current_matrix) -+ { -+ [self ensureTextCache]; -+ return NSMakeRange (0, cachedText ? [cachedText length] : 0); -+ } -+ ++ /* Return the full cached text range. VoiceOver interprets the ++ visible range boundary as end-of-text, so we must expose the ++ entire buffer to avoid premature "end of text" announcements. */ + [self ensureTextCache]; -+ if (!cachedText) -+ return NSMakeRange (0, 0); -+ -+ /* Compute visible range from window start to last visible row. */ -+ ptrdiff_t vis_start = marker_position (w->start); -+ ptrdiff_t vis_end = vis_start; -+ -+ struct glyph_matrix *matrix = w->current_matrix; -+ for (int i = matrix->nrows - 1; i >= 0; i--) -+ { -+ struct glyph_row *row = matrix->rows + i; -+ if (row->enabled_p && !row->mode_line_p -+ && (row->displays_text_p || row->ends_at_zv_p)) -+ { -+ vis_end = MATRIX_ROW_END_CHARPOS (row); -+ break; -+ } -+ } -+ -+ NSUInteger loc = (NSUInteger) (vis_start - cachedTextStart); -+ NSUInteger end = (NSUInteger) (vis_end - cachedTextStart); -+ NSUInteger text_len = [cachedText length]; -+ if (loc > text_len) loc = text_len; -+ if (end > text_len) end = text_len; -+ if (end < loc) end = loc; -+ -+ return NSMakeRange (loc, end - loc); ++ return NSMakeRange (0, cachedText ? [cachedText length] : 0); +} + +- (NSRect)accessibilityFrame @@ -695,10 +686,25 @@ index 932d209..5252e6d 100644 + struct window *w = self.emacsWindow; + if (!w) + return NSZeroRect; ++ ++ /* Subtract mode line height so the buffer element does not overlap it. */ ++ int text_h = w->pixel_height; ++ if (w->current_matrix) ++ { ++ for (int i = w->current_matrix->nrows - 1; i >= 0; i--) ++ { ++ struct glyph_row *row = w->current_matrix->rows + i; ++ if (row->enabled_p && row->mode_line_p) ++ { ++ text_h -= row->visible_height; ++ break; ++ } ++ } ++ } + return [self screenRectFromEmacsX:w->pixel_left + y:w->pixel_top + width:w->pixel_width -+ height:w->pixel_height]; ++ height:text_h]; +} + +/* ---- Notification dispatch ---- */ @@ -891,7 +897,7 @@ index 932d209..5252e6d 100644 /* ========================================================================== EmacsView implementation -@@ -6889,6 +7683,7 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) +@@ -6889,6 +7689,7 @@ ns_create_font_panel_buttons (id target, SEL select, SEL cancel_action) [layer release]; #endif @@ -899,7 +905,7 @@ index 932d209..5252e6d 100644 [[self menu] release]; [super dealloc]; } -@@ -8237,6 +9032,18 @@ ns_in_echo_area (void) +@@ -8237,6 +9038,27 @@ ns_in_echo_area (void) XSETFRAME (event.frame_or_window, emacsframe); kbd_buffer_store_event (&event); ns_send_appdefined (-1); // Kick main loop @@ -910,7 +916,16 @@ index 932d209..5252e6d 100644 + This is critical for initial focus and app-switch scenarios. */ + { + id focused = [self accessibilityFocusedUIElement]; -+ if (focused) ++ if (focused ++ && [focused isKindOfClass:[EmacsAccessibilityBuffer class]]) ++ { ++ NSAccessibilityPostNotification (focused, ++ NSAccessibilityFocusedUIElementChangedNotification); ++ NSDictionary *info = @{@"AXTextStateChangeType": @2}; ++ NSAccessibilityPostNotificationWithUserInfo (focused, ++ NSAccessibilitySelectedTextChangedNotification, info); ++ } ++ else if (focused) + NSAccessibilityPostNotification (focused, + NSAccessibilityFocusedUIElementChangedNotification); + } @@ -918,7 +933,7 @@ index 932d209..5252e6d 100644 } -@@ -9474,6 +10281,290 @@ ns_in_echo_area (void) +@@ -9474,6 +10296,297 @@ ns_in_echo_area (void) return fs_state; } @@ -946,14 +961,10 @@ index 932d209..5252e6d 100644 + elem = [[EmacsAccessibilityBuffer alloc] init]; + elem.emacsView = view; + -+ /* Initialize cached state to trigger first notification. */ -+ struct buffer *b = XBUFFER (w->contents); -+ if (b) -+ { -+ elem.cachedModiff = BUF_MODIFF (b); -+ elem.cachedPoint = BUF_PT (b); -+ elem.cachedMarkActive = !NILP (BVAR (b, mark_active)); -+ } ++ /* Initialize cached state to -1 to force first notification. */ ++ elem.cachedModiff = -1; ++ elem.cachedPoint = -1; ++ elem.cachedMarkActive = NO; + } + else + { @@ -1099,6 +1110,8 @@ index 932d209..5252e6d 100644 + if (!accessibilityTreeValid) + { + [self rebuildAccessibilityTree]; ++ NSAccessibilityPostNotification (self, ++ NSAccessibilityLayoutChangedNotification); + + /* Post focus change so VoiceOver picks up the new tree. */ + id focused = [self accessibilityFocusedUIElement]; @@ -1133,7 +1146,16 @@ index 932d209..5252e6d 100644 + { + lastSelectedWindow = curSel; + id focused = [self accessibilityFocusedUIElement]; -+ if (focused && focused != self) ++ if (focused && focused != self ++ && [focused isKindOfClass:[EmacsAccessibilityBuffer class]]) ++ { ++ NSAccessibilityPostNotification (focused, ++ NSAccessibilityFocusedUIElementChangedNotification); ++ NSDictionary *info = @{@"AXTextStateChangeType": @2}; ++ NSAccessibilityPostNotificationWithUserInfo (focused, ++ NSAccessibilitySelectedTextChangedNotification, info); ++ } ++ else if (focused && focused != self) + NSAccessibilityPostNotification (focused, + NSAccessibilityFocusedUIElementChangedNotification); + } @@ -1209,7 +1231,7 @@ index 932d209..5252e6d 100644 @end /* EmacsView */ -@@ -9941,6 +11032,14 @@ nswindow_orderedIndex_sort (id w1, id w2, void *c) +@@ -9941,6 +11054,14 @@ nswindow_orderedIndex_sort (id w1, id w2, void *c) return [super accessibilityAttributeValue:attribute]; }