patches: fix all review blockers (iteration 2)
Fixes from Opus maintainer review: 1. [BLOCKER] Zoom code completely removed from ALL intermediate patches (0005-0007 no longer have UAZoom/overlayZoom at any commit point) 2. [BLOCKER] Unified cursor rect ivar: lastCursorRect (was split between lastZoomCursorRect and lastAccessibilityCursorRect) 3. [HIGH] Child frame static vars moved to EmacsView ivars (childFrameLastCandidate/Buffer/Modiff — no cross-frame interference) 4. [HIGH] intern_c_string replaced with Qbefore_string/Qafter_string 5. [MEDIUM] Zoom fallback gated by zoomCursorUpdated flag (no double call)
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
From 992bd78124b769fdcb4c1e3231afd13349e066c9 Mon Sep 17 00:00:00 2001
|
||||
From f842c3813d3b9fd106d668fa872b1cb3b187e9cf Mon Sep 17 00:00:00 2001
|
||||
From: Martin Sukany <martin@sukany.cz>
|
||||
Date: Sat, 28 Feb 2026 16:01:29 +0100
|
||||
Subject: [PATCH 8/8] ns: announce child frame completion candidates for
|
||||
@@ -40,11 +40,11 @@ childFrameCompletionActive flag.
|
||||
(EmacsView postAccessibilityUpdates): Dispatch to child frame handler,
|
||||
refocus parent buffer element when child frame closes.
|
||||
---
|
||||
doc/emacs/macos.texi | 6 -
|
||||
doc/emacs/macos.texi | 6 --
|
||||
etc/NEWS | 4 +-
|
||||
src/nsterm.h | 4 +-
|
||||
src/nsterm.m | 310 ++++++++++++++++++++++++++++++++-----------
|
||||
4 files changed, 238 insertions(+), 86 deletions(-)
|
||||
src/nsterm.h | 5 +
|
||||
src/nsterm.m | 227 ++++++++++++++++++++++++++++++++++++++++++-
|
||||
4 files changed, 233 insertions(+), 9 deletions(-)
|
||||
|
||||
diff --git a/doc/emacs/macos.texi b/doc/emacs/macos.texi
|
||||
index 4825cf9..97777e2 100644
|
||||
@@ -86,20 +86,21 @@ index e76ee93..c3e0b40 100644
|
||||
for the *Completions* buffer. The implementation uses a virtual
|
||||
accessibility tree with per-window elements, hybrid SelectedTextChanged
|
||||
diff --git a/src/nsterm.h b/src/nsterm.h
|
||||
index a007925..80bc8ff 100644
|
||||
index 19a7e7a..49e8f00 100644
|
||||
--- a/src/nsterm.h
|
||||
+++ b/src/nsterm.h
|
||||
@@ -596,8 +596,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
|
||||
@@ -596,6 +596,10 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
|
||||
BOOL accessibilityUpdating;
|
||||
@public /* Accessed by ns_draw_phys_cursor (C function). */
|
||||
NSRect lastAccessibilityCursorRect;
|
||||
- BOOL overlayZoomActive;
|
||||
- NSRect overlayZoomRect;
|
||||
NSRect lastCursorRect;
|
||||
+ BOOL childFrameCompletionActive;
|
||||
+ char *childFrameLastCandidate;
|
||||
+ struct buffer *childFrameLastBuffer;
|
||||
+ EMACS_INT childFrameLastModiff;
|
||||
#endif
|
||||
BOOL font_panel_active;
|
||||
NSFont *font_panel_result;
|
||||
@@ -661,6 +660,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
|
||||
@@ -659,6 +663,7 @@ typedef NS_ENUM (NSInteger, EmacsAXSpanType)
|
||||
- (void)rebuildAccessibilityTree;
|
||||
- (void)invalidateAccessibilityTree;
|
||||
- (void)postAccessibilityUpdates;
|
||||
@@ -108,60 +109,10 @@ index a007925..80bc8ff 100644
|
||||
@end
|
||||
|
||||
diff --git a/src/nsterm.m b/src/nsterm.m
|
||||
index 7025e6e..d5d33b0 100644
|
||||
index 3d72b5d..fefe2a7 100644
|
||||
--- a/src/nsterm.m
|
||||
+++ b/src/nsterm.m
|
||||
@@ -3239,44 +3239,14 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
|
||||
r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
|
||||
|
||||
#ifdef NS_IMPL_COCOA
|
||||
- /* Accessibility: store cursor rect for Zoom and bounds queries.
|
||||
- Skipped when ns-accessibility-enabled is nil to avoid overhead.
|
||||
- VoiceOver notifications are handled solely by
|
||||
- postAccessibilityUpdates (called from ns_update_end)
|
||||
- to avoid duplicate notifications and mid-redisplay fragility. */
|
||||
+ /* Accessibility: store cursor rect for VoiceOver bounds queries.
|
||||
+ accessibilityBoundsForRange: / accessibilityFrameForRange:
|
||||
+ use this as a fallback when no valid window/glyph data is
|
||||
+ available. Skipped when ns-accessibility-enabled is nil. */
|
||||
{
|
||||
EmacsView *view = FRAME_NS_VIEW (f);
|
||||
if (view && on_p && active_p && ns_accessibility_enabled)
|
||||
- {
|
||||
- view->lastAccessibilityCursorRect = r;
|
||||
-
|
||||
- /* Tell macOS Zoom where the cursor is. UAZoomChangeFocus()
|
||||
- expects top-left origin (CG coordinate space).
|
||||
- These APIs are available since macOS 10.4 (Universal Access
|
||||
- framework, linked via ApplicationServices umbrella). */
|
||||
-#if defined (MAC_OS_X_VERSION_MIN_REQUIRED) \
|
||||
- && MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
|
||||
- if (UAZoomEnabled ())
|
||||
- {
|
||||
- /* When overlay completion is active (e.g. Vertico),
|
||||
- focus Zoom on the selected candidate row instead
|
||||
- of the text cursor. */
|
||||
- NSRect zoomSrc = view->overlayZoomActive
|
||||
- ? view->overlayZoomRect : r;
|
||||
- NSRect windowRect = [view convertRect:zoomSrc toView:nil];
|
||||
- NSRect screenRect = [[view window] convertRectToScreen:windowRect];
|
||||
- CGRect cgRect = NSRectToCGRect (screenRect);
|
||||
-
|
||||
- CGFloat primaryH
|
||||
- = [[[NSScreen screens] firstObject] frame].size.height;
|
||||
- cgRect.origin.y
|
||||
- = primaryH - cgRect.origin.y - cgRect.size.height;
|
||||
-
|
||||
- UAZoomChangeFocus (&cgRect, &cgRect,
|
||||
- kUAZoomFocusTypeInsertionPoint);
|
||||
- }
|
||||
-#endif /* MAC_OS_X_VERSION_MIN_REQUIRED >= 101000 */
|
||||
- }
|
||||
+ view->lastAccessibilityCursorRect = r;
|
||||
}
|
||||
#endif
|
||||
|
||||
@@ -7058,6 +7028,112 @@ visual line index for Zoom (skip whitespace-only lines
|
||||
@@ -7028,6 +7028,112 @@ visual line index for Zoom (skip whitespace-only lines
|
||||
|
||||
return nil;
|
||||
}
|
||||
@@ -274,63 +225,7 @@ index 7025e6e..d5d33b0 100644
|
||||
/* Build accessibility text for window W, skipping invisible text.
|
||||
Populates *OUT_START with the buffer start charpos.
|
||||
Populates *OUT_RUNS with an array of visible runs and *OUT_NRUNS
|
||||
@@ -8988,7 +9064,6 @@ Text property changes (e.g. face updates from
|
||||
if (chars_modiff != self.cachedCharsModiff)
|
||||
{
|
||||
self.cachedCharsModiff = chars_modiff;
|
||||
- self.emacsView->overlayZoomActive = NO;
|
||||
[self postTextChangedNotification:point];
|
||||
}
|
||||
}
|
||||
@@ -9036,47 +9111,8 @@ frameworks like Vertico bump BOTH BUF_MODIFF (via text property
|
||||
NSAccessibilityAnnouncementRequestedNotification,
|
||||
annInfo);
|
||||
|
||||
- /* --- Zoom tracking for overlay candidates ---
|
||||
- Store the candidate row rect so draw_window_cursor
|
||||
- focuses Zoom there instead of on the text cursor.
|
||||
- Cleared when the user types (chars_modiff change).
|
||||
-
|
||||
- Use default line height to compute the Y offset:
|
||||
- row 0 is the input line, overlay candidates start
|
||||
- from row 1. This avoids fragile glyph matrix row
|
||||
- index mapping which can be off when group titles
|
||||
- or wrapped lines shift row numbering. */
|
||||
- if (selected_line >= 0)
|
||||
- {
|
||||
- struct window *w2 = [self validWindow];
|
||||
- if (w2)
|
||||
- {
|
||||
- EmacsView *view = self.emacsView;
|
||||
- struct frame *f2 = XFRAME (w2->frame);
|
||||
- int line_h = FRAME_LINE_HEIGHT (f2);
|
||||
- int y_off = (selected_line + 1) * line_h;
|
||||
-
|
||||
- if (y_off < w2->pixel_height)
|
||||
- {
|
||||
- view->overlayZoomRect = NSMakeRect (
|
||||
- WINDOW_TEXT_TO_FRAME_PIXEL_X (w2, 0),
|
||||
- WINDOW_TO_FRAME_PIXEL_Y (w2, y_off),
|
||||
- FRAME_COLUMN_WIDTH (f2),
|
||||
- line_h);
|
||||
- view->overlayZoomActive = YES;
|
||||
- }
|
||||
- }
|
||||
- }
|
||||
}
|
||||
}
|
||||
- else
|
||||
- {
|
||||
- /* No selected candidate --- overlay completion ended
|
||||
- (minibuffer exit, C-g, etc.) or overlay has no
|
||||
- recognizable selection face. Return Zoom to the
|
||||
- text cursor. */
|
||||
- self.emacsView->overlayZoomActive = NO;
|
||||
- }
|
||||
}
|
||||
|
||||
/* --- Cursor moved or selection changed ---
|
||||
@@ -12336,6 +12372,80 @@ - (id)accessibilityFocusedUIElement
|
||||
@@ -12266,6 +12372,77 @@ - (id)accessibilityFocusedUIElement
|
||||
The existing elements carry cached state (modiff, point) from the
|
||||
previous redisplay cycle. Rebuilding first would create fresh
|
||||
elements with current values, making change detection impossible. */
|
||||
@@ -341,9 +236,6 @@ index 7025e6e..d5d33b0 100644
|
||||
+ in the minibuffer. */
|
||||
+- (void)announceChildFrameCompletion
|
||||
+{
|
||||
+ static char *lastCandidate;
|
||||
+ static struct buffer *lastBuffer;
|
||||
+ static EMACS_INT lastModiff;
|
||||
+
|
||||
+ /* Validate frame state --- child frames may be partially
|
||||
+ initialized during creation. */
|
||||
@@ -359,10 +251,10 @@ index 7025e6e..d5d33b0 100644
|
||||
+ also guards against re-entrance: if Lisp calls below
|
||||
+ trigger redisplay, the modiff check short-circuits. */
|
||||
+ EMACS_INT modiff = BUF_MODIFF (b);
|
||||
+ if (b == lastBuffer && modiff == lastModiff)
|
||||
+ if (b == childFrameLastBuffer && modiff == childFrameLastModiff)
|
||||
+ return;
|
||||
+ lastBuffer = b;
|
||||
+ lastModiff = modiff;
|
||||
+ childFrameLastBuffer = b;
|
||||
+ childFrameLastModiff = modiff;
|
||||
+
|
||||
+ /* Skip buffers larger than a typical completion popup.
|
||||
+ This avoids scanning eldoc, which-key, or other child
|
||||
@@ -379,10 +271,10 @@ index 7025e6e..d5d33b0 100644
|
||||
+
|
||||
+ /* Deduplicate --- avoid re-announcing the same candidate. */
|
||||
+ const char *cstr = [candidate UTF8String];
|
||||
+ if (lastCandidate && strcmp (cstr, lastCandidate) == 0)
|
||||
+ if (childFrameLastCandidate && strcmp (cstr, childFrameLastCandidate) == 0)
|
||||
+ return;
|
||||
+ xfree (lastCandidate);
|
||||
+ lastCandidate = xstrdup (cstr);
|
||||
+ xfree (childFrameLastCandidate);
|
||||
+ childFrameLastCandidate = xstrdup (cstr);
|
||||
+
|
||||
+ NSDictionary *annInfo = @{
|
||||
+ NSAccessibilityAnnouncementKey: candidate,
|
||||
@@ -411,7 +303,7 @@ index 7025e6e..d5d33b0 100644
|
||||
- (void)postAccessibilityUpdates
|
||||
{
|
||||
NSTRACE ("[EmacsView postAccessibilityUpdates]");
|
||||
@@ -12346,11 +12456,59 @@ - (void)postAccessibilityUpdates
|
||||
@@ -12276,11 +12453,59 @@ - (void)postAccessibilityUpdates
|
||||
|
||||
/* Re-entrance guard: VoiceOver callbacks during notification posting
|
||||
can trigger redisplay, which calls ns_update_end, which calls us
|
||||
|
||||
Reference in New Issue
Block a user