Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tentative: match new PR about 2E "unless the element is marked as presentational" #50002

Open
wants to merge 16 commits into
base: master
Choose a base branch
from
Open

Tentative: match new PR about 2E "unless the element is marked as presentational" #50002

wants to merge 16 commits into from

Conversation

giacomo-petri
Copy link
Contributor

@giacomo-petri giacomo-petri commented Jan 9, 2025 •
edited
Loading

Closes: web-platform-tests/interop-accessibility#167

Relates and match: w3c/aria#2405

Ambiguity: https://www.w3.org/TR/accname-1.2/#comp_host_language_label

This PR adds tests for:

  • elements receiving the accessible name from host language label elements (both explicit and implicit) with role="presentation";
  • elements with role=presentation that do not receive the accessible name from host language label elements (both explicit and implicit)
  • elements with role=presentation that receives the accessible name from host language label elements (both explicit and implicit) due to presentational roles conflict resolution

Note: since I created tests for acc name related to presentational roles conflict resolution, I have also included tests to verify that the computed role aligns with the expected result.

@giacomo-petri giacomo-petri changed the title input elements receiving the accessible name from label elements (both explicit and implicit) with role="presentation" Tentative: match new PR about 2E "unless the element is marked as presentational" Jan 11, 2025
@lolaodelola
Copy link

Hi @giacomo-petri, I'm coming from the review of PR 2405. This looks good to me for the most part, Scott's addressed some of the initial confusion. I noted some semantic confusion in the naming of the test sections vs what is being tested. While role="presentation" and role="none" do the same thing, it's confusing to have the test refer to testing role="none" on an input but the actual test setup has role="presentation" on the input. In most cases where you mention testing for none, you're testing for presentation, I didn't want to litter your inbox with every instance of this but I've noted two above.

Unless there's something I'm missing, the heading and test setup should match to avoid confusion for folks when reading.

Thank you for this work!

@giacomo-petri
Copy link
Contributor Author

Done

@lolaodelola
Copy link

Thanks @giacomo-petri! You have some lint errors, once you've addressed those I'll be happy to approve this.

@giacomo-petri
Copy link
Contributor Author

Thanks all:

  • replaced .ex with .ex-label (and added where missing)
  • replaced all role="presentation" with role="none"

cookiecrook
cookiecrook requested changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@cookiecrook cookiecrook left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Almost ready!

accname/name/comp_host_language_label.tentative.html Outdated Show resolved Hide resolved
accname/name/comp_host_language_label.tentative.html Outdated Show resolved Hide resolved
accname/name/comp_host_language_label.tentative.html Outdated Show resolved Hide resolved
accname/name/comp_host_language_label.tentative.html Outdated Show resolved Hide resolved
accname/name/comp_host_language_label.tentative.html Outdated
<fieldset data-expectedlabel="legend" data-testname="html: fieldset wrapping legend[role none]" class="ex-label"><legend role="none">legend</legend></fieldset>

<h2>HTML fieldset with role="none" wrapping legend</h2>
<fieldset role="none" data-expectedlabel="" data-testname="html: fieldset[role none] wrapping legend" class="ex-label"><legend>legend</legend></fieldset>
Copy link
Contributor

Choose a reason for hiding this comment

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

This might not be a valid test assumption, as the WebDriver methods don't guarantee a a return value for an element that is ignored by accessibility. That said, maybe it will but we won't know until all the CI bots run. This type of scenario may be more reliable to test once the new WebDriver accessibility extensions land. Ditto for the next one.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, this could be the cause of the Gecko CI crash, though I haven't looked into it yet.

accname/name/comp_host_language_label.tentative.html Outdated Show resolved Hide resolved
accname/name/comp_host_language_label.tentative.html Outdated
<table role="none" data-expectedlabel="" data-testname="html: table[role none] wrapping caption" class="ex-label"><caption>caption</caption></table>

<h2>HTML table role="none" wrapping caption with role="none"</h2>
<table role="none" data-expectedlabel="" data-testname="html: table[role none] wrapping caption[role none]" class="ex-label"><caption role="none">caption</caption></table>
Copy link
Contributor

Choose a reason for hiding this comment

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

Like the fieldset[role=none] test above, this may not be a valid test assumption due to an artifact of the WebDriver methods not defining the expected return type when a DOM element no longer has a backing AX element due to the none role. Wait until all the CI bots complete, since there are implementation detail differences this will likely illustrate.

wai-aria/role/role_none_conflict_resolution.tentative.html Outdated Show resolved Hide resolved
wai-aria/role/role_none_conflict_resolution.tentative.html Outdated Show resolved Hide resolved
wai-aria/role/role_none_conflict_resolution.tentative.html Show resolved Hide resolved
giacomo-petri and others added 7 commits January 17, 2025 08:56
@giacomo-petri @cookiecrook
Apply suggestions from code review
ff3b093 
Co-authored-by: James Craig <[email protected]>
@giacomo-petri
Update role_none_conflict_resolution.tentative.html
a2cb698 
- added readonly testcases with and w/o disabled
- replaced [role none] witg [role=none] in data-testname
@giacomo-petri
Update comp_host_language_label.tentative.html
46f594d 
replaced role none with role=none
@giacomo-petri
Update comp_host_language_label.tentative.html
ba37e50 
added cross reference comment
@giacomo-petri
Update role_none_conflict_resolution.tentative.html
3fa7dda 
add cross reference comment
@giacomo-petri
Update comp_host_language_label.tentative.html
c03918e 
expectations moved from svg to circle
@giacomo-petri
Update comp_host_language_label.tentative.html
87d1daf 
table fix (added THs and TDs)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lolaodelola lolaodelola lolaodelola left review comments

@cookiecrook cookiecrook cookiecrook requested changes

@jnurthen jnurthen Awaiting requested review from jnurthen

@MelSumner MelSumner Awaiting requested review from MelSumner

Requested changes must be addressed to merge this pull request.

Assignees

@cookiecrook cookiecrook

Labels
accname wai-aria wg-wai_pf
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Label element with role presentation and accname Host Language Label
4 participants
@giacomo-petri @lolaodelola @cookiecrook @wpt-pr-bot