From 00febf50657c3a2173a340195a8adb24c38c28bf Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Fri, 10 Jan 2020 09:54:51 -0700 Subject: [PATCH 1/8] fix(region): return outermost regionless node instead of html --- lib/checks/navigation/region-after.js | 1 - lib/checks/navigation/region.js | 41 +++- lib/checks/navigation/region.json | 1 - lib/rules/region.json | 3 +- test/checks/navigation/region.js | 199 +++++++++++--------- test/integration/full/region/region-fail.js | 12 +- 6 files changed, 153 insertions(+), 104 deletions(-) delete mode 100644 lib/checks/navigation/region-after.js diff --git a/lib/checks/navigation/region-after.js b/lib/checks/navigation/region-after.js deleted file mode 100644 index c60b163cf3..0000000000 --- a/lib/checks/navigation/region-after.js +++ /dev/null @@ -1 +0,0 @@ -return [results[0]]; diff --git a/lib/checks/navigation/region.js b/lib/checks/navigation/region.js index e890bea5ab..6b3e28e86b 100644 --- a/lib/checks/navigation/region.js +++ b/lib/checks/navigation/region.js @@ -1,6 +1,15 @@ const { dom, aria } = axe.commons; const landmarkRoles = aria.getRolesByType('landmark'); +let regionlessNodes = axe._cache.get('regionlessNodes'); +if (regionlessNodes) { + if (regionlessNodes.includes(node)) { + return false; + } + + return true; +} + // Create a list of nodeNames that have a landmark as an implicit role const implicitLandmarks = landmarkRoles .reduce((arr, role) => arr.concat(aria.implicitNodes(role)), []) @@ -47,6 +56,13 @@ function findRegionlessElms(virtualNode) { dom.getElementByReference(virtualNode.actualNode, 'href')) || !dom.isVisible(node, true) ) { + // Mark each parent node as having region child + let vNode = axe.utils.getNodeFromTree(node); + while (vNode) { + vNode._hasRegionDescendant = true; + vNode = vNode.parent; + } + return []; // Return the node is a content element @@ -62,7 +78,26 @@ function findRegionlessElms(virtualNode) { } } -var regionlessNodes = findRegionlessElms(virtualNode); -this.relatedNodes(regionlessNodes); +regionlessNodes = findRegionlessElms(virtualNode) + // Find first parent marked as having region descendant (or body) and + // return the node right before it as the "outer" element + .map(node => { + let vNode = axe.utils.getNodeFromTree(node); + while ( + vNode.parent && + !vNode.parent._hasRegionDescendant && + vNode.parent.actualNode !== document.body + ) { + vNode = vNode.parent; + } + + return vNode.actualNode; + }) + // Remove duplicate containers + .filter((node, index, array) => { + return array.indexOf(node) === index; + }); +this.data(regionlessNodes); +axe._cache.set('regionlessNodes', regionlessNodes); -return regionlessNodes.length === 0; +return !regionlessNodes.includes(node); diff --git a/lib/checks/navigation/region.json b/lib/checks/navigation/region.json index bd3a69b2cc..98b5612cf8 100644 --- a/lib/checks/navigation/region.json +++ b/lib/checks/navigation/region.json @@ -1,7 +1,6 @@ { "id": "region", "evaluate": "region.js", - "after": "region-after.js", "metadata": { "impact": "moderate", "messages": { diff --git a/lib/rules/region.json b/lib/rules/region.json index e43f17d59f..dea8ed46e5 100644 --- a/lib/rules/region.json +++ b/lib/rules/region.json @@ -1,7 +1,6 @@ { "id": "region", - "selector": "html", - "pageLevel": true, + "selector": "body *", "tags": ["cat.keyboard", "best-practice"], "metadata": { "description": "Ensures all page content is contained by landmarks", diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index 134f4dda6f..51befdafda 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -12,108 +12,107 @@ describe('region', function() { checkContext.reset(); }); - it('should return true when all content is inside the region', function() { + it('should return no regionless nodes when all content is inside the region', function() { var checkArgs = checkSetup( '
Click Here

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return false when img content is outside the region', function() { + it('should return regionless node when img content is outside the region', function() { var checkArgs = checkSetup( - '

Introduction

' + '

Introduction

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 1); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); }); - it('should return true when textless text content is outside the region', function() { + it('should return no regionless nodes when textless text content is outside the region', function() { var checkArgs = checkSetup( '

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return true when wrapper content is outside the region', function() { + it('should return no regionless nodes when wrapper content is outside the region', function() { var checkArgs = checkSetup( '

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return true when invisible content is outside the region', function() { + it('should return no regionless nodes when invisible content is outside the region', function() { var checkArgs = checkSetup( '

Click Here

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return true when there is a skiplink', function() { + it('should return no regionless nodes when there is a skiplink', function() { var checkArgs = checkSetup( '
Click Here

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return true when there is an Angular skiplink', function() { + it('should return no regionless nodes when there is an Angular skiplink', function() { var checkArgs = checkSetup( '
Click Here

Introduction

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should return false when there is a non-region element', function() { + it('should return regionless nodes when there is a non-region element', function() { var checkArgs = checkSetup( - '
This is random content.

Introduction

' + '
This is random content.

Introduction

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 1); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); }); - it('should return the first item when after is called', function() { - assert.equal(checks.region.after([2, 3, 1]), 2); - }); - - it('should return false when there is a non-skiplink', function() { + it('should return regionless nodes when there is a non-skiplink', function() { var checkArgs = checkSetup( - '
Click Here

Introduction

' + '
Click Here

Introduction

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.equal(checkContext._relatedNodes.length, 1); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); }); - it('should return true if the non-region element is a script', function() { + it('should return no regionless nodes if the non-region element is a script', function() { var checkArgs = checkSetup( '
Content
' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); - it('should considered aria labelled elements as content', function() { + it('should consider aria labelled elements as content', function() { var checkArgs = checkSetup( - '
Content
' + '
Content
' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._relatedNodes, [ - fixture.querySelector('div[aria-label]') - ]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); }); it('should allow native landmark elements', function() { @@ -121,30 +120,47 @@ describe('region', function() { '
branding
Content
copyright
' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('ignores native landmark elements with an overwriting role', function() { var checkArgs = checkSetup( - '
Content
' + '
Content
' + ); + + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); + }); + + it('returns regionless nodes for content outside of form tags with accessible names', function() { + var checkArgs = checkSetup( + '

Text

' + ); + + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + }); + + it('returns the outermost regionless element', function() { + var checkArgs = checkSetup( + '

Text

Text

Text

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 1); - assert.deepEqual(checkContext._relatedNodes, [ - fixture.querySelector('main') - ]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); - it('returns false for content outside of form tags with accessible names', function() { + it('returns the outermost element without a region descendant', function() { var checkArgs = checkSetup( - '

Text

' + '

Text

Text

Text

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 1); - assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 3); }); it('ignores unlabeled forms as they are not landmarks', function() { @@ -152,26 +168,25 @@ describe('region', function() { '

Text

foo
' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 2); - assert.deepEqual(checkContext._relatedNodes, [ - fixture.querySelector('p'), - fixture.querySelector('fieldset') - ]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); it('treats with aria label as landmarks', function() { var checkArgs = checkSetup( '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('treats role=forms with aria label as landmarks', function() { var checkArgs = checkSetup( '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('treats forms without aria label as not a landmarks', function() { @@ -179,9 +194,9 @@ describe('region', function() { '

This is random content.

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 1); - assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); it('treats forms with an empty aria label as not a landmarks', function() { @@ -189,9 +204,9 @@ describe('region', function() { '

This is random content.

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 1); - assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); it('treats forms with non empty titles as landmarks', function() { @@ -199,7 +214,8 @@ describe('region', function() { '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('treats forms with empty titles not as landmarks', function() { @@ -207,9 +223,9 @@ describe('region', function() { '

This is random content.

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 1); - assert.deepEqual(checkContext._relatedNodes, [fixture.querySelector('p')]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); it('treats ARIA forms with no label or title as landmarks', function() { @@ -217,28 +233,33 @@ describe('region', function() { '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('allows content in aria-live=assertive', function() { var checkArgs = checkSetup( '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('allows content in aria-live=polite', function() { var checkArgs = checkSetup( '

This is random content.

' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); it('does not allow content in aria-live=off', function() { var checkArgs = checkSetup( '

This is random content.

' ); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); it('treats role=dialog elements as regions', function() { @@ -246,7 +267,8 @@ describe('region', function() { '' ); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); (shadowSupport.v1 ? it : xit)('should test Shadow tree content', function() { @@ -255,8 +277,9 @@ describe('region', function() { shadow.innerHTML = 'Some text'; var checkArgs = checkSetup(div); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._relatedNodes, [div]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture); }); (shadowSupport.v1 ? it : xit)('should test slotted content', function() { @@ -266,22 +289,24 @@ describe('region', function() { shadow.innerHTML = '
'; var checkArgs = checkSetup(div); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); }); (shadowSupport.v1 ? it : xit)( 'should ignore skiplink targets inside shadow trees', function() { var div = document.createElement('div'); - div.innerHTML = 'skiplink
Content
'; + div.innerHTML = + 'skiplink
Content
'; var shadow = div.querySelector('div').attachShadow({ mode: 'open' }); shadow.innerHTML = '
'; var checkArgs = checkSetup(div); - assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.deepEqual(checkContext._relatedNodes, [div.querySelector('a')]); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.lengthOf(checkContext._data, 1); + assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); } ); @@ -295,8 +320,8 @@ describe('region', function() { 'skiplink
'; var checkArgs = checkSetup(div); - assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); - assert.lengthOf(checkContext._relatedNodes, 0); + checks.region.evaluate.apply(checkContext, checkArgs); + assert.isEmpty(checkContext._data); } ); }); diff --git a/test/integration/full/region/region-fail.js b/test/integration/full/region/region-fail.js index 20f2c4723d..abbeebaa1f 100644 --- a/test/integration/full/region/region-fail.js +++ b/test/integration/full/region/region-fail.js @@ -17,16 +17,8 @@ describe('region fail test', function() { assert.lengthOf(results.violations[0].nodes, 1); }); - it('should find html', function() { - assert.deepEqual(results.violations[0].nodes[0].target, ['html']); - }); - - it('should have all text content as related nodes', function() { - var wrapper = document.querySelector('#wrapper'); - assert.equal( - results.violations[0].nodes[0].any[0].relatedNodes.length, - wrapper.querySelectorAll('h1, li, p, a').length - ); + it('should find wrapper', function() { + assert.deepEqual(results.violations[0].nodes[0].target, ['#wrapper']); }); }); From 86bdb96225fab42d608b2e532e87a35fe624db9a Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 13 Jan 2020 10:55:00 -0700 Subject: [PATCH 2/8] fix --- lib/checks/navigation/region.js | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/lib/checks/navigation/region.js b/lib/checks/navigation/region.js index 6b3e28e86b..02a4b902af 100644 --- a/lib/checks/navigation/region.js +++ b/lib/checks/navigation/region.js @@ -3,11 +3,7 @@ const landmarkRoles = aria.getRolesByType('landmark'); let regionlessNodes = axe._cache.get('regionlessNodes'); if (regionlessNodes) { - if (regionlessNodes.includes(node)) { - return false; - } - - return true; + return !regionlessNodes.includes(node); } // Create a list of nodeNames that have a landmark as an implicit role From 971aa1d09e7863c97dd13928f063dd1d50df7cd6 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 13 Jan 2020 10:55:58 -0700 Subject: [PATCH 3/8] typo --- lib/checks/navigation/region.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/checks/navigation/region.js b/lib/checks/navigation/region.js index 02a4b902af..915bc9a3fd 100644 --- a/lib/checks/navigation/region.js +++ b/lib/checks/navigation/region.js @@ -52,7 +52,7 @@ function findRegionlessElms(virtualNode) { dom.getElementByReference(virtualNode.actualNode, 'href')) || !dom.isVisible(node, true) ) { - // Mark each parent node as having region child + // Mark each parent node as having region descendant let vNode = axe.utils.getNodeFromTree(node); while (vNode) { vNode._hasRegionDescendant = true; From 67cb0c4f59d003f8ac10f9f575e13a5d70ee38f2 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 13 Jan 2020 11:05:32 -0700 Subject: [PATCH 4/8] fix tests --- test/integration/full/region/region-fail.js | 6 ------ test/integration/full/region/region-pass.js | 8 ++------ 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/test/integration/full/region/region-fail.js b/test/integration/full/region/region-fail.js index abbeebaa1f..21e6871159 100644 --- a/test/integration/full/region/region-fail.js +++ b/test/integration/full/region/region-fail.js @@ -21,10 +21,4 @@ describe('region fail test', function() { assert.deepEqual(results.violations[0].nodes[0].target, ['#wrapper']); }); }); - - describe('passes', function() { - it('should find none', function() { - assert.lengthOf(results.passes, 0); - }); - }); }); diff --git a/test/integration/full/region/region-pass.js b/test/integration/full/region/region-pass.js index b1a1325fa3..1c57978bd5 100644 --- a/test/integration/full/region/region-pass.js +++ b/test/integration/full/region/region-pass.js @@ -13,12 +13,8 @@ describe('region pass test', function() { }); describe('passes', function() { - it('should find one', function() { - assert.lengthOf(results.passes[0].nodes, 1); - }); - - it('should find html', function() { - assert.deepEqual(results.passes[0].nodes[0].target, ['html']); + it('should pass all nodes', function() { + assert.lengthOf(results.passes[0].nodes, 33); }); }); From 3365c614ea5e55bc5f3ed18bc542285fa91fd11b Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 13 Jan 2020 11:14:45 -0700 Subject: [PATCH 5/8] fix test again --- test/integration/full/region/region-pass.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integration/full/region/region-pass.js b/test/integration/full/region/region-pass.js index 1c57978bd5..a6406bb1f5 100644 --- a/test/integration/full/region/region-pass.js +++ b/test/integration/full/region/region-pass.js @@ -13,8 +13,10 @@ describe('region pass test', function() { }); describe('passes', function() { - it('should pass all nodes', function() { - assert.lengthOf(results.passes[0].nodes, 33); + it('should pass nodes', function() { + // it seems CircleCI and localhost have different number of DOM nodes, + // so as long as everything passes and nothing fails, the rule is working + assert.isTrue(results.passes[0].nodes.length > 0); }); }); From 8656048d77b1d466f5dc3544cd04bd1c7f2416ce Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Fri, 24 Jan 2020 11:47:52 -0700 Subject: [PATCH 6/8] fixes --- lib/checks/navigation/region.js | 20 +- test/checks/navigation/region.js | 231 ++++++++++---------- test/integration/full/region/region-pass.js | 6 +- 3 files changed, 121 insertions(+), 136 deletions(-) diff --git a/lib/checks/navigation/region.js b/lib/checks/navigation/region.js index 915bc9a3fd..cb9ecf8e6d 100644 --- a/lib/checks/navigation/region.js +++ b/lib/checks/navigation/region.js @@ -3,7 +3,7 @@ const landmarkRoles = aria.getRolesByType('landmark'); let regionlessNodes = axe._cache.get('regionlessNodes'); if (regionlessNodes) { - return !regionlessNodes.includes(node); + return !regionlessNodes.includes(virtualNode); } // Create a list of nodeNames that have a landmark as an implicit role @@ -53,7 +53,7 @@ function findRegionlessElms(virtualNode) { !dom.isVisible(node, true) ) { // Mark each parent node as having region descendant - let vNode = axe.utils.getNodeFromTree(node); + let vNode = virtualNode; while (vNode) { vNode._hasRegionDescendant = true; vNode = vNode.parent; @@ -63,7 +63,7 @@ function findRegionlessElms(virtualNode) { // Return the node is a content element } else if (dom.hasContent(node, /* noRecursion: */ true)) { - return [node]; + return [virtualNode]; // Recursively look at all child elements } else { @@ -74,11 +74,10 @@ function findRegionlessElms(virtualNode) { } } -regionlessNodes = findRegionlessElms(virtualNode) +regionlessNodes = findRegionlessElms(axe._tree[0]) // Find first parent marked as having region descendant (or body) and // return the node right before it as the "outer" element - .map(node => { - let vNode = axe.utils.getNodeFromTree(node); + .map(vNode => { while ( vNode.parent && !vNode.parent._hasRegionDescendant && @@ -87,13 +86,12 @@ regionlessNodes = findRegionlessElms(virtualNode) vNode = vNode.parent; } - return vNode.actualNode; + return vNode; }) // Remove duplicate containers - .filter((node, index, array) => { - return array.indexOf(node) === index; + .filter((vNode, index, array) => { + return array.indexOf(vNode) === index; }); -this.data(regionlessNodes); axe._cache.set('regionlessNodes', regionlessNodes); -return !regionlessNodes.includes(node); +return !regionlessNodes.includes(virtualNode); diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index 51befdafda..15a1b10c72 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -1,9 +1,10 @@ -describe('region', function() { +describe.only('region', function() { 'use strict'; var fixture = document.getElementById('fixture'); var shadowSupport = axe.testUtils.shadowSupport; var checkSetup = axe.testUtils.checkSetup; + var fixtureSetup = axe.testUtils.fixtureSetup; var checkContext = new axe.testUtils.MockCheckContext(); @@ -12,201 +13,178 @@ describe('region', function() { checkContext.reset(); }); - it('should return no regionless nodes when all content is inside the region', function() { + it('should return true when content is inside the region', function() { var checkArgs = checkSetup( - '
Click Here

Introduction

' + '
Click Here

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return regionless node when img content is outside the region', function() { + it('should return false when img content is outside the region', function() { var checkArgs = checkSetup( - '

Introduction

' + '

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes when textless text content is outside the region', function() { + it('should return true when textless text content is outside the region', function() { var checkArgs = checkSetup( - '

Introduction

' + '

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes when wrapper content is outside the region', function() { + it('should return true when wrapper content is outside the region', function() { var checkArgs = checkSetup( - '

Introduction

' + '

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes when invisible content is outside the region', function() { + it('should return true when invisible content is outside the region', function() { var checkArgs = checkSetup( - '

Click Here

Introduction

' + '

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes when there is a skiplink', function() { + it('should return true when there is a skiplink', function() { var checkArgs = checkSetup( - '
Click Here

Introduction

' + 'Click Here

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes when there is an Angular skiplink', function() { + it('should return true when there is an Angular skiplink', function() { var checkArgs = checkSetup( - '
Click Here

Introduction

' + 'Click Here

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return regionless nodes when there is a non-region element', function() { + it('should return false when there is a non-region element', function() { var checkArgs = checkSetup( - '
This is random content.

Introduction

' + '
This is random content.

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return regionless nodes when there is a non-skiplink', function() { + it('should return false when there is a non-skiplink', function() { var checkArgs = checkSetup( - '
Click Here

Introduction

' + 'Click Here

Introduction

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should return no regionless nodes if the non-region element is a script', function() { + it('should return true if the non-region element is a script', function() { var checkArgs = checkSetup( - '
Content
' + '
Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should consider aria labelled elements as content', function() { + it('should considered aria labelled elements as content', function() { var checkArgs = checkSetup( - '
Content
' + '
Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('should allow native landmark elements', function() { + it('should allow native header elements', function() { var checkArgs = checkSetup( - '
branding
Content
copyright
' + '
branding
Content
copyright
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('ignores native landmark elements with an overwriting role', function() { + it('should allow native main elements', function() { var checkArgs = checkSetup( - '
Content
' + '
branding
Content
copyright
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('returns regionless nodes for content outside of form tags with accessible names', function() { + it('should allow native aside elements', function() { var checkArgs = checkSetup( - '

Text

' + '
branding
Content
copyright
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('returns the outermost regionless element', function() { + it('should allow native footer elements', function() { var checkArgs = checkSetup( - '

Text

Text

Text

' + '
branding
Content
copyright
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); - it('returns the outermost element without a region descendant', function() { + it('ignores native landmark elements with an overwriting role', function() { var checkArgs = checkSetup( - '

Text

Text

Text

' + '
Content
Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 3); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); + }); + + it('returns false for content outside of form tags with accessible names', function() { + var checkArgs = checkSetup( + '

Text

' + ); + + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('ignores unlabeled forms as they are not landmarks', function() { var checkArgs = checkSetup( - '

Text

foo
' + '
foo
Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats with aria label as landmarks', function() { var checkArgs = checkSetup( '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats role=forms with aria label as landmarks', function() { var checkArgs = checkSetup( '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats forms without aria label as not a landmarks', function() { var checkArgs = checkSetup( - '

This is random content.

' + '

This is random content.

Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats forms with an empty aria label as not a landmarks', function() { var checkArgs = checkSetup( - '

This is random content.

' + '

This is random content.

Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats forms with non empty titles as landmarks', function() { @@ -214,18 +192,15 @@ describe('region', function() { '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats forms with empty titles not as landmarks', function() { var checkArgs = checkSetup( - '

This is random content.

' + '

This is random content.

Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats ARIA forms with no label or title as landmarks', function() { @@ -233,33 +208,28 @@ describe('region', function() { '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('allows content in aria-live=assertive', function() { var checkArgs = checkSetup( '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('allows content in aria-live=polite', function() { var checkArgs = checkSetup( '

This is random content.

' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('does not allow content in aria-live=off', function() { var checkArgs = checkSetup( - '

This is random content.

' + '

This is random content.

Content
' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); it('treats role=dialog elements as regions', function() { @@ -267,19 +237,33 @@ describe('region', function() { '' ); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + }); + + it('returns the outermost element as the error', function() { + var checkArgs = checkSetup( + '

This is random content.

Introduction

' + ); + + assert.isFalse(checks.region.evaluate.apply(checkContext, checkArgs)); }); (shadowSupport.v1 ? it : xit)('should test Shadow tree content', function() { var div = document.createElement('div'); var shadow = div.attachShadow({ mode: 'open' }); shadow.innerHTML = 'Some text'; - var checkArgs = checkSetup(div); - - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture); + fixtureSetup(div); + var virutalNode = axe._tree[0]; + + // fixture is the outermost element + assert.isFalse( + checks.region.evaluate.call( + checkContext, + virutalNode.actualNode, + null, + virutalNode + ) + ); }); (shadowSupport.v1 ? it : xit)('should test slotted content', function() { @@ -289,8 +273,7 @@ describe('region', function() { shadow.innerHTML = '
'; var checkArgs = checkSetup(div); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); }); (shadowSupport.v1 ? it : xit)( @@ -298,15 +281,21 @@ describe('region', function() { function() { var div = document.createElement('div'); div.innerHTML = - 'skiplink
Content
'; + 'skiplink
Content
'; var shadow = div.querySelector('div').attachShadow({ mode: 'open' }); shadow.innerHTML = '
'; - var checkArgs = checkSetup(div); - - checks.region.evaluate.apply(checkContext, checkArgs); - assert.lengthOf(checkContext._data, 1); - assert.equal(checkContext._data[0], fixture.querySelector('#regionless')); + fixtureSetup(div); + var virutalNode = axe.utils.getNodeFromTree(div.querySelector('#target')); + + assert.isFalse( + checks.region.evaluate.call( + checkContext, + virutalNode.actualNode, + null, + virutalNode + ) + ); } ); @@ -320,8 +309,8 @@ describe('region', function() { 'skiplink
'; var checkArgs = checkSetup(div); - checks.region.evaluate.apply(checkContext, checkArgs); - assert.isEmpty(checkContext._data); + assert.isTrue(checks.region.evaluate.apply(checkContext, checkArgs)); + assert.lengthOf(checkContext._relatedNodes, 0); } ); }); diff --git a/test/integration/full/region/region-pass.js b/test/integration/full/region/region-pass.js index a6406bb1f5..b52d914829 100644 --- a/test/integration/full/region/region-pass.js +++ b/test/integration/full/region/region-pass.js @@ -13,10 +13,8 @@ describe('region pass test', function() { }); describe('passes', function() { - it('should pass nodes', function() { - // it seems CircleCI and localhost have different number of DOM nodes, - // so as long as everything passes and nothing fails, the rule is working - assert.isTrue(results.passes[0].nodes.length > 0); + it('should find one', function() { + assert.lengthOf(results.passes[0].nodes, 33); }); }); From 75d96d306b25337303dbc75daca744dd2d97ef09 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Fri, 24 Jan 2020 11:57:19 -0700 Subject: [PATCH 7/8] revert pass --- test/integration/full/region/region-pass.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integration/full/region/region-pass.js b/test/integration/full/region/region-pass.js index b52d914829..a6406bb1f5 100644 --- a/test/integration/full/region/region-pass.js +++ b/test/integration/full/region/region-pass.js @@ -13,8 +13,10 @@ describe('region pass test', function() { }); describe('passes', function() { - it('should find one', function() { - assert.lengthOf(results.passes[0].nodes, 33); + it('should pass nodes', function() { + // it seems CircleCI and localhost have different number of DOM nodes, + // so as long as everything passes and nothing fails, the rule is working + assert.isTrue(results.passes[0].nodes.length > 0); }); }); From 2ed489fbee94395ac145e84173e2f10ee66bf1e0 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Mon, 27 Jan 2020 09:29:19 -0700 Subject: [PATCH 8/8] Update test/checks/navigation/region.js Co-Authored-By: Wilco Fiers --- test/checks/navigation/region.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/checks/navigation/region.js b/test/checks/navigation/region.js index 15a1b10c72..77d701cc84 100644 --- a/test/checks/navigation/region.js +++ b/test/checks/navigation/region.js @@ -1,4 +1,4 @@ -describe.only('region', function() { +describe('region', function() { 'use strict'; var fixture = document.getElementById('fixture');