diff --git a/crates/glass-a11y-macos/src/mapping.rs b/crates/glass-a11y-macos/src/mapping.rs index c8f92427..3e72c862 100644 --- a/crates/glass-a11y-macos/src/mapping.rs +++ b/crates/glass-a11y-macos/src/mapping.rs @@ -5,7 +5,7 @@ //! `"AXButton"`/`"AXTextField"`/... constants; the reader passes the string so this //! module needs no macOS-only dependency. -use glass_core::{AxRole, AxStates}; +use glass_core::{AxRole, AxStates, normalize_description}; /// Every AX role string glass maps. AX role strings are canonical constants, so lookup is /// case-sensitive. @@ -162,8 +162,53 @@ pub fn checkable_checked(role: AxRole, ax_value: Option) -> (bool, bool) { } } +/// A node's `name`: its `AXTitle`, else its `AXDescription` (`setAccessibilityLabel` surfaces as +/// `AXDescription`). Never `AXValue` — volatile content in a field that is half the `AxTarget` +/// fingerprint `set_value`/`click_element` re-walk against. +/// +/// The description arrives as a reader because each label costs an AX round-trip and a titled node +/// never needs it. +/// +/// The one place the rule lives: a second copy could read a name differently and reject an element +/// that never moved. [`labels`] repeats it only because an `FnOnce` cannot fill both the name slot +/// and the description fallback, and a test quantifies their agreement over every input. +/// +/// Both labels arrive already empty-filtered by the reader; a whitespace-only one is a name, +/// matching the Windows and Linux readers' `nonempty`. +pub fn name( + title: Option, + read_description: impl FnOnce() -> Option, +) -> Option { + title.or_else(read_description) +} + +/// A node's `(name, description)`, decided together because which attribute is left to describe a +/// node depends on which one named it. +/// +/// `AXDescription` costs at most one read per node either way: as the name when there is no title, +/// as the description only when there IS a title and no `AXHelp`. Reading it unconditionally +/// re-reads the string that is already the name on the untitled nodes that are most of a macOS +/// tree, and [`glass_core::normalize_description`] drops the copy — so only a read count catches +/// that edit. +/// +/// The returned description is normalized: blank, or a repeat of `name`, becomes `None`. +pub fn labels( + title: Option, + read_description: impl FnOnce() -> Option, + read_help: impl FnOnce() -> Option, +) -> (Option, Option) { + let (name, secondary) = match title { + Some(title) => (Some(title), read_help().or_else(read_description)), + None => (read_description(), read_help()), + }; + let description = secondary.and_then(|raw| normalize_description(&raw, name.as_deref())); + (name, description) +} + #[cfg(test)] mod tests { + use std::cell::Cell; + use super::*; use glass_core::AxRole; @@ -449,4 +494,164 @@ mod tests { assert_eq!(checkable_checked(Button, Some(1)), (false, false)); assert_eq!(checkable_checked(Slider, Some(1)), (false, false)); } + + /// A label reader that counts its calls. Which attributes `labels` reads is invisible in its + /// return value, so a test that only inspects the tuple cannot tell the gated reader from one + /// that reads everything. + fn counted<'a>( + text: Option<&'a str>, + calls: &'a Cell, + ) -> impl FnOnce() -> Option + 'a { + move || { + calls.set(calls.get() + 1); + text.map(str::to_string) + } + } + + fn reader(text: Option<&str>) -> impl FnOnce() -> Option + '_ { + move || text.map(str::to_string) + } + + #[test] + fn a_titled_node_is_named_by_its_title_and_described_by_its_help() { + let (name, description) = labels( + Some("Save".to_string()), + reader(Some("a description")), + reader(Some("Saves the document")), + ); + assert_eq!(name.as_deref(), Some("Save")); + assert_eq!(description.as_deref(), Some("Saves the document")); + } + + #[test] + fn a_titled_node_without_help_is_described_by_its_description() { + let (name, description) = labels( + Some("Save".to_string()), + reader(Some("Saves the document")), + reader(None), + ); + assert_eq!(name.as_deref(), Some("Save")); + assert_eq!(description.as_deref(), Some("Saves the document")); + } + + #[test] + fn an_untitled_node_is_named_by_its_description() { + let (name, description) = labels(None, reader(Some("Close")), reader(Some("Closes it"))); + assert_eq!(name.as_deref(), Some("Close")); + assert_eq!(description.as_deref(), Some("Closes it")); + } + + #[test] + fn a_titled_node_with_help_never_reads_its_description() { + // The gate this extraction exists to keep verified: deleting the titled/untitled split + // leaves every output byte-identical, so this count is the only assertion that can fail. + let reads = Cell::new(0); + let (name, description) = labels( + Some("Save".to_string()), + counted(Some("a description"), &reads), + reader(Some("Saves the document")), + ); + assert_eq!(reads.get(), 0, "AXDescription must not be read at all here"); + assert_eq!(name.as_deref(), Some("Save")); + assert_eq!(description.as_deref(), Some("Saves the document")); + } + + #[test] + fn an_untitled_node_reads_its_description_exactly_once() { + // The other half of the gate: the description already named the node, so consulting it + // again buys a second cross-process read of a string `normalize_description` drops. + let reads = Cell::new(0); + let (name, description) = labels(None, counted(Some("Close"), &reads), reader(None)); + assert_eq!( + reads.get(), + 1, + "AXDescription must be read once, as the name" + ); + assert_eq!(name.as_deref(), Some("Close")); + assert_eq!(description, None); + } + + #[test] + fn a_secondary_label_that_repeats_the_name_is_dropped() { + let (name, description) = + labels(Some("Save".to_string()), reader(None), reader(Some("Save"))); + assert_eq!(name.as_deref(), Some("Save")); + assert_eq!(description, None); + } + + #[test] + fn a_whitespace_only_secondary_label_is_dropped() { + let (name, description) = + labels(Some("Save".to_string()), reader(None), reader(Some(" "))); + assert_eq!(name.as_deref(), Some("Save")); + assert_eq!(description, None); + } + + #[test] + fn a_whitespace_only_title_still_names_the_node() { + // `read_label` folds "" but not " ", matching the Windows and Linux readers' `nonempty`. + // Pinned rather than trimmed: which strings may occupy the name slot is a cross-backend + // convention, not this function's call. + let (name, description) = labels( + Some(" ".to_string()), + reader(Some("a description")), + reader(Some("Closes it")), + ); + assert_eq!(name.as_deref(), Some(" ")); + assert_eq!(description.as_deref(), Some("Closes it")); + } + + #[test] + fn every_title_description_help_combination_lands_where_the_reader_put_it() { + // Exhaustive over the eight inputs: a hand-picked sample cannot show that an arm was + // dropped rather than merely unexercised. + let cases = [ + (Some("t"), Some("d"), Some("h"), Some("t"), Some("h")), + (Some("t"), Some("d"), None, Some("t"), Some("d")), + (Some("t"), None, Some("h"), Some("t"), Some("h")), + (Some("t"), None, None, Some("t"), None), + (None, Some("d"), Some("h"), Some("d"), Some("h")), + (None, Some("d"), None, Some("d"), None), + (None, None, Some("h"), None, Some("h")), + (None, None, None, None, None), + ]; + for (title, description, help, want_name, want_description) in cases { + let got = labels(title.map(str::to_string), reader(description), reader(help)); + assert_eq!( + (got.0.as_deref(), got.1.as_deref()), + (want_name, want_description), + "title={title:?} description={description:?} help={help:?}" + ); + } + } + + #[test] + fn the_fingerprint_name_and_the_walked_name_agree_on_every_combination() { + // `set_value`/`invoke` re-derive a name through `name` to fingerprint the element `walk` + // recorded through `labels`; a divergence would reject an element that never moved. + for title in [Some("t"), None] { + for description in [Some("d"), None] { + for help in [Some("h"), None] { + let walked = + labels(title.map(str::to_string), reader(description), reader(help)).0; + let fingerprint = name(title.map(str::to_string), reader(description)); + assert_eq!( + walked, fingerprint, + "title={title:?} description={description:?} help={help:?}" + ); + } + } + } + } + + #[test] + fn a_titled_node_needs_no_description_read_to_be_named() { + // `set_value` and `invoke` pay these reads on the element they are about to act on. + let reads = Cell::new(0); + assert_eq!( + name(Some("Save".to_string()), counted(Some("d"), &reads)).as_deref(), + Some("Save") + ); + assert_eq!(reads.get(), 0); + } } diff --git a/crates/glass-a11y-macos/src/reader.rs b/crates/glass-a11y-macos/src/reader.rs index 9dfd36af..ca50f28c 100644 --- a/crates/glass-a11y-macos/src/reader.rs +++ b/crates/glass-a11y-macos/src/reader.rs @@ -20,7 +20,7 @@ use glass_core::coords::pixel_geometry_from_content_rect; use glass_core::platform::WindowGeometry; use glass_core::{ Accessibility, AxContext, AxNode, AxNodeId, AxRect, AxRole, AxTarget, AxTree, GlassError, - Result, WalkBudget, normalize_description, read_back_confirms, write_took_no_effect, + Result, WalkBudget, read_back_confirms, write_took_no_effect, }; use objc2_application_services::AXUIElement; use objc2_core_foundation::CFRetained; @@ -113,10 +113,9 @@ impl Accessibility for MacosA11y { // and it is rejected here rather than silently overwritten. let ax_role = ffi::attribute_string(&el, attr::ROLE).unwrap_or_default(); let role = mapping::map_role(&ax_role, read_subrole(&el, &ax_role).as_deref()); - // Same two reads, in the same order, that `walk` derived this element's `name` from — - // through the same helper, so a fingerprint can never be computed from a differently-read - // name and reject an element that never moved. - let name = read_label(&el, attr::TITLE).or_else(|| read_label(&el, attr::DESCRIPTION)); + // Same rule `walk` derived this element's `name` from, so a fingerprint can never be + // computed from a differently-read name and reject an element that never moved. + let name = read_name(&el); let bounds = window_relative_rect(&el, scale, &ctx.window); if !target.matches(role, name.as_deref()) || !target.bounds_consistent(bounds, SET_VALUE_BOUNDS_TOL) @@ -188,7 +187,7 @@ impl Accessibility for MacosA11y { let ax_role = ffi::attribute_string(&el, attr::ROLE).unwrap_or_default(); let role = mapping::map_role(&ax_role, read_subrole(&el, &ax_role).as_deref()); // `name` derived exactly as in `walk` and `set_value` — see there. - let name = read_label(&el, attr::TITLE).or_else(|| read_label(&el, attr::DESCRIPTION)); + let name = read_name(&el); let bounds = window_relative_rect(&el, scale, &ctx.window); if !target.matches(role, name.as_deref()) || !target.bounds_consistent(bounds, SET_VALUE_BOUNDS_TOL) @@ -379,6 +378,14 @@ fn read_label(el: &AXUIElement, attr_name: &str) -> Option { } } +/// `el`'s `name` as [`walk`] records it: the fingerprint `set_value`/`invoke` re-walk against has +/// to come from the same reads, in the same order, that produced the name in the snapshot. +fn read_name(el: &AXUIElement) -> Option { + mapping::name(read_label(el, attr::TITLE), || { + read_label(el, attr::DESCRIPTION) + }) +} + /// `el`'s `AXSubrole`, but only for the base roles whose subrole actually changes the mapped /// role ([`mapping::subrole_matters`]) — every other node skips the AX IPC round-trip and gets /// `None`. @@ -459,25 +466,14 @@ fn walk( _ => ax_role, } }; - // Name = title, else description — both stable labels (e.g. `setAccessibilityLabel` - // surfaces as `AXDescription`). Never fold in `AXValue`: it's volatile content, and a - // node's name must stay stable for the `AxTarget` fingerprint `set_value` relies on. - // - // Which attribute is left to describe the node depends on which one named it, so both - // labels are decided in one place. `AXDescription` costs at most one read per node either - // way: as the name when there is no title, as the description only when there IS a title - // and no `AXHelp`. - let (name, secondary) = match read_label(el, attr::TITLE) { - Some(title) => ( - Some(title), - read_label(el, attr::HELP).or_else(|| read_label(el, attr::DESCRIPTION)), - ), - None => ( - read_label(el, attr::DESCRIPTION), - read_label(el, attr::HELP), - ), - }; - let description = secondary.and_then(|raw| normalize_description(&raw, name.as_deref())); + // Which attribute names the node and which is left to describe it is decided in + // `mapping::labels`, where the rule — and which reads it declines to make — is unit-tested on + // any host. + let (name, description) = mapping::labels( + read_label(el, attr::TITLE), + || read_label(el, attr::DESCRIPTION), + || read_label(el, attr::HELP), + ); let value = ffi::attribute_string(el, attr::VALUE); let bounds = window_relative_rect(el, scale, win); let states = mapping::map_states(&gather_states(el, role));