Skip to content

Core-AAM: Add remaining role tests#60410

Merged
Ms2ger merged 4 commits into
masterfrom
aamtests-role-tests
Jul 3, 2026
Merged

Core-AAM: Add remaining role tests#60410
Ms2ger merged 4 commits into
masterfrom
aamtests-role-tests

Conversation

@spectranaut

Copy link
Copy Markdown
Contributor

No description provided.

@spectranaut

Copy link
Copy Markdown
Contributor Author

@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 :)

@spectranaut

Copy link
Copy Markdown
Contributor Author

@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.

@jcsteh

jcsteh commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

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.

@spectranaut

Copy link
Copy Markdown
Contributor Author

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 cyns left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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?

@cookiecrook cookiecrook left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's clear the commented expectations provide value for later updates. Keep them. Thanks for working on this.

@spectranaut spectranaut force-pushed the aamtests-role-tests branch from e53c8e5 to 3387cf6 Compare June 29, 2026 21:13
@spectranaut spectranaut force-pushed the aamtests-role-tests branch from 3387cf6 to 49a09d0 Compare June 29, 2026 23:11
@spectranaut

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review cyns!! you caught helpful things.

* progressbar.py has aria-valuenow=20. Should the test check the value as well as the role?

We will test the mapping for aria-valuenow in a separate test, so I don't think we need to do it in this one.

* [ ]  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?

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

* [ ]  textbox.py and textbox_multiline.py should there be tests for aria-readoly=true?

Good question and idea. In this case, since aria-readonly changes the mapping, it makes sense to add, I added tests to both files.

@spectranaut

Copy link
Copy Markdown
Contributor Author

@Ms2ger can you merge this PR as well? Same flake failures.

@Ms2ger Ms2ger merged commit aea6b2b into master Jul 3, 2026
26 of 28 checks passed
@Ms2ger Ms2ger deleted the aamtests-role-tests branch July 3, 2026 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants