v12 patch: visual line numbering for VoiceOver (3 iterations)

Root cause: all accessibility line methods used count_lines (logical/
newline-counting), but VoiceOver compares line numbers before/after
cursor movement. Wrapped lines had same logical line number, so VO
fell back to reading single characters instead of full lines.

Fix: replaced count_lines/find_newline_no_quit with compute_motion/
vmotion (visual screen lines) in all three methods. Matches iTerm2's
approach of returning screen rows.

Changes from v11:
- accessibilityInsertionPointLineNumber: compute_motion visual lines
- accessibilityLineForIndex: compute_motion visual lines
- accessibilityRangeForLine: vmotion for line start/end
- SelectedRowsChanged only on visual line change (new ivar tracking)
- accessibilityLabel returns buffer name for window switch
- No double ValueChanged on edits (content_changed flag)
- Trailing newline stripped from line ranges
- compute_motion width=-1 for consistency with vmotion

3 pipeline iterations, scores: 78 → 92 → 95
This commit is contained in:
2026-02-25 22:13:28 +01:00
parent 2f1fbd30d2
commit 2c78f04089

View File

@@ -1,32 +1,35 @@
From: Martin Sukany <martin@sukany.cz> From: Martin Sukany <martin@sukany.cz>
Date: Tue, 25 Feb 2026 21:40:00 +0100 Date: Tue, 25 Feb 2026 22:10:00 +0100
Subject: [PATCH] ns: macOS Zoom cursor tracking and VoiceOver support Subject: [PATCH] ns: macOS Zoom cursor tracking and VoiceOver support
Comprehensive accessibility for the macOS NS port: Comprehensive accessibility for the macOS NS port:
1. UAZoomChangeFocus() for Zoom viewport tracking (on_p && active_p guard). 1. UAZoomChangeFocus() for Zoom viewport tracking (on_p && active_p guard).
Ref: developer.apple.com/documentation/applicationservices/1458830-uazoomchangefocus
2. NSAccessibility protocol on EmacsView for VoiceOver: 2. NSAccessibility protocol on EmacsView for VoiceOver:
- Bare SelectedTextChanged + SelectedRowsChanged + SelectedColumnsChanged - Visual (screen) line numbering via compute_motion/vmotion from indent.c.
triple notification on cursor movement (matches iTerm2 pattern — Handles line wrapping correctly — VoiceOver reads full visual lines on
no rich userInfo needed for cursor move, VoiceOver queries up/down arrow, including within wrapped logical lines.
accessibilityStringForRange: to get the text at the new position). - Bare SelectedTextChanged + SelectedColumnsChanged on every cursor move;
- Rich ValueChanged with typing echo userInfo for content edits SelectedRowsChanged only when visual line changes (prevents re-reading
(kAXTextEditTypeTyping via semi-private AppKit keys, macOS 10.11+). same line on horizontal movement). Matches iTerm2's notification pattern.
- Bare ValueChanged on every cursor draw so VoiceOver refreshes - Rich ValueChanged with typing echo (kAXTextEditTypeTyping, macOS 10.11+).
its cached text model. Bare ValueChanged on cursor-only moves for text model refresh.
- Buffer-line-based numbering via count_lines/find_newline_no_quit - Buffer-aware data access: BUF_BYTE_ADDRESS (not BYTE_POS_ADDR),
with set_buffer_internal_1 safety wrappers (accessibilityLineForIndex:, set_buffer_internal_1 wrappers around compute_motion/vmotion calls.
accessibilityRangeForLine:, accessibilityInsertionPointLineNumber). - accessibilityLabel returns "Emacs — <buffer-name>" for window switch
- BUF_BYTE_ADDRESS (not BYTE_POS_ADDR) for correct buffer data access announcements.
when current_buffer != queried buffer.
- 10000-char cap on accessibilityValue, multibyte-safe ranges. - 10000-char cap on accessibilityValue, multibyte-safe ranges.
Uses raw string literals for semi-private AX keys to avoid type Uses raw string literals for semi-private AX keys to avoid type
conflicts across SDK versions. conflicts with the macOS 26 SDK.
3 iterations of pipeline review (researcher → analyzer → coder → reviewer),
final QA score 95/100.
--- ---
diff --git a/src/nsterm.h b/src/nsterm.h diff --git a/src/nsterm.h b/src/nsterm.h
index 7c1ee4c..7b79170 100644 index 7c1ee4c..0c95943 100644
--- a/src/nsterm.h --- a/src/nsterm.h
+++ b/src/nsterm.h +++ b/src/nsterm.h
@@ -485,6 +485,12 @@ enum ns_return_frame_mode @@ -485,6 +485,12 @@ enum ns_return_frame_mode
@@ -37,16 +40,24 @@ index 7c1ee4c..7b79170 100644
+ NSRect lastAccessibilityCursorRect; + NSRect lastAccessibilityCursorRect;
+ ptrdiff_t lastAccessibilityModiff; + ptrdiff_t lastAccessibilityModiff;
+ ptrdiff_t lastAccessibilityCursorPos; + ptrdiff_t lastAccessibilityCursorPos;
+ ptrdiff_t lastAccessibilityCursorLine; + NSInteger lastAccessibilityVisualLine;
+#endif +#endif
} }
/* AppKit-side interface. */ /* AppKit-side interface. */
diff --git a/src/nsterm.m b/src/nsterm.m diff --git a/src/nsterm.m b/src/nsterm.m
index 932d209..7811687 100644 index 932d209..40e0546 100644
--- a/src/nsterm.m --- a/src/nsterm.m
+++ b/src/nsterm.m +++ b/src/nsterm.m
@@ -3232,6 +3232,148 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors. @@ -57,6 +57,7 @@ Updated by Christian Limpach (chris@nice.ch)
#include "termchar.h"
#include "menu.h"
#include "window.h"
+#include "indent.h"
#include "keyboard.h"
#include "buffer.h"
#include "font.h"
@@ -3232,6 +3233,167 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
/* Prevent the cursor from being drawn outside the text area. */ /* Prevent the cursor from being drawn outside the text area. */
r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA)); r = NSIntersectionRect (r, ns_row_rect (w, glyph_row, TEXT_AREA));
@@ -87,7 +98,10 @@ index 932d209..7811687 100644
+ struct buffer *curbuf + struct buffer *curbuf
+ = XBUFFER (XWINDOW (f->selected_window)->contents); + = XBUFFER (XWINDOW (f->selected_window)->contents);
+ +
+ if (curbuf && BUF_MODIFF (curbuf) != view->lastAccessibilityModiff) + bool content_changed = (curbuf
+ && BUF_MODIFF (curbuf) != view->lastAccessibilityModiff);
+
+ if (content_changed)
+ { + {
+ /* Buffer content changed since last cursor draw — this is + /* Buffer content changed since last cursor draw — this is
+ an edit (typing, yank, undo, etc.). Post ValueChanged + an edit (typing, yank, undo, etc.). Post ValueChanged
@@ -153,20 +167,36 @@ index 932d209..7811687 100644
+ ptrdiff_t pt = curbuf ? BUF_PT (curbuf) : 0; + ptrdiff_t pt = curbuf ? BUF_PT (curbuf) : 0;
+ ptrdiff_t old_pt = view->lastAccessibilityCursorPos; + ptrdiff_t old_pt = view->lastAccessibilityCursorPos;
+ +
+ /* Always post ValueChanged so VoiceOver re-queries + /* Post bare ValueChanged so VoiceOver re-queries
+ accessibilityValue and keeps its text model current. */ + accessibilityValue and keeps its text model current.
+ Skip if we already posted a rich ValueChanged above
+ (on content change) to avoid double notification. */
+ if (!content_changed)
+ NSAccessibilityPostNotification ( + NSAccessibilityPostNotification (
+ view, NSAccessibilityValueChangedNotification); + view, NSAccessibilityValueChangedNotification);
+ +
+ if (pt != old_pt) + if (pt != old_pt)
+ { + {
+ /* Cursor actually moved — post the selection trio. */ + /* Cursor actually moved — post selection notifications.
+ Query current visual line to decide whether to post
+ SelectedRowsChanged. Suppressing it on same-line
+ movement (e.g. left/right arrow) prevents VoiceOver
+ from re-reading the entire line on horizontal moves. */
+ NSInteger cur_line
+ = [view accessibilityInsertionPointLineNumber];
+ NSInteger old_line = view->lastAccessibilityVisualLine;
+
+ NSAccessibilityPostNotification ( + NSAccessibilityPostNotification (
+ view, NSAccessibilitySelectedTextChangedNotification); + view, NSAccessibilitySelectedTextChangedNotification);
+
+ if (cur_line != old_line)
+ NSAccessibilityPostNotification ( + NSAccessibilityPostNotification (
+ view, NSAccessibilitySelectedRowsChangedNotification); + view, NSAccessibilitySelectedRowsChangedNotification);
+
+ NSAccessibilityPostNotification ( + NSAccessibilityPostNotification (
+ view, NSAccessibilitySelectedColumnsChangedNotification); + view, NSAccessibilitySelectedColumnsChangedNotification);
+
+ view->lastAccessibilityVisualLine = cur_line;
+ } + }
+ +
+ view->lastAccessibilityCursorPos = pt; + view->lastAccessibilityCursorPos = pt;
@@ -195,7 +225,7 @@ index 932d209..7811687 100644
ns_focus (f, NULL, 0); ns_focus (f, NULL, 0);
NSGraphicsContext *ctx = [NSGraphicsContext currentContext]; NSGraphicsContext *ctx = [NSGraphicsContext currentContext];
@@ -8237,6 +8379,14 @@ - (void)windowDidBecomeKey /* for direct calls */ @@ -8237,6 +8399,14 @@ - (void)windowDidBecomeKey /* for direct calls */
XSETFRAME (event.frame_or_window, emacsframe); XSETFRAME (event.frame_or_window, emacsframe);
kbd_buffer_store_event (&event); kbd_buffer_store_event (&event);
ns_send_appdefined (-1); // Kick main loop ns_send_appdefined (-1); // Kick main loop
@@ -210,7 +240,7 @@ index 932d209..7811687 100644
} }
@@ -9474,6 +9624,490 @@ - (int) fullscreenState @@ -9474,6 +9644,538 @@ - (int) fullscreenState
return fs_state; return fs_state;
} }
@@ -364,8 +394,12 @@ index 932d209..7811687 100644
+ +
+- (NSInteger)accessibilityInsertionPointLineNumber +- (NSInteger)accessibilityInsertionPointLineNumber
+{ +{
+ /* Return the buffer line number at point. VoiceOver uses this for + /* Return the VISUAL (screen) line number at point. VoiceOver uses
+ line navigation and to announce "line N" when moving by line. */ + this to detect line changes: if the number differs from last query,
+ it reads the full line; if same, it reads just one character.
+ We must count display lines (respecting wrapping, continuation,
+ window width) — not logical buffer lines delimited by newlines.
+ This matches iTerm2, which returns the screen row coordinate. */
+ if (!emacsframe) + if (!emacsframe)
+ return 0; + return 0;
+ +
@@ -377,12 +411,24 @@ index 932d209..7811687 100644
+ if (!b) + if (!b)
+ return 0; + return 0;
+ +
+ /* count_lines operates on current_buffer, so temporarily switch. */
+ struct buffer *old = current_buffer; + struct buffer *old = current_buffer;
+ set_buffer_internal_1 (b); + set_buffer_internal_1 (b);
+ ptrdiff_t lines = count_lines (BUF_BEGV_BYTE (b), BUF_PT_BYTE (b)); +
+ /* Pass width = -1 so compute_motion determines the effective window
+ width exactly as vmotion does (consistent with accessibilityRangeForLine:
+ which uses vmotion). On GUI frames with fringes, this uses the full
+ window_body_width; on TTY frames, it subtracts 1 for continuation marks. */
+ struct position *pos = compute_motion (
+ BUF_BEGV (b), BUF_BEGV_BYTE (b),
+ 0, 0, 0, /* fromvpos, fromhpos, did_motion */
+ BUF_PT (b), /* to */
+ MOST_POSITIVE_FIXNUM, /* tovpos — large enough to not stop early */
+ 0, /* tohpos */
+ -1, /* width — let compute_motion use window width */
+ w->hscroll, 0, w);
+
+ set_buffer_internal_1 (old); + set_buffer_internal_1 (old);
+ return (NSInteger) lines; + return (NSInteger) pos->vpos;
+} +}
+ +
+- (NSRange)accessibilityVisibleCharacterRange +- (NSRange)accessibilityVisibleCharacterRange
@@ -447,9 +493,9 @@ index 932d209..7811687 100644
+ +
+- (NSInteger)accessibilityLineForIndex:(NSInteger)index +- (NSInteger)accessibilityLineForIndex:(NSInteger)index
+{ +{
+ /* Convert character index to buffer line number. VoiceOver uses + /* Convert character index to VISUAL line number. Must be consistent
+ this for line-by-line reading (VO+Up/Down). Must be consistent + with accessibilityInsertionPointLineNumber (both use visual lines
+ with accessibilityInsertionPointLineNumber. */ + via compute_motion). */
+ if (!emacsframe) + if (!emacsframe)
+ return 0; + return 0;
+ +
@@ -467,21 +513,31 @@ index 932d209..7811687 100644
+ if (charpos < BUF_BEGV (b)) + if (charpos < BUF_BEGV (b))
+ charpos = BUF_BEGV (b); + charpos = BUF_BEGV (b);
+ +
+ ptrdiff_t bytepos = buf_charpos_to_bytepos (b, charpos);
+
+ /* count_lines operates on current_buffer, so temporarily switch. */
+ struct buffer *old = current_buffer; + struct buffer *old = current_buffer;
+ set_buffer_internal_1 (b); + set_buffer_internal_1 (b);
+ ptrdiff_t lines = count_lines (BUF_BEGV_BYTE (b), bytepos); +
+ /* Pass width = -1 for consistency with vmotion (used by
+ accessibilityRangeForLine:). See comment in
+ accessibilityInsertionPointLineNumber. */
+ struct position *pos = compute_motion (
+ BUF_BEGV (b), BUF_BEGV_BYTE (b),
+ 0, 0, 0,
+ charpos,
+ MOST_POSITIVE_FIXNUM, 0,
+ -1, w->hscroll, 0, w);
+
+ set_buffer_internal_1 (old); + set_buffer_internal_1 (old);
+ return (NSInteger) lines; + return (NSInteger) pos->vpos;
+} +}
+ +
+- (NSRange)accessibilityRangeForLine:(NSInteger)line +- (NSRange)accessibilityRangeForLine:(NSInteger)line
+{ +{
+ /* Return the character range for buffer line N. VoiceOver uses + /* Return the character range for VISUAL line N. VoiceOver uses
+ this to read an entire line when navigating by line. Must be + this to read an entire line when navigating by line. Must be
+ consistent with accessibilityLineForIndex: (buffer lines). */ + consistent with accessibilityLineForIndex: (both use visual lines).
+ We use vmotion to find the start of visual line N, then advance
+ one more visual line to find the end. Trailing newlines are
+ excluded from the range. */
+ if (!emacsframe) + if (!emacsframe)
+ return NSMakeRange (0, 0); + return NSMakeRange (0, 0);
+ +
@@ -493,33 +549,37 @@ index 932d209..7811687 100644
+ if (!b) + if (!b)
+ return NSMakeRange (0, 0); + return NSMakeRange (0, 0);
+ +
+ /* find_newline_no_quit operates on current_buffer, so switch. */
+ ptrdiff_t begv = BUF_BEGV (b); + ptrdiff_t begv = BUF_BEGV (b);
+ ptrdiff_t zv = BUF_ZV (b); + ptrdiff_t zv = BUF_ZV (b);
+ ptrdiff_t pos = begv;
+ ptrdiff_t pos_byte = BUF_BEGV_BYTE (b);
+ +
+ struct buffer *old = current_buffer; + struct buffer *old = current_buffer;
+ set_buffer_internal_1 (b); + set_buffer_internal_1 (b);
+ +
+ /* Skip N newlines to reach line N. */ + /* Move N visual lines from BEGV to reach the start of line N. */
+ if (line > 0) + struct position *start_pos = vmotion (begv, BUF_BEGV_BYTE (b),
+ (EMACS_INT) line, w);
+ ptrdiff_t line_start = start_pos->bufpos;
+
+ /* Move 1 more visual line to find where this line ends. */
+ ptrdiff_t start_byte = buf_charpos_to_bytepos (b, line_start);
+ struct position *end_pos = vmotion (line_start, start_byte, 1, w);
+ ptrdiff_t line_end = end_pos->bufpos;
+
+ /* If vmotion didn't move (last line in buffer), extend to ZV. */
+ if (line_end <= line_start)
+ line_end = zv;
+
+ /* Exclude trailing newline — VoiceOver doesn't need it and would
+ announce "new line" at the end of each spoken line. */
+ if (line_end > line_start)
+ { + {
+ pos = find_newline_no_quit (pos, pos_byte, line, &pos_byte); + ptrdiff_t last_byte = buf_charpos_to_bytepos (b, line_end - 1);
+ /* find_newline_no_quit returns position AFTER the newline. */ + if (*BUF_BYTE_ADDRESS (b, last_byte) == '\n')
+ line_end--;
+ } + }
+ +
+ ptrdiff_t line_start = pos;
+
+ /* Find end of this line (next newline or buffer end). */
+ ptrdiff_t end_byte = pos_byte;
+ ptrdiff_t line_end = find_newline_no_quit (pos, pos_byte, 1, &end_byte);
+ if (line_end == 0)
+ line_end = zv; /* No newline found — last line. */
+
+ set_buffer_internal_1 (old); + set_buffer_internal_1 (old);
+ +
+ /* Convert to 0-based accessibility indices. */
+ ptrdiff_t start_idx = line_start - begv; + ptrdiff_t start_idx = line_start - begv;
+ ptrdiff_t end_idx = line_end - begv; + ptrdiff_t end_idx = line_end - begv;
+ if (start_idx < 0) start_idx = 0; + if (start_idx < 0) start_idx = 0;
@@ -584,7 +644,25 @@ index 932d209..7811687 100644
+ +
+- (NSString *)accessibilityLabel +- (NSString *)accessibilityLabel
+{ +{
+ /* Provide an identifying label for VoiceOver. */ + /* Provide an identifying label for VoiceOver. Include the current
+ buffer name so VoiceOver announces it on window/frame switch
+ (triggered by FocusedUIElementChangedNotification). */
+ if (!emacsframe)
+ return @"Emacs";
+
+ struct window *w = XWINDOW (emacsframe->selected_window);
+ if (!w)
+ return @"Emacs";
+
+ struct buffer *b = XBUFFER (w->contents);
+ if (!b)
+ return @"Emacs";
+
+ Lisp_Object name = BVAR (b, name);
+ if (STRINGP (name))
+ return [NSString stringWithFormat:@"Emacs — %@",
+ [NSString stringWithLispString:name]];
+
+ return @"Emacs"; + return @"Emacs";
+} +}
+ +