feat(context): allow selecting shadow DOM nodes by WilcoFiers · Pull Request #3798 · dequelabs/axe-core

WilcoFiers · 2022-11-24T19:54:22Z

Add support for shadow DOM selection
Look at all uses of Context to ensure it works with shadow DOM selectors
Normalise fromFrames option when constructing Context
Normalise fromShadowDom option when constructing Context
Update type definition
create doc/context.md file
Update API.md file

Closes #3793, closes #3765, and closes #3799

lib/core/utils/contains.js

WilcoFiers · 2022-11-28T13:02:30Z

lib/core/utils/contains.js

-  if (vNode.shadowId || otherVNode.shadowId) {
-    do {
-      if (vNode.shadowId === otherVNode.shadowId) {
-        return true;


This was wrong. We never caught it because contains is never called with vNode as a node in the shadow DOM tree.

WilcoFiers · 2022-11-28T13:07:27Z

lib/core/public/run-virtual-rule.js

+    include: [vNode],
+    exclude: [],
+    frames: [],
+    page: false,
+    focusable: true,
+    size: {},
+    flatTree: []


Spotted some missing props. Don't know if it can cause a problem, but I figured I'd fix it and add a test.

WilcoFiers · 2022-11-28T13:09:04Z

lib/core/utils/is-node-in-context.js

- * @return {Node}             The deepest node
- */
-function getDeepest(collection) {
-  return collection.sort((a, b) => {


This shouldn't have used sort. We only need the last item in the array.

WilcoFiers · 2022-11-28T14:15:02Z

lib/core/base/context/parse-selector-array.js

  for (let i = 0, l = context[type].length; i < l; i++) {
    const item = context[type][i];
-    // selector
-    if (typeof item === 'string') {


This cannot happen. After normalising, this is always either a node, or an array..

WilcoFiers · 2022-11-28T14:16:50Z

lib/core/base/context/parse-selector-array.js

-    // Handle Iframe selection
-    if (item && item.length) {
+      // Handle Iframe selection
+    } else if (item && item.length) {


This might just as well be "else". If this isn't an array, normalization has a bug. Open to suggestions.

straker

This should include tests for the types in typings/axe-core-tests.ts.

Also, when a shadow DOM selector targets an element that is not a shadow host, axe-core should call axe.log() to log the issue. Right now it's throwing with "No elements found for include in page Context"

lib/core/base/context/normalize-context.js

straker · 2022-11-28T22:36:13Z

lib/core/utils/contains.js

+  do {
+    if (vNode === otherVNode) {
+      return true;
+    } else if (otherVNode.nodeIndex < vNode.nodeIndex) {


Is this just a short circuit so you don't have to navigate up the entire parent tree?

test/core/base/context.js

straker · 2022-11-28T23:54:05Z

test/core/utils/contains.js

+      assert.isFalse(axe.utils.contains(node2, node1));
+    });
+
+    it('should work', () => {


What should work exactly?

straker · 2022-11-28T23:54:35Z

test/core/utils/contains.js

+    assert.isFalse(axe.utils.contains(node2, node1));
+  });
+
+  describe.skip('using fallbacks', () => {


Should this skip be here?

Ahh yes. I got rid of .compareDocumentPosition. Don't care for maintaining that code path, but I can keep the other two tests.

test/core/utils/contains.js

straker · 2022-11-29T00:01:52Z

test/core/utils/contains.js

+        `<section id="target"></section>`
+      );
+      createNestedShadowDom(section, `<img>`);
+      createNestedShadowDom(section, `<input>`);


Did you mean for this to not have any shadow trees? Passing a single html code to createNestedShadowDom just appends the code to innerHTML and never creates a shadow tree, so the end result is just flat nodes:

<div id="fixture"> <section id="target"> <img> <input> </section> </div>

Ooff! Good catch. This test is completely broken. Will fix.

WilcoFiers · 2022-11-29T15:28:12Z

doc/context.md

+
+**Note**: The `fromShadowDom` property cannot be used on the same object as `include` and `exclude`.
+
+### Slotted elements


I need to add tests for this. I think this will work, but I didn't try it.

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

doc/API.md

enlarsen · 2022-11-29T15:58:37Z

doc/context.md

+
+1. [Test specific elements](#test-specific-elements)
+1. [Test DOM nodes](#test-dom-nodes)
+1. [Excludes elements from test](#exclude-elements-from-test)


Suggested change

1. [Excludes elements from test](#exclude-elements-from-test)

1. [Exclude elements from test](#exclude-elements-from-test).

enlarsen · 2022-11-29T16:00:13Z

doc/context.md

+1. [Limit frame testing](#limit-frame-testing)
+1. [Limit shadow DOM testing](#limit-shadow-dom-testing)
+1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context)
+1. [Implicit frame and shadow DOM selection](#implicit-frame-and-shadow-dom-selection)


Suggested change

1. [Implicit frame and shadow DOM selection](#implicit-frame-and-shadow-dom-selection)

1. [Select frame and shadow DOM implicitly](#implicit-frame-and-shadow-dom-selection).

I see where you're coming from, but this isn't a call to action thing. I don't want people to do this. It's more about explaining some background.

Makes sense.

doc/context.md

enlarsen · 2022-11-29T17:25:28Z

doc/context.md

+Axe-core's `context` argument is a powerful tool for controlling precisely which elements are tested and which are ignored. The context lets you do many things, including:
+
+1. [Test specific elements](#test-specific-elements)
+1. [Test DOM nodes](#test-dom-nodes)


Suggested change

1. [Test DOM nodes](#test-dom-nodes)

1. [Test DOM nodes](#test-dom-nodes).

enlarsen · 2022-11-29T17:26:17Z

doc/context.md

+1. [Test specific elements](#test-specific-elements)
+1. [Test DOM nodes](#test-dom-nodes)
+1. [Excludes elements from test](#exclude-elements-from-test)
+1. [Select from prior tests](#select-from-prior-tests)


Suggested change

1. [Select from prior tests](#select-from-prior-tests)

1. [Select from prior tests](#select-from-prior-tests).

enlarsen · 2022-11-29T17:27:18Z

doc/context.md

+1. [Test DOM nodes](#test-dom-nodes)
+1. [Excludes elements from test](#exclude-elements-from-test)
+1. [Select from prior tests](#select-from-prior-tests)
+1. [Limit frame testing](#limit-frame-testing)


Suggested change

1. [Limit frame testing](#limit-frame-testing)

1. [Limit frame testing](#limit-frame-testing).

enlarsen · 2022-11-29T17:27:41Z

doc/context.md

+1. [Excludes elements from test](#exclude-elements-from-test)
+1. [Select from prior tests](#select-from-prior-tests)
+1. [Limit frame testing](#limit-frame-testing)
+1. [Limit shadow DOM testing](#limit-shadow-dom-testing)


Suggested change

1. [Limit shadow DOM testing](#limit-shadow-dom-testing)

1. [Limit shadow DOM testing](#limit-shadow-dom-testing).

enlarsen · 2022-11-29T17:28:08Z

doc/context.md

+1. [Select from prior tests](#select-from-prior-tests)
+1. [Limit frame testing](#limit-frame-testing)
+1. [Limit shadow DOM testing](#limit-shadow-dom-testing)
+1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context)


Suggested change

1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context)

1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context).

straker · 2022-11-29T22:11:38Z

doc/context.md

+await axe.run([navbar, cookiePopup]);
+```
+
+### Component Frameworks


If we're listing component frameworks, could we also list a small section for testing frameworks like Jest? Then we could add a quick example showing that you need to mount instead of shallowMount a component for testing

Good point, but I'm going for a note instead.

doc/context.md

Co-authored-by: Erik Larsen <enlarsen@users.noreply.github.com> Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

…hadow-context

Sorted

straker · 2022-11-30T15:25:30Z

typings/axe-core/axe-core-tests.ts

+  await axe.run(document.querySelectorAll('main'));
+  // axe.run with frameContext context
+  await axe.run({ fromShadowDom: ['#app', '#main', '#inner'] });
+  // @ts-expect-error // Must be two long:


I did not know about @ts-expect-error. That's really cool.

straker · 2022-11-30T15:27:10Z

typings/axe-core/axe-core-tests.ts

+  await axe.run({ include: ['main', document.head] });
+  await axe.run(document.querySelector('main'));
+  await axe.run(document.querySelectorAll('main'));
+  // axe.run with frameContext context


Suggested change

// axe.run with frameContext context

// axe.run with shadow DOM context

straker · 2022-11-30T15:28:10Z

typings/axe-core/axe-core-tests.ts

  }
 );
+export async function runAsync() {
+  await axe.run('main'); // Single selector


We'll also want to add tests for unlabeled frame and shadow DOM selectors.

Sorted

feat(context): allow scoping shadow DOM nodes

be9554b

WilcoFiers changed the title ~~feat(context): allow scoping shadow DOM nodes~~ feat(context): allow selecting shadow DOM nodes Nov 24, 2022

WilcoFiers commented Nov 25, 2022

View reviewed changes

lib/core/utils/contains.js Show resolved Hide resolved

WilcoFiers added 2 commits November 25, 2022 15:55

Refactor Context code

56b91bc

Code complete

80c0ffd

WilcoFiers commented Nov 28, 2022

View reviewed changes

WilcoFiers added 2 commits November 28, 2022 14:14

cleanup

ecbc232

Fix tests

8cd13d0

WilcoFiers commented Nov 28, 2022

View reviewed changes

Update type definition

b58beb1

WilcoFiers marked this pull request as ready for review November 28, 2022 14:42

WilcoFiers requested a review from a team as a code owner November 28, 2022 14:42

straker previously requested changes Nov 29, 2022

View reviewed changes

Write docs

5c99df3

WilcoFiers commented Nov 29, 2022

View reviewed changes

Apply suggestions from code review

0a51dca

Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

enlarsen reviewed Nov 29, 2022

View reviewed changes

Resolve comments

9856f70

straker reviewed Nov 29, 2022

View reviewed changes

doc/context.md Outdated Show resolved Hide resolved

WilcoFiers and others added 4 commits November 30, 2022 14:11

Add type definition tests

de63ccf

Apply suggestions from code review

b8a6fd3

Co-authored-by: Erik Larsen <enlarsen@users.noreply.github.com> Co-authored-by: Steven Lambert <2433219+straker@users.noreply.github.com>

Merge branch 'shadow-context' of github.com:dequelabs/axe-core into s…

1c395c5

…hadow-context

Update context.md

0e58343

WilcoFiers requested review from enlarsen and straker November 30, 2022 13:58

WilcoFiers mentioned this pull request Nov 30, 2022

Make it easier to test only test shadow DOM trees #3805

Open

fix test

09a859a

straker previously requested changes Nov 30, 2022

View reviewed changes

Update type test

4f7aad3

WilcoFiers requested a review from straker November 30, 2022 17:16

straker approved these changes Nov 30, 2022

View reviewed changes

michael-siek mentioned this pull request Nov 30, 2022

Support axe core shadow dom selector dequelabs/axe-core-npm#615

Closed

WilcoFiers merged commit 9e1e31b into develop Dec 1, 2022

WilcoFiers deleted the shadow-context branch December 1, 2022 10:09

michael-siek mentioned this pull request Dec 1, 2022

Support axe core shadow dom selector dequelabs/axe-core-maven-html#280

Closed

WilcoFiers mentioned this pull request Dec 2, 2022

chore: improve type definitions #3808

Merged

This was referenced Dec 5, 2022

TESTING chore(release): v4.6.0 #3814

Closed

chore(release): v4.6.0 #3823

Closed

attest-team-ci mentioned this pull request Dec 12, 2022

chore(release): v4.6.0 #3824

Merged

Shubhabrata08 mentioned this pull request Jan 13, 2024

Investigate the accessibility rules configured in the jest-axe library. microsoft/fluentui-charting-contrib#28

Closed


		Note: The `fromShadowDom` property cannot be used on the same object as `include` and `exclude`.

		### Slotted elements

	1. [Excludes elements from test](#exclude-elements-from-test)
	1. [Exclude elements from test](#exclude-elements-from-test).

	1. [Implicit frame and shadow DOM selection](#implicit-frame-and-shadow-dom-selection)
	1. [Select frame and shadow DOM implicitly](#implicit-frame-and-shadow-dom-selection).

	1. [Test DOM nodes](#test-dom-nodes)
	1. [Test DOM nodes](#test-dom-nodes).

	1. [Select from prior tests](#select-from-prior-tests)
	1. [Select from prior tests](#select-from-prior-tests).

	1. [Limit frame testing](#limit-frame-testing)
	1. [Limit frame testing](#limit-frame-testing).

	1. [Limit shadow DOM testing](#limit-shadow-dom-testing)
	1. [Limit shadow DOM testing](#limit-shadow-dom-testing).

	1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context)
	1. [Combine shadow DOM and frame context](#combine-shadow-dom-and-frame-context).

	// axe.run with frameContext context
	// axe.run with shadow DOM context

Conversation

WilcoFiers commented Nov 24, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

straker left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

WilcoFiers commented Nov 24, 2022 •

edited

Loading