Skip to content

Conversation

@pomerantsev
Copy link
Contributor

Before: if a shadow root (not the shadow host element, but an actual ShadowRoot instance) is passed as part of context, it's silently ignored.

After: the shadow root node is substituted with its host element for the rest of Axe algorithms to work with.

Closes: #4941

@pomerantsev pomerantsev marked this pull request as ready for review December 1, 2025 03:18
@pomerantsev pomerantsev requested a review from a team as a code owner December 1, 2025 03:18
@pomerantsev pomerantsev changed the title [WIP] Add shadow host instead of ignoring the shadow root Add shadow host instead of ignoring the shadow root Dec 1, 2025
@pomerantsev pomerantsev changed the title Add shadow host instead of ignoring the shadow root fix(core): stop ignoring shadow roots in context Dec 1, 2025
Copy link
Contributor

@straker straker 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 pr, it's a great start. For handling shadow roots, instead of adding the host element we should instead add all the children of the shadow root. This way if there are accessibility violations on the host node we do not report them since the shadow root was passed in and not the host node itself.

We'll also want a test added to https://github.com/dequelabs/axe-core/blob/develop/test/core/base/context.js under the describe('include') block that tests that passing a shadow root to the context will add all it's children. You can use createNestedShadowDom (and tests that use it) to make creating the shadow DOM element and children easier.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

it('should not mutate exclude in input', () => {
fixture.innerHTML = '<div id="foo"></div>';
const context = { exclude: [['iframe', '#foo']] };
// eslint-disable-next-line no-new
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed by a pre-commit hook.

@pomerantsev
Copy link
Contributor Author

@straker thanks for the review! I followed your guidance, updated the logic and wrote a test. Could you please take another look?

@pomerantsev pomerantsev requested a review from straker December 3, 2025 02:51
@pomerantsev
Copy link
Contributor Author

CLA assistant check Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.You have signed the CLA already but the status is still pending? Let us recheck it.

By the way, I did previously sign the CLA, not sure why the bot is complaining: #4859

@pomerantsev
Copy link
Contributor Author

Fixed format.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Should it be possible to use a ShadowRoot as context?

3 participants