Skip to content

fix: Base AT-SPI ENABLED/SENSITIVE states on the disabled flag - #787

Closed
dylanpulver wants to merge 1 commit into
AccessKit:mainfrom
dylanpulver:fix-atspi-enabled-sensitive-from-disabled
Closed

fix: Base AT-SPI ENABLED/SENSITIVE states on the disabled flag#787
dylanpulver wants to merge 1 commit into
AccessKit:mainfrom
dylanpulver:fix-atspi-enabled-sensitive-from-disabled

Conversation

@dylanpulver

Copy link
Copy Markdown

NodeWrapper::state computed STATE_ENABLED and STATE_SENSITIVE in the else arm of the read-only branch (adapters/atspi-common/src/node.rs:376-380), so neither ever consulted is_disabled. Measured on clean main by calling state() directly:

role=Button    disabled=true   => ENABLED=true  SENSITIVE=true  READ_ONLY=false
role=Button    disabled=false  => ENABLED=true  SENSITIVE=true  READ_ONLY=false
role=Link      disabled=true   => ENABLED=true  SENSITIVE=true  READ_ONLY=false
role=MenuItem  disabled=true   => ENABLED=true  SENSITIVE=true  READ_ONLY=false
role=TextInput read_only=true  => ENABLED=false SENSITIVE=false READ_ONLY=true

So a disabled node whose role doesn't support aria-readonly (Button, Link, MenuItem, Tab, ...) is indistinguishable from the enabled one, and a read-only-but-enabled control loses both states.

Core-AAM 1.2 §3.5.2.27 maps aria-disabled="true" to ATK/AT-SPI "State: STATE_ENABLED not exposed" and §3.5.2.28 maps aria-disabled="false" to STATE_ENABLED; the aria-readonly="true" row (§3.5.2.77) withdraws only STATE_EDITABLE. The other four adapters already derive the platform enabled state from is_disabledmacos/src/node.rs:660, windows/src/node.rs:535, android/src/node.rs:35, ios/src/node.rs:144 — and #474 was that same change for macOS.

The fix leaves the read-only branch exactly as it was and gates Enabled | Sensitive on !is_disabled(). Behaviour changes in the two cells above and nowhere else.

cargo +1.85 test -p accesskit_atspi_common (the MSRV toolchain CI uses): 7 passed on clean main, 11 passed here, 13 with --all-features. cargo fmt --all -- --check, cargo clippy -p accesskit_atspi_common --all-features --all-targets -- -D warnings and cargo doc with RUSTDOCFLAGS=-D warnings all clean locally.

The new tests discriminate: reverting the source alone fails 2 of the 4; the narrower } else if !state.is_disabled() { still fails the read-only one; dropping the read-only branch instead fails both READ_ONLY assertions. enabled_node_is_enabled_and_sensitive passes in every one of those trees.

Not tested: I have no Linux machine or Orca here, so this is verified at the accesskit_atspi_common translation layer only, not end to end through accesskit_unix against a live AT.

This change was prepared with AI assistance (Claude Opus 5).

STATE_ENABLED and STATE_SENSITIVE were set by the else arm of the
read-only branch, so they never consulted `is_disabled`. For any role
that doesn't support aria-readonly, a disabled node was exposed as
enabled and sensitive; conversely a read-only-but-enabled control lost
both states.

Core-AAM 1.2 maps aria-disabled=true to "STATE_ENABLED not exposed" and
aria-disabled=false to STATE_ENABLED, and its aria-readonly=true row
withdraws only STATE_EDITABLE. The other four adapters already derive
the platform enabled state from `is_disabled`.
@DataTriny

Copy link
Copy Markdown
Member

Closing in favor of #788.

Please @dylanpulver next time consider opening an issue instead of submitting such contribution. Thanks.

@DataTriny DataTriny closed this Sep 1, 2026
@dylanpulver

Copy link
Copy Markdown
Author

Understood, and noted for next time — I'll open an issue first here rather than arriving with a patch.

Glad the fix landed in #788.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants