Skip to content

Remove hidden nodes from the acccessibility tree - #823

Open
hovinen wants to merge 9 commits into
DioxusLabs:mainfrom
hovinen:support-setting-hidden-flag
Open

Remove hidden nodes from the acccessibility tree#823
hovinen wants to merge 9 commits into
DioxusLabs:mainfrom
hovinen:support-setting-hidden-flag

Conversation

@hovinen

@hovinen hovinen commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

This is as per https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion. It also facilitates testing: in cases where there is only one non-hidden element of a particular role, one can just query for that role without having to specify the query more narrowly.

This includes unit tests of the functionality. To facilitate the assertions it adds a dependency on the crate Test That!.

WPT results

No changes in test results compared to main.

Generated by the WPT workflow.

@nicoburns nicoburns left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Core change looks good, but some notes.

Comment thread packages/blitz-dom/src/accessibility.rs Outdated
Comment thread packages/blitz-dom/src/accessibility.rs Outdated
Comment thread packages/blitz-dom/src/accessibility.rs Outdated
Comment on lines +272 to +280
verify_that!(
tree_update.nodes,
contains((
anything(),
matches_pattern!(AccessKitNode {
role(): eq(Role::GenericContainer)
})
))
)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the use of the library here. It introduces an extra DSL that needs to be learnt, and it doesn't seem like it'd be much shorter than a pure-rust assertion...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Of course it's your call. If you want, I will replace it with a built-in assertion.

I found it easier to write the assertions this way. I can write the precise assertion I want in the test without over- or underspecifying, and I get a context-rich failure message for free.

There's an example showing why I care about that in another test which I had to investigate and fix as part of this change: a_semantic_page_has_no_unknown_elements. The assertion looks overspecified compared to the name of the test -- if I take the test name literally, it should be asserting that the elements with unknown Role is a subset of the given set, not a sequence equal to it. But it's not clear whether this is intentional -- is it important that there be a head element with unknown Role in the accessibility tree? Did I break something? (For now, I've assumed that the test name is correct, since that seems to make sense, but I'm really not sure.)

@hovinen
hovinen force-pushed the support-setting-hidden-flag branch from 0db19fc to 2d0d644 Compare September 4, 2026 08:20
@hovinen
hovinen requested a review from nicoburns September 4, 2026 11:39
@hovinen
hovinen force-pushed the support-setting-hidden-flag branch from 8c24fee to 8a9fe28 Compare September 5, 2026 11:36
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