Fix VoiceOver losing focus after completion popup closes

When a child frame completion popup appeared, VoiceOver could navigate into its accessibility tree. When the child frame was then hidden or destroyed, VoiceOver held a stale reference to the child frame's elements and stopped announcing typed characters in the parent frame. The root cause was that EmacsView reported all child frame views as accessible elements (isAccessibilityElement=YES) with a full accessibility tree. VoiceOver tracked into these elements; when they were destroyed, it lost track of the parent frame's buffer element. Fix this following macOS accessibility best practices for transient popup UI: 1. Make child frame EmacsView invisible to VoiceOver. 2. Post UIElementDestroyedNotification on child frame close. 3. Always post FocusedUIElementChanged on the parent. * src/nsterm.m (accessibilityRole): Return nil for child frame EmacsView instances. (isAccessibilityElement): Return NO for child frames. (accessibilityChildren): Return empty array for child frames. (accessibilityFocusedUIElement): Return self for child frames. (ns_ax_maybe_refocus_after_child_frame_close): Rewrite. Post UIElementDestroyedNotification on the child view. Post FocusedUIElementChanged unconditionally. (ns_make_frame_invisible): Update comment. (ns_destroy_window): Update comment. (postAccessibilityUpdates): Simplify child-frame-gone path; always post FocusedUIElementChanged.
2026-04-10 21:18:21 +02:00
parent 3bce527500
commit 3d5d9a3f1d
1 changed files with 90 additions and 52 deletions
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2049,35 +2049,61 @@ ns_make_frame_visible (struct frame *f)


 #ifdef NS_IMPL_COCOA
-/* When a child frame that was used as a completion popup is hidden or
-   destroyed, VoiceOver focus must be explicitly returned to the parent
-   frame's buffer element.  Without this, VoiceOver loses track of the
-   focused element and stops announcing typed characters.
+/* When a child frame is hidden or destroyed, clean up VoiceOver
+   state so it returns to the parent frame's buffer element.

-   The problem arises because hiding/deleting a child frame does not
-   necessarily trigger a redisplay of the parent frame (no buffer
-   content changed), so the parent's postAccessibilityUpdates --- which
-   normally detects child frame disappearance --- may never run.
+   Two problems must be addressed:

-   This function is safe to call from ns_make_frame_invisible and
-   ns_destroy_window: it performs no Lisp calls, only C struct access
-   and dispatch_async of an ObjC notification.  */
+   1. VoiceOver may be tracking the child frame's window or view
+      (despite our isAccessibilityElement=NO, VoiceOver can
+      still hold a stale reference from before the override took
+      effect, or from a race with window creation).  Posting
+      UIElementDestroyedNotification tells VoiceOver that the
+      element it was tracking no longer exists.
+
+   2. Hiding/deleting a child frame does not necessarily trigger
+      a redisplay of the parent frame (no buffer content
+      changed), so the parent's postAccessibilityUpdates may
+      never run.  Posting FocusedUIElementChanged on the
+      parent's buffer element tells VoiceOver where to go next.
+
+   Both notifications are posted unconditionally --- even during
+   typing.  When VoiceOver is stuck on a destroyed child frame
+   element, only an explicit FocusedUIElementChanged on the
+   parent can unstick it.  The per-element notification loop
+   (postAccessibilityNotificationsForFrame:) handles text-edit
+   and cursor-move speech; FocusedUIElementChanged merely tells
+   VoiceOver which element to track, it does not by itself
+   produce speech that would interrupt character echo.
+
+   This function is safe to call from ns_make_frame_invisible
+   and ns_destroy_window: it performs no Lisp calls, only C
+   struct access and dispatch_async of ObjC notifications.  */
 static void
 ns_ax_maybe_refocus_after_child_frame_close (struct frame *f)
 {
  if (!ns_accessibility_enabled)
    return;
+
+  EmacsView *childView = FRAME_NS_VIEW (f);
+
+  /* Tell VoiceOver the child frame element is going away.
+     This is the macOS-recommended way to clean up transient UI;
+     without it, VoiceOver holds a stale reference to the
+     destroyed element and stops tracking the parent.  */
+  if (childView)
+    NSAccessibilityPostNotification (
+      childView, NSAccessibilityUIElementDestroyedNotification);
+
  struct frame *parent = FRAME_PARENT_FRAME (f);
  if (!parent)
    return;
  EmacsView *parentView = FRAME_NS_VIEW (parent);
-  if (!parentView || !parentView->childFrameCompletionActive)
+  if (!parentView)
    return;

+  /* Reset completion tracking state.  */
  parentView->childFrameCompletionActive = NO;
-
-  /* Reset dedup state so the next completion session announces
-     its first candidate even if it matches the last one.  */
  if (parentView->childFrameLastCandidate)
    {
      xfree (parentView->childFrameLastCandidate);
@@ -2089,16 +2115,11 @@ ns_ax_maybe_refocus_after_child_frame_close (struct frame *f)
      parentView->childFrameLastBufferName = NULL;
    }

-  /* Tell VoiceOver to track the parent's focused buffer element.
-     Use accessibilityFocusedUIElement (public API) to find the
-     element, and dispatch_async + NSAccessibilityPostNotification
-     directly --- ns_ax_post_notification and its enum constants
-     are defined later in the file.  Post both
-     FocusedUIElementChanged ("this element has focus") and
-     SelectedTextChanged ("the cursor is here"); without the
-     latter, VoiceOver knows which element is focused but does not
-     re-query the insertion point, so character echo stays silent.
-     The literal 2 == ns_ax_text_state_change_selection_move.  */
+  /* Tell VoiceOver to focus the parent's buffer element.
+     Use dispatch_async because NSAccessibilityPostNotification
+     can synchronously invoke VoiceOver callbacks that
+     dispatch_sync back to the main queue; deferring avoids
+     a deadlock.  */
  id focused = [parentView accessibilityFocusedUIElement];
  if (focused && focused != (id)parentView)
    {
@@ -2107,14 +2128,6 @@ ns_ax_maybe_refocus_after_child_frame_close (struct frame *f)
 	  NSAccessibilityPostNotification (
 	    focused,
 	    NSAccessibilityFocusedUIElementChangedNotification);
-	  NSDictionary *info = @{
-	    @"AXTextStateChangeType": @(2),
-	    @"AXTextChangeElement": focused
-	  };
-	  NSAccessibilityPostNotificationWithUserInfo (
-	    focused,
-	    NSAccessibilitySelectedTextChangedNotification,
-	    info);
 	  [focused release];
 	});
    }
@@ -2131,7 +2144,7 @@ ns_make_frame_invisible (struct frame *f)
  NSTRACE ("ns_make_frame_invisible");
  check_window_system (f);
 #ifdef NS_IMPL_COCOA
-  /* Restore VoiceOver focus before the window is hidden.  */
+  /* Clean up VoiceOver state before the child frame is hidden.  */
  if (FRAME_PARENT_FRAME (f))
    ns_ax_maybe_refocus_after_child_frame_close (f);
 #endif
@@ -2243,8 +2256,9 @@ ns_destroy_window (struct frame *f)
  check_window_system (f);

 #ifdef NS_IMPL_COCOA
-  /* Restore VoiceOver focus before the child frame is detached and
-     freed.  Must run while FRAME_PARENT_FRAME is still valid.  */
+  /* Post UIElementDestroyedNotification and refocus VoiceOver on the
+     parent before the child frame is detached and freed.  Must run
+     while FRAME_PARENT_FRAME and FRAME_NS_VIEW are still valid.  */
  if (FRAME_PARENT_FRAME (f))
    ns_ax_maybe_refocus_after_child_frame_close (f);
 #endif
@@ -10296,7 +10310,7 @@ ns_ax_completion_text_for_span (EmacsAccessibilityBuffer *elem,
     Used below to suppress overlay completion announcements: when the
     user types, character echo (via postTextChangedNotification) must
     take priority over overlay candidate updates.  Without this guard,
-     Overlay completion frameworks update immediately after each keystroke,
+     overlay completion frameworks update immediately after each keystroke,
     and the High-priority overlay announcement interrupts the character
     echo, effectively silencing typed characters.  */
  BOOL didTextChange = NO;
@@ -13794,6 +13808,17 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,

 - (NSAccessibilityRole)accessibilityRole
 {
+  /* Child frame windows (used for completion popups, eldoc, etc.)
+     must not appear as separate groups in the accessibility
+     hierarchy.  VoiceOver navigates into accessible groups and
+     tracks their children; when the child frame is hidden or
+     destroyed, VoiceOver loses track of the focused element and
+     stops announcing typed characters in the parent frame.
+     Completion candidates are announced via
+     AnnouncementRequested from the parent, so child frames need
+     no accessibility role of their own.  */
+  if (emacsframe && FRAME_PARENT_FRAME (emacsframe))
+    return nil;
  return NSAccessibilityGroupRole;
 }

@@ -13804,11 +13829,19 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,

 - (BOOL)isAccessibilityElement
 {
+  /* See accessibilityRole for the rationale: child frame views
+     must be invisible to VoiceOver.  */
+  if (emacsframe && FRAME_PARENT_FRAME (emacsframe))
+    return NO;
  return YES;
 }

 - (NSArray *)accessibilityChildren
 {
+  /* Child frames are invisible to VoiceOver; report no children
+     so VoiceOver cannot navigate into the transient popup.  */
+  if (emacsframe && FRAME_PARENT_FRAME (emacsframe))
+    return @[];
  if (!accessibilityElements || !accessibilityTreeValid)
    [self rebuildAccessibilityTree];
  return accessibilityElements;
@@ -13819,6 +13852,12 @@ ns_ax_collect_windows (Lisp_Object window, EmacsView *view,
  if (!emacsframe)
    return self;

+  /* Child frames have no accessible children; return self
+     (which is not an accessibility element) so VoiceOver
+     does not track anything inside the child frame.  */
+  if (FRAME_PARENT_FRAME (emacsframe))
+    return self;
+
  if (!accessibilityElements || !accessibilityTreeValid)
    [self rebuildAccessibilityTree];

@@ -13981,8 +14020,8 @@ ns_ax_count_leaves (struct frame *f)
    return;

  /* Suppress completion announcements briefly after a text edit in the
-     parent frame.  When the user types, completion frameworks (Corfu,
-     Company) update their candidate list immediately, triggering a
+     parent frame.  When the user types, child frame completion
+     frameworks update their candidate list immediately, triggering a
     buffer change in the child frame.  Without this guard, the
     High-priority completion announcement interrupts VoiceOver's
     character echo from the parent frame's SelectedTextChanged
@@ -14184,6 +14223,15 @@ ns_ax_count_leaves (struct frame *f)
 	      xfree (childFrameLastBufferName);
 	      childFrameLastBufferName = NULL;
 	    }
+
+	  /* Always tell VoiceOver to focus the parent buffer
+	     element.  When VoiceOver was tracking the (now
+	     destroyed) child frame, only an explicit
+	     FocusedUIElementChanged can unstick it.
+	     FocusedUIElementChanged tells VoiceOver which
+	     element to track; it does not produce speech by
+	     itself, so it will not interrupt character echo
+	     from the per-element text-edit notifications.  */
 	  EmacsAccessibilityBuffer *focused = nil;
 	  for (id elem in accessibilityElements)
 	    if ([elem isKindOfClass:
@@ -14195,19 +14243,9 @@ ns_ax_count_leaves (struct frame *f)
 		break;
 	      }
 	  if (focused)
-	    {
-	      ns_ax_post_notification (
-		focused,
-		NSAccessibilityFocusedUIElementChangedNotification);
-	      NSDictionary *info = @{
-		@"AXTextStateChangeType":
-		  @(ns_ax_text_state_change_selection_move),
-		@"AXTextChangeElement": focused
-	      };
-	      ns_ax_post_notification_with_info (focused,
-		NSAccessibilitySelectedTextChangedNotification,
-		info);
-	    }
+	    ns_ax_post_notification (
+	      focused,
+	      NSAccessibilityFocusedUIElementChangedNotification);
 	}
    }