Core-AAM: Add remaining role tests#60410
Conversation
|
@jcsteh -- I'm not sure if you actually want to review this, there are a lot more failures this time, for both chrome and firefox: https://wpt.fyi/results/core-aam/aamtests/role?sha=c3ccf0f11f&label=pr_head I plan on opening issues after the PR lands, like I did for the previous set of tests. So you can wait until those issues come in if you prefer :) |
|
@cyns cynthia I wonder if you can review these for me? I've generated these tests (from a script, not AI, fwiw) from Joanie's manual tests and the core-aam spec. |
|
Food for thought: Should these be marked as tentative for now? On one hand, they comply with the spec as currently written. On the other hand, if we have failures across multiple browsers, that suggests a broader issue than just one browser failing to keep up. |
Hmm interesting. I'm inclined to not mark the tests marked tentative -- for whatever reason, at whatever point, there was consensus on the current mappings, represented in Core-AAM. We also can't mark individual subtests as tentative for this test type. I mean, we could, but we'd have to implement the feature. For now you can only mark a whole file as tentative, and every file will eventually have a test for every platform API. Your main point is a good one though, for every role that fails the same way on Firefox and Chrome, I'll open a core-aam issue for it. There are only 3 in this pull request btw! Most of the other failures are just on one browser or the other. 16 out of 39 of the mappings in this PR fail on either Firefox or (exclusive-or) Chrome. |
cyns
left a comment
There was a problem hiding this comment.
Overall, looks good!
A couple of comments/questions
- progressbar.py has aria-valuenow=20. Should the test check the value as well as the role?
- tab.py the comments say state_selected is only exposed if focus is in the tab panel. It looks like you're always testing that it's exposed, and I don't see any code that puts the focus there. Will the focus be in the right place?
- textbox.py and textbox_multiline.py should there be tests for aria-readoly=true?
e53c8e5 to
3387cf6
Compare
3387cf6 to
49a09d0
Compare
|
Thanks for the thorough review cyns!! you caught helpful things.
We will test the mapping for
Ahh good catch! I should have realized this. Both firefox and chrome failed this test anyway, when searching for SELECTED in the states. What we should do is set focus and then check for the state. Since I'm just doing the static mapping for these elements, I just changed it to a TODO and opened: w3c/core-aam#264
Good question and idea. In this case, since aria-readonly changes the mapping, it makes sense to add, I added tests to both files. |
|
@Ms2ger can you merge this PR as well? Same flake failures. |
No description provided.