-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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.
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
base: master
Are you sure you want to change the base?
Conversation
//https://github.com/web-platform-tests/interop-accessibility/issues/161 starts | ||
const innerButton = document.createElement("button"); | ||
innerButton.textContent = " (inner button)"; | ||
innerButton.id = "inner-button"; | ||
document.getElementById("inner-button-container").appendChild(innerButton); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Co-authored-by: James Craig <[email protected]>
Co-authored-by: James Craig <[email protected]>
innerButton.classList.add("ex"); | ||
innerButton.setAttribute('data-expectedrole','button'); | ||
innerButton.setAttribute('data-testname','JS injected HTML button within another HTML button'); | ||
document.getElementById("inner-button-container").appendChild(innerButton); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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".
There was a problem hiding this comment.
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!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approved pending one more change.
innerButton.classList.add("ex"); | ||
innerButton.setAttribute('data-expectedrole','button'); | ||
innerButton.setAttribute('data-testname','JS injected HTML button within another HTML button'); | ||
document.getElementById("inner-button-container").appendChild(innerButton); |
There was a problem hiding this comment.
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!
Closes: web-platform-tests/interop-accessibility#161
Relates: w3c/aria#1174
Description:
This WPT is designed to evaluate how browsers expose ARIA (non-focusable) vs HTML (focusable) buttons and assess their consistency.
Note:
I’ve set the expectation for the ARIA button nesting a non-focusable ARIA button to "generic" because, as of now (pending resolution of w3c/aria#1174), the ARIA specification states that button children are presentational. This means it does not trigger Presentational Roles Conflict Resolution. However, WebKit, Chromium, and Gecko currently ignore this specification.