From 148a842f187a97882bb3de07812b4515376f436c Mon Sep 17 00:00:00 2001 From: William Selna Date: Mon, 31 Aug 2026 10:21:40 +0200 Subject: [PATCH 1/2] feat(macos): emit geometry notifications so VoiceOver tracks moved content When content moves under a stationary VoiceOver cursor -- the canonical case being a scroll while the cursor sits on an element inside the viewport -- the macOS adapter gave VoiceOver no signal that geometry had changed, so the cursor highlight stayed drawn at the element's old frame until some unrelated trigger made VoiceOver re-read it. The element's accessibilityFrame was correct the whole time; nothing prompted a re-read. Two complementary additions fix that: * The event generator compares each updated node's bounding box and queues a single NSAccessibilityLayoutChangedNotification per tree update, posted on the focused element (root fallback) with no user info. Both details matter: NSAccessibilityUIElementsKey marks elements as added or changed, which makes VoiceOver move focus and re-announce, and posting one notification per moved node fires at scroll rate with the same effect. * Adapter::scroll_settled_event, surfaced on SubclassingAdapter, re-posts the focus notification on the still-focused node. That is the documented trigger that makes VoiceOver re-read an element's frame and re-anchor its cursor. Toolkits call it once per scroll gesture, after the scroll offset settles, never once per frame. This brings macOS closer to the Unix adapter, which already emits ObjectEvent::BoundsChanged on a bounds and transform delta. EventGenerator no longer holds the Context. It only used it to build the QueuedEvents at the end, so the Adapter now does that instead, which lets the event generator be unit tested without AppKit and therefore without pinning the tests to the main thread. --- adapters/macos/src/adapter.rs | 72 ++++++++-- adapters/macos/src/event.rs | 78 +++++++++-- adapters/macos/src/lib.rs | 3 + adapters/macos/src/subclass.rs | 11 ++ adapters/macos/src/tests.rs | 238 +++++++++++++++++++++++++++++++++ 5 files changed, 382 insertions(+), 20 deletions(-) create mode 100644 adapters/macos/src/tests.rs diff --git a/adapters/macos/src/adapter.rs b/adapters/macos/src/adapter.rs index 86b4b0073..c71fbbfbe 100644 --- a/adapters/macos/src/adapter.rs +++ b/adapters/macos/src/adapter.rs @@ -14,7 +14,7 @@ use accesskit::{ ActionHandler, ActionRequest, ActivationHandler, Node, NodeId, Role, TreeId, TreeInfo, TreeUpdate, }; -use accesskit_consumer::{FilterResult, Tree}; +use accesskit_consumer::{FilterResult, FullNodeId, Tree, TreeState}; use objc2::rc::{Id, WeakId}; use objc2_app_kit::NSView; use objc2_foundation::{MainThreadMarker, NSArray, NSObject, NSPoint}; @@ -23,6 +23,18 @@ use std::{ffi::c_void, ptr::null_mut, rc::Rc}; const PLACEHOLDER_ROOT_ID: NodeId = NodeId(0); +/// The node that a scroll-settle focus notification is posted on: the focused +/// node, but only if it is one the platform can actually focus. +/// +/// Re-posting a focus notification on the window itself would be meaningless, +/// so an unfocusable focus yields no event at all. +pub(crate) fn settle_focus_target(state: &TreeState) -> Option { + state + .focus() + .filter(|node| can_be_focused(node)) + .map(|node| node.id()) +} + enum State { Inactive { view: WeakId, @@ -136,14 +148,48 @@ impl Adapter { result } State::Active(context) => { - let mut event_generator = EventGenerator::new(context.clone()); - let mut tree = context.tree.borrow_mut(); - tree.update_and_process_changes(update_factory(), &mut event_generator); - Some(event_generator.into_result()) + let mut event_generator = EventGenerator::new(); + { + let mut tree = context.tree.borrow_mut(); + tree.update_and_process_changes(update_factory(), &mut event_generator); + } + Some(QueuedEvents::new( + Rc::clone(context), + event_generator.into_events(), + )) } } } + /// If the tree is active and a node that the platform can focus currently + /// has focus, return events that re-post + /// `NSAccessibilityFocusedUIElementChangedNotification` on that node. + /// + /// Posting the focus notification again, without the focus having moved, + /// is the documented way to make VoiceOver re-read the focused element, + /// including its frame. A toolkit should call this once per scroll + /// gesture, after the scroll offset has settled, so that the VoiceOver + /// cursor re-anchors to the content that moved under it. It must not be + /// called once per frame: posting a focus notification at frame rate makes + /// VoiceOver announce the element repeatedly. + /// + /// Only the adapter can implement this, because the platform node objects + /// live in its private registry and the focused node comes from its + /// consumer tree state. + /// + /// If a [`QueuedEvents`] instance is returned, the caller must call + /// [`QueuedEvents::raise`] on it. + pub fn scroll_settled_event(&self) -> Option { + let State::Active(context) = &self.state else { + return None; + }; + let target = { + let tree = context.tree.borrow(); + settle_focus_target(tree.state()) + }; + target.map(|id| QueuedEvents::new(Rc::clone(context), vec![focus_event(id)])) + } + /// Update the tree state based on whether the window is focused. /// /// If a [`QueuedEvents`] instance is returned, the caller must call @@ -163,10 +209,18 @@ impl Adapter { None } State::Active(context) => { - let mut event_generator = EventGenerator::new(context.clone()); - let mut tree = context.tree.borrow_mut(); - tree.update_host_focus_state_and_process_changes(is_focused, &mut event_generator); - Some(event_generator.into_result()) + let mut event_generator = EventGenerator::new(); + { + let mut tree = context.tree.borrow_mut(); + tree.update_host_focus_state_and_process_changes( + is_focused, + &mut event_generator, + ); + } + Some(QueuedEvents::new( + Rc::clone(context), + event_generator.into_events(), + )) } } } diff --git a/adapters/macos/src/event.rs b/adapters/macos/src/event.rs index 445419cb3..04695ba62 100644 --- a/adapters/macos/src/event.rs +++ b/adapters/macos/src/event.rs @@ -4,7 +4,7 @@ // the LICENSE-MIT file), at your option. use accesskit::{Live, Role}; -use accesskit_consumer::{FilterResult, FullNodeId, NodeRef, TreeChangeHandler}; +use accesskit_consumer::{FilterResult, FullNodeId, NodeRef, TreeChangeHandler, TreeState}; use hashbrown::HashSet; use objc2::runtime::{AnyObject, ProtocolObject}; use objc2_app_kit::*; @@ -25,6 +25,11 @@ pub(crate) enum QueuedEvent { notification: &'static NSAccessibilityNotificationName, }, NodeDestroyed(FullNodeId), + /// A single, batched notification that at least one node changed + /// geometry during one tree update. The target element is resolved when + /// the event is raised, not when it is queued, because the tree is still + /// mutably borrowed while the event generator runs. + LayoutChanged, Announcement { text: String, priority: NSAccessibilityPriorityLevel, @@ -62,6 +67,29 @@ impl QueuedEvent { }; } } + Self::LayoutChanged => { + // Post on an element rather than on the view, and without + // user info. `NSAccessibilityUIElementsKey` marks elements + // that the assistive technology should treat as added or + // changed, which makes VoiceOver move its cursor and + // re-announce -- the opposite of the silent "this element's + // frame is stale" hint we want here. + // + // The tree borrow is released before posting, because posting + // can synchronously re-enter the platform node, which borrows + // the same tree. + let target = { + let tree = context.tree.borrow(); + layout_changed_target(tree.state()) + }; + let platform_node = context.get_or_create_platform_node(target); + unsafe { + NSAccessibilityPostNotification( + &platform_node, + NSAccessibilityLayoutChangedNotification, + ) + }; + } Self::Announcement { text, priority } => { let view = match context.view.load() { Some(view) => view, @@ -130,6 +158,19 @@ impl QueuedEvents { } } +/// The element that a batched layout-changed notification is posted on: the +/// focused node, or the root if the host window isn't focused. +/// +/// A layout-changed notification has no way to name the nodes that moved, so +/// the target only needs to be a stable element in the affected tree. The +/// focused node is preferred because it is the one whose frame the assistive +/// technology is most likely to be relying on. +pub(crate) fn layout_changed_target(state: &TreeState) -> FullNodeId { + state + .focus() + .map_or_else(|| state.root().id(), |node| node.id()) +} + pub(crate) fn focus_event(node_id: FullNodeId) -> QueuedEvent { QueuedEvent::Generic { node_id, @@ -137,25 +178,32 @@ pub(crate) fn focus_event(node_id: FullNodeId) -> QueuedEvent { } } +#[derive(Default)] pub(crate) struct EventGenerator { - context: Rc, events: Vec, text_changed: HashSet, selected_rows_changed: HashSet, + /// How many platform-exposed nodes changed their bounding box during this + /// update. Only the zero/non-zero distinction matters; see + /// [`EventGenerator::into_events`]. + moved: usize, } impl EventGenerator { - pub(crate) fn new(context: Rc) -> Self { - Self { - context, - events: Vec::new(), - text_changed: HashSet::new(), - selected_rows_changed: HashSet::new(), - } + pub(crate) fn new() -> Self { + Self::default() } - pub(crate) fn into_result(self) -> QueuedEvents { - QueuedEvents::new(self.context, self.events) + pub(crate) fn into_events(mut self) -> Vec { + // Every geometry change in one update is collapsed into one + // notification. Posting one per moved node would fire at scroll rate, + // and VoiceOver responds to a flood of layout-changed notifications by + // re-announcing repeatedly, which is worse for the user than the stale + // frame this is meant to fix. + if self.moved > 0 { + self.events.push(QueuedEvent::LayoutChanged); + } + self.events } fn remove_subtree(&mut self, node: &NodeRef) { @@ -259,6 +307,14 @@ impl TreeChangeHandler for EventGenerator { return; } let node_id = new_node.id(); + // A node that moved or was resized -- by scrolling, a window reflow or + // an animation -- reports a new frame, but assistive technologies do + // not poll geometry. Count it so that `into_events` emits one batched + // layout-changed notification for this update. Only included nodes get + // here; the early return above has already handled the rest. + if old_node.bounding_box() != new_node.bounding_box() { + self.moved += 1; + } let old_wrapper = NodeWrapper(old_node); let new_wrapper = NodeWrapper(new_node); if old_wrapper.title() != new_wrapper.title() { diff --git a/adapters/macos/src/lib.rs b/adapters/macos/src/lib.rs index 5b795424e..924231749 100644 --- a/adapters/macos/src/lib.rs +++ b/adapters/macos/src/lib.rs @@ -22,4 +22,7 @@ pub use patch::add_focus_forwarder_to_window_class; mod subclass; pub use subclass::SubclassingAdapter; +#[cfg(test)] +mod tests; + pub use objc2_foundation::{NSArray, NSObject, NSPoint}; diff --git a/adapters/macos/src/subclass.rs b/adapters/macos/src/subclass.rs index a873f7c85..bfaf9aee6 100644 --- a/adapters/macos/src/subclass.rs +++ b/adapters/macos/src/subclass.rs @@ -254,6 +254,17 @@ impl SubclassingAdapter { let mut state = self.associated.ivars().state.borrow_mut(); state.adapter.update_view_focus_state(is_focused) } + + /// Re-post a focus notification so that VoiceOver re-reads the focused + /// element's frame; see [`Adapter::scroll_settled_event`]. + /// + /// If a [`QueuedEvents`] instance is returned, the caller must call + /// [`QueuedEvents::raise`] on it. The internal borrow taken here is + /// released before this method returns. + pub fn scroll_settled_event(&self) -> Option { + let state = self.associated.ivars().state.borrow(); + state.adapter.scroll_settled_event() + } } impl Drop for SubclassingAdapter { diff --git a/adapters/macos/src/tests.rs b/adapters/macos/src/tests.rs new file mode 100644 index 000000000..4f4a6c7f3 --- /dev/null +++ b/adapters/macos/src/tests.rs @@ -0,0 +1,238 @@ +// Copyright 2026 The AccessKit Authors. All rights reserved. +// Licensed under the Apache License, Version 2.0 (found in +// the LICENSE-APACHE file) or the MIT license (found in +// the LICENSE-MIT file), at your option. + +//! Tests for the parts of the adapter that decide *whether* and *where* to +//! post a notification. They deliberately don't touch AppKit: an `NSView` +//! would pin the tests to the main thread, which the test harness doesn't +//! guarantee. Actually posting the notification is one call and is covered by +//! manual VoiceOver testing. + +use accesskit::{Node, NodeId, Rect, Role, TreeId, TreeInfo, TreeUpdate}; +use accesskit_consumer::Tree; + +use crate::{ + adapter::settle_focus_target, + event::{EventGenerator, QueuedEvent, layout_changed_target}, +}; + +const ROOT: NodeId = NodeId(0); +const BUTTON: NodeId = NodeId(1); +const OTHER_BUTTON: NodeId = NodeId(2); + +fn rect(y0: f64, height: f64) -> Rect { + Rect { + x0: 0.0, + y0, + x1: 100.0, + y1: y0 + height, + } +} + +fn root_node() -> Node { + let mut node = Node::new(Role::Window); + node.push_child(BUTTON); + node.push_child(OTHER_BUTTON); + node +} + +fn button(label: &str, bounds: Rect) -> Node { + let mut node = Node::new(Role::Button); + node.set_label(label); + node.set_bounds(bounds); + node +} + +/// A window with two buttons, the first of which has focus. +fn tree(is_host_focused: bool) -> Tree { + Tree::new( + TreeUpdate { + nodes: vec![ + (ROOT, root_node()), + (BUTTON, button("first", rect(0.0, 20.0))), + (OTHER_BUTTON, button("second", rect(40.0, 20.0))), + ], + tree: Some(TreeInfo::new(ROOT)), + tree_id: TreeId::ROOT, + focus: BUTTON, + }, + is_host_focused, + ) +} + +fn update(nodes: Vec<(NodeId, Node)>) -> TreeUpdate { + TreeUpdate { + nodes, + tree: None, + tree_id: TreeId::ROOT, + focus: BUTTON, + } +} + +fn apply(tree: &mut Tree, update: TreeUpdate) -> Vec { + let mut event_generator = EventGenerator::new(); + tree.update_and_process_changes(update, &mut event_generator); + event_generator.into_events() +} + +fn layout_changed_count(events: &[QueuedEvent]) -> usize { + events + .iter() + .filter(|event| matches!(event, QueuedEvent::LayoutChanged)) + .count() +} + +#[test] +fn moving_a_node_queues_a_layout_changed_event() { + let mut tree = tree(true); + let events = apply( + &mut tree, + update(vec![(BUTTON, button("first", rect(-30.0, 20.0)))]), + ); + assert_eq!(layout_changed_count(&events), 1); +} + +#[test] +fn resizing_a_node_queues_a_layout_changed_event() { + // Same origin, different size: comparing origins alone would miss this. + let mut tree = tree(true); + let events = apply( + &mut tree, + update(vec![(BUTTON, button("first", rect(0.0, 50.0)))]), + ); + assert_eq!(layout_changed_count(&events), 1); +} + +#[test] +fn moving_several_nodes_queues_only_one_layout_changed_event() { + // The batching is load-bearing. One notification per moved node would fire + // at scroll rate, and VoiceOver reacts to a flood of layout-changed + // notifications by re-announcing repeatedly. + let mut tree = tree(true); + let events = apply( + &mut tree, + update(vec![ + (BUTTON, button("first", rect(-30.0, 20.0))), + (OTHER_BUTTON, button("second", rect(10.0, 20.0))), + ]), + ); + assert_eq!(layout_changed_count(&events), 1); +} + +#[test] +fn an_update_that_moves_nothing_queues_no_layout_changed_event() { + let mut tree = tree(true); + let events = apply( + &mut tree, + update(vec![(BUTTON, button("renamed", rect(0.0, 20.0)))]), + ); + assert_eq!(layout_changed_count(&events), 0); + // The update did change something, so this isn't vacuously true. + assert!(!events.is_empty()); +} + +#[test] +fn re_submitting_an_unchanged_tree_queues_no_layout_changed_event() { + let mut tree = tree(true); + let events = apply( + &mut tree, + update(vec![ + (BUTTON, button("first", rect(0.0, 20.0))), + (OTHER_BUTTON, button("second", rect(40.0, 20.0))), + ]), + ); + assert_eq!(layout_changed_count(&events), 0); +} + +#[test] +fn moving_a_node_that_is_not_exposed_queues_no_layout_changed_event() { + // A hidden node is excluded by the filter, so its frame is not something + // any assistive technology is tracking. + let mut tree = tree(true); + let hidden = |bounds| { + let mut node = button("second", bounds); + node.set_hidden(); + node + }; + apply( + &mut tree, + update(vec![(OTHER_BUTTON, hidden(rect(40.0, 20.0)))]), + ); + let events = apply( + &mut tree, + update(vec![(OTHER_BUTTON, hidden(rect(70.0, 20.0)))]), + ); + assert_eq!(layout_changed_count(&events), 0); +} + +#[test] +fn adding_and_removing_nodes_queues_no_layout_changed_event() { + // Only a change of geometry on an existing node counts. Additions and + // removals are already reported by their own notifications. + let mut tree = tree(true); + let mut root_with_one_child = Node::new(Role::Window); + root_with_one_child.push_child(BUTTON); + let events = apply(&mut tree, update(vec![(ROOT, root_with_one_child)])); + assert_eq!(layout_changed_count(&events), 0); + assert!( + events + .iter() + .any(|event| matches!(event, QueuedEvent::NodeDestroyed(_))) + ); +} + +#[test] +fn layout_changed_is_targeted_at_the_focused_node() { + let tree = tree(true); + let state = tree.state(); + assert_eq!( + layout_changed_target(state), + state.focus().unwrap().id(), + "expected the focused node" + ); + assert_ne!(layout_changed_target(state), state.root().id()); +} + +#[test] +fn layout_changed_falls_back_to_the_root_when_the_host_is_not_focused() { + let tree = tree(false); + let state = tree.state(); + assert!(state.focus().is_none()); + assert_eq!(layout_changed_target(state), state.root().id()); +} + +#[test] +fn scroll_settle_targets_the_focused_node() { + let tree = tree(true); + let state = tree.state(); + assert_eq!( + settle_focus_target(state), + Some(state.focus().unwrap().id()) + ); +} + +#[test] +fn scroll_settle_has_no_target_when_the_host_is_not_focused() { + let tree = tree(false); + assert_eq!(settle_focus_target(tree.state()), None); +} + +#[test] +fn scroll_settle_has_no_target_when_focus_is_on_the_window_itself() { + // The placeholder tree the adapter builds before the toolkit supplies a + // real one focuses a `Window` node. Re-posting a focus notification on the + // window would tell VoiceOver nothing about an element's frame. + let tree = Tree::new( + TreeUpdate { + nodes: vec![(ROOT, Node::new(Role::Window))], + tree: Some(TreeInfo::new(ROOT)), + tree_id: TreeId::ROOT, + focus: ROOT, + }, + true, + ); + let state = tree.state(); + assert!(state.focus().is_some()); + assert_eq!(settle_focus_target(state), None); +} From 4f8f58ef74e26ed94757ba886ba5345792812ba1 Mon Sep 17 00:00:00 2001 From: William Selna Date: Fri, 16 Oct 2026 00:13:44 +0200 Subject: [PATCH 2/2] chore(macos): Trim comments on the geometry notification code Co-Authored-By: Claude Fable 5.1 --- adapters/macos/src/adapter.rs | 25 ++++--------------- adapters/macos/src/event.rs | 44 ++++++++-------------------------- adapters/macos/src/subclass.rs | 6 ++--- adapters/macos/src/tests.rs | 22 ++++------------- 4 files changed, 22 insertions(+), 75 deletions(-) diff --git a/adapters/macos/src/adapter.rs b/adapters/macos/src/adapter.rs index c71fbbfbe..88eda8ca8 100644 --- a/adapters/macos/src/adapter.rs +++ b/adapters/macos/src/adapter.rs @@ -23,11 +23,7 @@ use std::{ffi::c_void, ptr::null_mut, rc::Rc}; const PLACEHOLDER_ROOT_ID: NodeId = NodeId(0); -/// The node that a scroll-settle focus notification is posted on: the focused -/// node, but only if it is one the platform can actually focus. -/// -/// Re-posting a focus notification on the window itself would be meaningless, -/// so an unfocusable focus yields no event at all. +/// The focused node, if the platform can focus it. pub(crate) fn settle_focus_target(state: &TreeState) -> Option { state .focus() @@ -161,21 +157,10 @@ impl Adapter { } } - /// If the tree is active and a node that the platform can focus currently - /// has focus, return events that re-post - /// `NSAccessibilityFocusedUIElementChangedNotification` on that node. - /// - /// Posting the focus notification again, without the focus having moved, - /// is the documented way to make VoiceOver re-read the focused element, - /// including its frame. A toolkit should call this once per scroll - /// gesture, after the scroll offset has settled, so that the VoiceOver - /// cursor re-anchors to the content that moved under it. It must not be - /// called once per frame: posting a focus notification at frame rate makes - /// VoiceOver announce the element repeatedly. - /// - /// Only the adapter can implement this, because the platform node objects - /// live in its private registry and the focused node comes from its - /// consumer tree state. + /// If a focusable node has focus, return events that re-post + /// `NSAccessibilityFocusedUIElementChangedNotification` on it, which makes + /// VoiceOver re-read the element's frame. Call this once per scroll + /// gesture, after the scroll offset has settled, never once per frame. /// /// If a [`QueuedEvents`] instance is returned, the caller must call /// [`QueuedEvents::raise`] on it. diff --git a/adapters/macos/src/event.rs b/adapters/macos/src/event.rs index 04695ba62..eb32e051d 100644 --- a/adapters/macos/src/event.rs +++ b/adapters/macos/src/event.rs @@ -25,10 +25,9 @@ pub(crate) enum QueuedEvent { notification: &'static NSAccessibilityNotificationName, }, NodeDestroyed(FullNodeId), - /// A single, batched notification that at least one node changed - /// geometry during one tree update. The target element is resolved when - /// the event is raised, not when it is queued, because the tree is still - /// mutably borrowed while the event generator runs. + /// One notification per tree update in which an exposed node changed + /// geometry. The target is resolved when raised, since the tree is still + /// borrowed while events are generated. LayoutChanged, Announcement { text: String, @@ -68,16 +67,9 @@ impl QueuedEvent { } } Self::LayoutChanged => { - // Post on an element rather than on the view, and without - // user info. `NSAccessibilityUIElementsKey` marks elements - // that the assistive technology should treat as added or - // changed, which makes VoiceOver move its cursor and - // re-announce -- the opposite of the silent "this element's - // frame is stale" hint we want here. - // - // The tree borrow is released before posting, because posting - // can synchronously re-enter the platform node, which borrows - // the same tree. + // Posted on an element without user info: `NSAccessibilityUIElementsKey` + // would make VoiceOver move focus and re-announce. The borrow is + // released first because posting can re-enter the platform node. let target = { let tree = context.tree.borrow(); layout_changed_target(tree.state()) @@ -158,13 +150,7 @@ impl QueuedEvents { } } -/// The element that a batched layout-changed notification is posted on: the -/// focused node, or the root if the host window isn't focused. -/// -/// A layout-changed notification has no way to name the nodes that moved, so -/// the target only needs to be a stable element in the affected tree. The -/// focused node is preferred because it is the one whose frame the assistive -/// technology is most likely to be relying on. +/// The focused node, or the root if the host isn't focused. pub(crate) fn layout_changed_target(state: &TreeState) -> FullNodeId { state .focus() @@ -183,9 +169,7 @@ pub(crate) struct EventGenerator { events: Vec, text_changed: HashSet, selected_rows_changed: HashSet, - /// How many platform-exposed nodes changed their bounding box during this - /// update. Only the zero/non-zero distinction matters; see - /// [`EventGenerator::into_events`]. + /// Number of exposed nodes whose bounding box changed in this update. moved: usize, } @@ -195,11 +179,8 @@ impl EventGenerator { } pub(crate) fn into_events(mut self) -> Vec { - // Every geometry change in one update is collapsed into one - // notification. Posting one per moved node would fire at scroll rate, - // and VoiceOver responds to a flood of layout-changed notifications by - // re-announcing repeatedly, which is worse for the user than the stale - // frame this is meant to fix. + // One notification per update; one per moved node would make VoiceOver + // re-announce at scroll rate. if self.moved > 0 { self.events.push(QueuedEvent::LayoutChanged); } @@ -307,11 +288,6 @@ impl TreeChangeHandler for EventGenerator { return; } let node_id = new_node.id(); - // A node that moved or was resized -- by scrolling, a window reflow or - // an animation -- reports a new frame, but assistive technologies do - // not poll geometry. Count it so that `into_events` emits one batched - // layout-changed notification for this update. Only included nodes get - // here; the early return above has already handled the rest. if old_node.bounding_box() != new_node.bounding_box() { self.moved += 1; } diff --git a/adapters/macos/src/subclass.rs b/adapters/macos/src/subclass.rs index bfaf9aee6..0a34bceb9 100644 --- a/adapters/macos/src/subclass.rs +++ b/adapters/macos/src/subclass.rs @@ -255,12 +255,10 @@ impl SubclassingAdapter { state.adapter.update_view_focus_state(is_focused) } - /// Re-post a focus notification so that VoiceOver re-reads the focused - /// element's frame; see [`Adapter::scroll_settled_event`]. + /// See [`Adapter::scroll_settled_event`]. /// /// If a [`QueuedEvents`] instance is returned, the caller must call - /// [`QueuedEvents::raise`] on it. The internal borrow taken here is - /// released before this method returns. + /// [`QueuedEvents::raise`] on it. pub fn scroll_settled_event(&self) -> Option { let state = self.associated.ivars().state.borrow(); state.adapter.scroll_settled_event() diff --git a/adapters/macos/src/tests.rs b/adapters/macos/src/tests.rs index 4f4a6c7f3..ebcac9fb4 100644 --- a/adapters/macos/src/tests.rs +++ b/adapters/macos/src/tests.rs @@ -3,11 +3,7 @@ // the LICENSE-APACHE file) or the MIT license (found in // the LICENSE-MIT file), at your option. -//! Tests for the parts of the adapter that decide *whether* and *where* to -//! post a notification. They deliberately don't touch AppKit: an `NSView` -//! would pin the tests to the main thread, which the test harness doesn't -//! guarantee. Actually posting the notification is one call and is covered by -//! manual VoiceOver testing. +//! These tests don't touch AppKit, so they don't need the main thread. use accesskit::{Node, NodeId, Rect, Role, TreeId, TreeInfo, TreeUpdate}; use accesskit_consumer::Tree; @@ -95,7 +91,7 @@ fn moving_a_node_queues_a_layout_changed_event() { #[test] fn resizing_a_node_queues_a_layout_changed_event() { - // Same origin, different size: comparing origins alone would miss this. + // Same origin, different size. let mut tree = tree(true); let events = apply( &mut tree, @@ -106,9 +102,6 @@ fn resizing_a_node_queues_a_layout_changed_event() { #[test] fn moving_several_nodes_queues_only_one_layout_changed_event() { - // The batching is load-bearing. One notification per moved node would fire - // at scroll rate, and VoiceOver reacts to a flood of layout-changed - // notifications by re-announcing repeatedly. let mut tree = tree(true); let events = apply( &mut tree, @@ -128,7 +121,7 @@ fn an_update_that_moves_nothing_queues_no_layout_changed_event() { update(vec![(BUTTON, button("renamed", rect(0.0, 20.0)))]), ); assert_eq!(layout_changed_count(&events), 0); - // The update did change something, so this isn't vacuously true. + // Not vacuous: the update did change something. assert!(!events.is_empty()); } @@ -147,8 +140,6 @@ fn re_submitting_an_unchanged_tree_queues_no_layout_changed_event() { #[test] fn moving_a_node_that_is_not_exposed_queues_no_layout_changed_event() { - // A hidden node is excluded by the filter, so its frame is not something - // any assistive technology is tracking. let mut tree = tree(true); let hidden = |bounds| { let mut node = button("second", bounds); @@ -168,8 +159,7 @@ fn moving_a_node_that_is_not_exposed_queues_no_layout_changed_event() { #[test] fn adding_and_removing_nodes_queues_no_layout_changed_event() { - // Only a change of geometry on an existing node counts. Additions and - // removals are already reported by their own notifications. + // Additions and removals have their own notifications. let mut tree = tree(true); let mut root_with_one_child = Node::new(Role::Window); root_with_one_child.push_child(BUTTON); @@ -220,9 +210,7 @@ fn scroll_settle_has_no_target_when_the_host_is_not_focused() { #[test] fn scroll_settle_has_no_target_when_focus_is_on_the_window_itself() { - // The placeholder tree the adapter builds before the toolkit supplies a - // real one focuses a `Window` node. Re-posting a focus notification on the - // window would tell VoiceOver nothing about an element's frame. + // The placeholder tree focuses the window node. let tree = Tree::new( TreeUpdate { nodes: vec![(ROOT, Node::new(Role::Window))],