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

Tweak <aside> conditional mappings to check for more cases #43013

Merged
merged 7 commits into from
Feb 3, 2024
Merged

Tweak <aside> conditional mappings to check for more cases #43013

Changes from 6 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
7adb584
Add contextual role case for <aside> in <main> element
sivakusayan Nov 8, 2023
506ac5a
Add case for section in <main>
sivakusayan Nov 8, 2023
bf4e7b9
Naming changes
sivakusayan Nov 8, 2023
e5e4904
Check aside mapping for all sectioning content
sivakusayan Nov 8, 2023
5bf06f8
Merge branch 'master' into nested-aside-manual-role
sivakusayan Nov 13, 2023
3aacf09
Fix test name from merge
sivakusayan Nov 13, 2023
fb5e299
Only assert conditional aside mappings for named sections
sivakusayan Nov 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 23 additions & 4 deletions html-aam/roles-contextual.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,31 @@
<a data-testname="el-a-no-href" class="ex-generic">x</a>

<!-- el-aside -->
<aside data-testname="el-aside" data-expectedrole="complementary" class="ex">x</aside>
<main>
<aside data-testname="el-aside-in-main" data-expectedrole="complementary" class="ex">x</aside>
<section>
<aside data-testname="el-aside-in-section-in-main" class="ex-generic">x</aside>
Copy link
Contributor

Choose a reason for hiding this comment

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

noting that while this is what the spec currently says to do, because the section is without a name it is treated no differently than a div in the accTree.

but if that section were given a name, then it would be a role=region, and then it would would be more appropriate for this to be generic.

https://github.com/w3c/html-aam/pull/484/files will eventually address this, but mentioning it here if we want to do anything about it now / modify html aam to at least rectify this single case

Copy link
Contributor Author

@sivakusayan sivakusayan Nov 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Would it make sense to name the <sections> in this WPT change and to ignore the un-named <section> case until w3c/html-aam#484 is merged? That way, browsers that are following to the spec word-for-word aren't dinged yet, but browsers that eagerly implement the behavior in w3c/html-aam#484 can still pass the WPT.

Alternatively, we could include the un-named <section> case here and assert that:

<body>
  <section>
      <aside data-expectedrole="complementary">x</aside>
  </section>
</body>

if people are okay with it, but I don't know if people feel weird about WPT tests that aren't formally in the spec yet.

Copy link
Contributor

Choose a reason for hiding this comment

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

If there is general agreement to the spec change and no objection, I think it's fine to merge the test.

ARIA WG Chairs changed the process so it's now the expectation that any spec change PR will be approved, but not merged until there are 1. tests, and 2. multiple passing implementations. So if we didn't merge the test, it would cause a circular dependency with the spec PR. 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if you two have fully resolved this thread, but the rest of the PR looks good to me, so I'm approving.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I noticed that w3c/html-aam#484 still seems to be under discussion, so I ended up doing the half measure of just naming the <section> in this PR, while ignoring the un-named <section> case. We can always refine these tests as needed once people seem to come into an agreement there.

<aside data-testname="el-aside-in-section-in-main-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
</section>
</main>
<article>
<aside data-testname="el-aside-in-article" class="ex-generic">x</aside>
<aside data-testname="el-aside-in-article-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
</article>
<aside>
<aside data-testname="el-aside-in-aside" class="ex-generic">x</aside>
<aside data-testname="el-aside-in-aside-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
</aside>
<nav>
<aside data-testname="el-aside-in-section-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
<aside data-testname="el-aside-in-section-without-name" class="ex-generic">x</aside>
<aside data-testname="el-aside-in-section-with-role" data-expectedrole="complementary" class="ex" role="complementary">x</aside>
<aside data-testname="el-aside-in-nav" class="ex-generic">x</aside>
<aside data-testname="el-aside-in-nav-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
<aside data-testname="el-aside-in-nav-with-role" data-expectedrole="complementary" class="ex" role="complementary">x</aside>
</nav>
<aside data-testname="el-aside-ancestorbodymain" data-expectedrole="complementary" class="ex">x</aside>
<section>
<aside data-testname="el-aside-in-section" class="ex-generic">x</aside>
<aside data-testname="el-aside-in-section-with-name" data-expectedrole="complementary" aria-label="x" class="ex">x</aside>
</section>

<!-- el-footer -->
<nav>
Expand Down