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

[Accessibility] Nested ARIA and HTML buttons - Presentational Roles Conflict Resolution #50045

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

[Accessibility] Nested ARIA and HTML buttons - Presentational Roles Conflict Resolution #50045

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
e2f7125
Nested ARIA and HTML buttons - Presentational Roles Conflict Resolution
giacomo-petri Jan 13, 2025
23c4853
Update wai-aria/role/role_none_conflict_resolution.tentative.html
giacomo-petri Jan 15, 2025
5d2a636
Update wai-aria/role/role_none_conflict_resolution.tentative.html
giacomo-petri Jan 15, 2025
99d9117
Update role_none_conflict_resolution.tentative.html
giacomo-petri Jan 15, 2025
db7d403
Update role_none_conflict_resolution.tentative.html
giacomo-petri Jan 15, 2025
767d7d1
Apply suggestions from code review
giacomo-petri Jan 17, 2025
ddab884
Update role_none_conflict_resolution.tentative.html
giacomo-petri Jan 17, 2025
cbd4f58
Update role_none_conflict_resolution.tentative.html
giacomo-petri Jan 17, 2025
d33999c
Update role_none_conflict_resolution.tentative.html
giacomo-petri Jan 21, 2025
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
48 changes: 48 additions & 0 deletions wai-aria/role/role_none_conflict_resolution.tentative.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<!doctype html>
<html>
<head>
<title>Role None Conflict Resolution Verification Tests</title>
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
<script src="/resources/testdriver.js"></script>
<script src="/resources/testdriver-vendor.js"></script>
<script src="/resources/testdriver-actions.js"></script>
<script src="/wai-aria/scripts/aria-utils.js"></script>
</head>
<body>

giacomo-petri marked this conversation as resolved.
Show resolved Hide resolved
cookiecrook marked this conversation as resolved.
Show resolved Hide resolved
<!-- no conflict, since not focusable; inner button should be exposed as generic - children presentational - //https://github.com/web-platform-tests/interop-accessibility/issues/161 -->
<h2>ARIA role="button" nesting ARIA role="button" (no presentational roles conflict resolution, since not focusable)</h2>
<div role="button">
giacomo-petri marked this conversation as resolved.
Show resolved Hide resolved
<span>outer button</span>
<span role="button" class="ex-generic" data-testname="non-focusable ARIA[role button] nesting non-focusable ARIA[role button]">Inner button</span>
giacomo-petri marked this conversation as resolved.
Show resolved Hide resolved
</div>

<!-- conflict arise, since inner button is focusable - //https://github.com/web-platform-tests/interop-accessibility/issues/161-->
<h2>HTML button nesting HTML button</h2>
<div id="button-container">
<button id="outer-button">
<span>outer button</span>
<span id="inner-button-container"></span>
</button>
</div>

<script>
AriaUtils.verifyRolesBySelector(".ex");
AriaUtils.verifyGenericRolesBySelector(".ex-generic");

//https://github.com/web-platform-tests/interop-accessibility/issues/161 starts
const innerButton = document.createElement("button");
innerButton.textContent = " (inner button)";
innerButton.id = "inner-button";
giacomo-petri marked this conversation as resolved.
Show resolved Hide resolved
document.getElementById("inner-button-container").appendChild(innerButton);
Copy link
Contributor

@cookiecrook cookiecrook Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Do you need to change [an attr] a change of the same element, or would testing the before/after states of separate elements be sufficient?

Copy link
Contributor Author

@giacomo-petri giacomo-petri Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm not entirely sure I understand the question. However, if I'm interpreting it correctly, the issue arises because browsers automatically adjust nested buttons by moving the inner button outside the parent button.

To address this, I'm using JavaScript to dynamically inject a button inside another button.
Therefore, the final state of a button nested into another button will suffice.

Copy link
Contributor

@cookiecrook cookiecrook Jan 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

IIRC, you've moved the ARIAUtils.verify…() calls after this DOM modification, which is another way to address the concern I alluded to but insufficiently explained... No further modification is necessary. Thanks. Okay to mark this thread as resolved.

Copy link
Contributor

@cookiecrook cookiecrook Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Unfortunately, this test may not return anything useful, since button > span > button is invalid (I think?) and not guaranteed to render anything as it's own inner button. If it doesn't get a RenderObject, there won't be a reliable way to check the role.

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 was curious to see the result because button > span > button isn’t allowed in HTML; in fact, as expected, browsers automatically move the nested button outside the parent button.
However, if you wrap the button using JavaScript, the nested buttons remain and "work as intended".

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay then... Since this is undefined behavior, the only remaining change needed is to remove the expectation that this should be a button... After further WG discussion, we may decide that's right, or it may end up being something else, so I don't want a tentative test (with a potentially invalid assumption) to be the cause of an rendering engine change that needs to be rolled back later.

innerButton.setAttribute('data-expectedrole','SPEC_AMBIGUOUS_LOG_VALUE');

I'm marking the PR as approved, pending this last change. Thanks for your diligence and patience!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


promise_test(async t => {
const role = await test_driver.get_computed_role(document.getElementById('inner-button'));
assert_equals(role, "button");
}, "test nested HTML button within HTML button");
//https://github.com/web-platform-tests/interop-accessibility/issues/161 ends
giacomo-petri marked this conversation as resolved.
Show resolved Hide resolved
</script>

</body>
</html>