refactor(a11y-macos): move the name/description precedence into mapping - #498
Merged
Conversation
…ng (fixes #262) The macOS reader decided a node's `name` and `description` inside `walk`, which is `cfg(target_os = "macos")` and takes a live `AXUIElement`, so none of it could be tested off macOS. The gate at the heart of it is behaviourally invisible: hoist the `AXDescription` read above the titled/untitled decision and every output stays byte-identical while every titled node with help text pays an extra cross-process round-trip. Only a read count can catch that edit. `mapping::labels` takes the title as a value and the description and help as `FnOnce` readers, so the extraction preserves the reads exactly — same attributes, same order, same counts — and a test can assert which attributes went unread. It delegates its name slot to `mapping::node_name`, which holds the `AXTitle`-else-`AXDescription` precedence that `walk`, `set_value` and `invoke` each spelled out inline; the reader reaches it through a new `read_name` helper. The precedence now lives in one place, so the walk and the two fingerprint sites cannot rank the two attributes differently and reject an element that never moved. Which attributes each site feeds it is still spelled at that site. Whitespace-only labels are pinned as they were rather than trimmed: `read_label` folds `""` but not `" "`, matching the Windows and Linux readers' `nonempty`. The Android reader counts a blank label as absent, so this is a live divergence between backends rather than a rule this crate settles; the test says so. No behaviour change. Verified by ablation: hoisting the description read fails only `a_titled_node_with_help_never_reads_its_description`; retrying the help fallback on a label that normalizes away fails only the two no-retry tests; an inline `normalize_description` lookalike that tests blankness without trimming fails only the combination table; and un-delegating `labels` while trimming its own title arm fails the agreement test. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This was referenced Aug 21, 2026
xxx
added a commit
that referenced
this pull request
Aug 21, 2026
… did not carry (#502) #498's commit message and PR body describe this content; the branch was pushed after an amend that rewrote the message without staging the working tree, so the panel's fixes never left the machine. Behaviour is unchanged either way, and #498 as merged is correct — it is the un-reviewed version of the same refactor. This lands the review. `labels` delegates its name slot to `node_name` through a one-shot `Option<F>` rather than restating the `AXTitle`-else-`AXDescription` precedence. The rule now lives in one place structurally, not by a test that has to enumerate inputs, and the doc no longer claims an `FnOnce` cannot serve both slots — it can, taken from an `Option`. Read counts and read order are unchanged: title, then help, then description on a titled node; title, description, help on an untitled one. `name` is renamed `node_name`, which `labels` shadowed with its own local binding. Tests: `an_untitled_node_reads_its_description_exactly_once` is deleted — the `FnOnce` bound makes a second call a compile error, so the count could not fail and the behaviour is already row 6 of the combination table. The two dropped-secondary-label tests now pass a *present* description, so they pin that the fallback does not retry a help label that normalizes away rather than an absent-value coincidence. The table gains a padded-help row pinning `normalize_description`'s trim at this seam, and the agreement loops gain blank labels, since a trim added to one side and not the other is the realistic way the two functions drift. Docs: `click_element` corrected to `invoke` (the seam method this crate implements); the extra-read narrative now names the one edit its read count catches, instead of welding that edit's premise to a different edit's consequence; the unmeasured "most of a macOS tree" is dropped; and the whitespace rule is described as a live divergence from the Android reader, which trims deliberately, rather than as a settled cross-backend convention. Verified by ablation: hoisting the description read fails only `a_titled_node_with_help_never_reads_its_description`; retrying the help fallback fails only the two no-retry tests; a blankness-only `normalize_description` lookalike fails only the combination table; un-delegating `labels` while trimming its own title arm fails the agreement test. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #262.
The macOS reader decided a node's
nameanddescriptioninsidewalk—cfg(target_os = "macos"), over a liveAXUIElementwith no constructible stand-in — so none of the rule could be tested off macOS. The issue's point was that the gate at its heart is behaviourally invisible: hoist theAXDescriptionread above the titled/untitled decision and every output stays byte-identical while every titled node with help text pays an extra cross-process round-trip. Only a read count catches that.What moved
mapping::labels(title, read_description, read_help) -> (name, description)— the issue's form 2. The title is a value; the other two areFnOncereaders, so the extraction preserves the reads exactly (same attributes, same order, same counts) and a test can assert which attributes went unread.mapping::node_name(title, read_description)holds theAXTitle-else-AXDescriptionprecedence thatwalk,set_valueandinvokeeach spelled out inline.labelsdelegates its name slot to it rather than restating the rule, so the walk and the two fingerprint sites cannot rank the two attributes differently and reject an element that never moved. The reader is one-shot: whichever slot spends theAXDescriptionread, the other finds it gone.normalize_descriptionmoves with the decision, which brings the "description repeats the name" drop under test off macOS for the first time.Whitespace
Pinned as it was, not trimmed.
read_labelfolds""but not" ", matchingnonemptyin the Windows and Linux readers. The Android reader deliberately counts a blank label as absent — so this is a live divergence between backends, not a rule this crate settles, and the test says that rather than claiming a convention.Verification
Ablations, each run and restored:
AXDescriptionread above the decisiona_titled_node_with_help_never_reads_its_descriptiononly..._without_retrying_the_descriptiontests onlynormalize_descriptionlookalike: blankness, no trimlabelsand trim its own title armthe_fingerprint_name_and_the_walked_name_agree_on_every_combinationAlso:
cargo test --workspacegreen;cargo clippy --workspace --all-targets --locked -- -D warningsclean;cargo clippy --target x86_64-apple-darwin -p glass-a11y-macos --all-targets --locked -- -D warningsclean, which type-checks thecfg(macos)reader from Linux;cargo fmt --all --checkclean.Not run here:
crates/glass-macos/tests/a11y.rs, the on-box output check — it needs a TCC-granted Mac. Unchanged by this branch.No CHANGELOG entry: internal refactor, which the file's own maintenance note excludes.
Known limits
labels' two reader parameters are the same type, so transposing them at the call site compiles. One call site, and each closure names itsattr::constant on its own line, so this was judged not worth a newtype.AxTargetfingerprint" is imprecise — name is one of three facts the gate checks — but the phrasing is inherited fromglass-core/src/accessibility.rsandreader.rs, so it is left consistent rather than fixed in one place.Follow-ups found while reviewing, not fixed here
read_label's""fold (reader.rs:370) andread_subrole's gate (reader.rs:411) are the same "delete it and only a read count changes" invariant this issue was filed about, and are stillcfg(macos)-only.name:selector reaches it, andoutline::is_scaffoldingstops eliding the node.walkreadsattr::VALUEas a string (reader.rs:477) andgather_statesreads the same attribute as ani64(reader.rs:621), so every checkbox/radio/switch pays two round-trips on one attribute.