v15.4: comprehensive VoiceOver fix — 9 bugs from 4-reviewer audit

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)
2026-02-26 14:53:47 +01:00
parent fd523f501f
commit f961f46b9a
1 changed files with 76 additions and 54 deletions
--- 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];
 }