Tweak <aside> conditional mappings to check for more cases #43013
Conversation
|
There are no reviewers for this pull request. Please reach out on the chat room to get help with this. Thank you! |
sivakusayan
changed the title
|
There may be two similar PRs trampling each other ATM... #43028 |
I approved that one since it's a downstream export. I like test names to be even more specific so that we never need to rename and breaking the pass/fail history, so if you don't mind, I'll my editorial test name feedback for that one on this PR. |
Yeah, that should be fine. I'll rebase and apply the test name changes + other feedback later after work (or sometime during the weekend if I don't get to it today). |
html-aam/roles-contextual.html
Outdated
| <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> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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. 😉
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
html-aam/roles-contextual.html
Outdated
| <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> |
There was a problem hiding this comment.
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.
|
@sivakusayan @scottaohara is this ready to be merged? |
|
I think it's ready to merge if @scottaohara does. |
Follow-up from this comment.