Remove hidden nodes from the acccessibility tree - #823
Conversation
nicoburns
left a comment
There was a problem hiding this comment.
Core change looks good, but some notes.
| verify_that!( | ||
| tree_update.nodes, | ||
| contains(( | ||
| anything(), | ||
| matches_pattern!(AccessKitNode { | ||
| role(): eq(Role::GenericContainer) | ||
| }) | ||
| )) | ||
| ) |
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.)
0db19fc to
2d0d644
Compare
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.
…s, as per the spec
8c24fee to
8a9fe28
Compare
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.