From 28a35531b97b987e2fd1ad0beb25fbda3822fbd5 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 30 Jan 2020 08:59:45 -0700 Subject: [PATCH] fix(aria-hidden-focus): mark as needs review if a modal is open (#1995) * fix(aria-hidden-focus): mark as needs review if a modal is open * message * fix tests * fix windows * resolve changes * tests --- doc/rule-descriptions.md | 2 +- lib/checks/keyboard/focusable-disabled.js | 5 + lib/checks/keyboard/focusable-modal-open.js | 14 +++ lib/checks/keyboard/focusable-modal-open.json | 11 +++ lib/checks/keyboard/focusable-not-tabbable.js | 5 + lib/commons/dom/is-modal-open.js | 98 +++++++++++++++++++ lib/rules/aria-hidden-focus.json | 6 +- test/checks/keyboard/focusable-disabled.js | 11 +++ test/checks/keyboard/focusable-modal-open.js | 55 +++++++++++ .../checks/keyboard/focusable-not-tabbable.js | 11 +++ test/commons/dom/is-modal-open.js | 89 +++++++++++++++++ .../full/aria-hidden-focus/modal.html | 61 ++++++++++++ .../full/aria-hidden-focus/modal.js | 38 +++++++ 13 files changed, 404 insertions(+), 2 deletions(-) create mode 100644 lib/checks/keyboard/focusable-modal-open.js create mode 100644 lib/checks/keyboard/focusable-modal-open.json create mode 100644 lib/commons/dom/is-modal-open.js create mode 100644 test/checks/keyboard/focusable-modal-open.js create mode 100644 test/commons/dom/is-modal-open.js create mode 100644 test/integration/full/aria-hidden-focus/modal.html create mode 100644 test/integration/full/aria-hidden-focus/modal.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index e049bd7270..b978dcdebb 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -6,7 +6,7 @@ | aria-allowed-role | Ensures role attribute has an appropriate value for the element | Minor | cat.aria, best-practice | true | true | true | | aria-dpub-role-fallback | Ensures unsupported DPUB roles are only used on elements with implicit fallback roles | Moderate | cat.aria, wcag2a, wcag131, deprecated | false | true | false | | aria-hidden-body | Ensures aria-hidden='true' is not present on the document body. | Critical | cat.aria, wcag2a, wcag412 | true | true | false | -| aria-hidden-focus | Ensures aria-hidden elements do not contain focusable elements | Serious | cat.name-role-value, wcag2a, wcag412, wcag131 | true | true | false | +| aria-hidden-focus | Ensures aria-hidden elements do not contain focusable elements | Serious | cat.name-role-value, wcag2a, wcag412, wcag131 | true | true | true | | aria-input-field-name | Ensures every ARIA input field has an accessible name | Moderate, Serious | wcag2a, wcag412 | true | true | true | | aria-required-attr | Ensures elements with ARIA roles have all required ARIA attributes | Critical | cat.aria, wcag2a, wcag412 | true | true | false | | aria-required-children | Ensures elements with an ARIA role that require child roles contain them | Critical | cat.aria, wcag2a, wcag131 | true | true | true | diff --git a/lib/checks/keyboard/focusable-disabled.js b/lib/checks/keyboard/focusable-disabled.js index 6aa69afffb..46af6cee35 100644 --- a/lib/checks/keyboard/focusable-disabled.js +++ b/lib/checks/keyboard/focusable-disabled.js @@ -20,6 +20,11 @@ const relatedNodes = tabbableElements.reduce((out, { actualNode: el }) => { } return out; }, []); + this.relatedNodes(relatedNodes); +if (relatedNodes.length && axe.commons.dom.isModalOpen()) { + return true; +} + return relatedNodes.length === 0; diff --git a/lib/checks/keyboard/focusable-modal-open.js b/lib/checks/keyboard/focusable-modal-open.js new file mode 100644 index 0000000000..49a9621578 --- /dev/null +++ b/lib/checks/keyboard/focusable-modal-open.js @@ -0,0 +1,14 @@ +const tabbableElements = virtualNode.tabbableElements.map( + ({ actualNode }) => actualNode +); + +if (!tabbableElements || !tabbableElements.length) { + return true; +} + +if (axe.commons.dom.isModalOpen()) { + this.relatedNodes(tabbableElements); + return undefined; +} + +return true; diff --git a/lib/checks/keyboard/focusable-modal-open.json b/lib/checks/keyboard/focusable-modal-open.json new file mode 100644 index 0000000000..3ba9530893 --- /dev/null +++ b/lib/checks/keyboard/focusable-modal-open.json @@ -0,0 +1,11 @@ +{ + "id": "focusable-modal-open", + "evaluate": "focusable-modal-open.js", + "metadata": { + "impact": "serious", + "messages": { + "pass": "No focusable elements while a modal is open", + "incomplete": "Check that focusable elements are not tabbable in the current state" + } + } +} diff --git a/lib/checks/keyboard/focusable-not-tabbable.js b/lib/checks/keyboard/focusable-not-tabbable.js index 33d978f2d9..82178ae6a1 100644 --- a/lib/checks/keyboard/focusable-not-tabbable.js +++ b/lib/checks/keyboard/focusable-not-tabbable.js @@ -20,6 +20,11 @@ const relatedNodes = tabbableElements.reduce((out, { actualNode: el }) => { } return out; }, []); + this.relatedNodes(relatedNodes); +if (relatedNodes.length > 0 && axe.commons.dom.isModalOpen()) { + return true; +} + return relatedNodes.length === 0; diff --git a/lib/commons/dom/is-modal-open.js b/lib/commons/dom/is-modal-open.js new file mode 100644 index 0000000000..d6e21c8363 --- /dev/null +++ b/lib/commons/dom/is-modal-open.js @@ -0,0 +1,98 @@ +/* global dom, axe */ + +/** + * Determines if there is a modal currently open. + * @method isModalOpen + * @memberof axe.commons.dom + * @instance + * @return {Boolean|undefined} True if we know (or our best guess) that a modal is open, undefined if we can't tell (doesn't mean there isn't one open) + */ +dom.isModalOpen = function isModalOpen(options) { + options = options || {}; + let modalPercent = options.modalPercent || 0.75; + + // there is no "definitive" way to code a modal so detecting when one is open + // is a bit of a guess. a modal won't always be accessible, so we can't rely + // on the `role` attribute, and relying on a class name as a convention is + // unreliable. we also cannot rely on the body/html not scrolling. + // + // because of this, we will look for two different types of modals: + // "definitely a modal" and "could be a modal." + // + // "definitely a modal" is any visible element that is coded to be a modal + // by using one of the following criteria: + // + // - has the attribute `role=dialog` + // - has the attribute `aria-modal=true` + // - is the dialog element + // + // "could be a modal" is a visible element that takes up more than 75% of + // the screen (though typically full width/height) and is the top-most element + // in the viewport. since we aren't sure if it is or is not a modal this is + // just our best guess of being one based on convention. + + if (axe._cache.get('isModalOpen')) { + return axe._cache.get('isModalOpen'); + } + + const definiteModals = axe.utils.querySelectorAllFilter( + axe._tree[0], + 'dialog, [role=dialog], [aria-modal=true]', + vNode => dom.isVisible(vNode.actualNode) + ); + + if (definiteModals.length) { + axe._cache.set('isModalOpen', true); + return true; + } + + // to find a "could be a modal" we will take the element stack from each of + // four corners and one from the middle of the viewport (total of 5). if each + // stack contains an element whose width/height is >= 75% of the screen, we + // found a "could be a modal" + const viewport = dom.getViewportSize(window); + const percentWidth = viewport.width * modalPercent; + const percentHeight = viewport.height * modalPercent; + const x = (viewport.width - percentWidth) / 2; + const y = (viewport.height - percentHeight) / 2; + + const points = [ + // top-left corner + { x, y }, + // top-right corner + { x: viewport.width - x, y }, + // center + { x: viewport.width / 2, y: viewport.height / 2 }, + // bottom-left corner + { x, y: viewport.height - y }, + // bottom-right corner + { x: viewport.width - x, y: viewport.height - y } + ]; + + const stacks = points.map(point => { + return Array.from(document.elementsFromPoint(point.x, point.y)); + }); + + for (let i = 0; i < stacks.length; i++) { + // a modal isn't guaranteed to be the top most element so we'll have to + // find the first element in the stack that meets the modal criteria + // and make sure it's in the other stacks + const modalElement = stacks[i].find(elm => { + const style = window.getComputedStyle(elm); + return ( + parseInt(style.width, 10) >= percentWidth && + parseInt(style.height, 10) >= percentHeight && + style.getPropertyValue('pointer-events') !== 'none' && + (style.position === 'absolute' || style.position === 'fixed') + ); + }); + + if (modalElement && stacks.every(stack => stack.includes(modalElement))) { + axe._cache.set('isModalOpen', true); + return true; + } + } + + axe._cache.set('isModalOpen', undefined); + return undefined; +}; diff --git a/lib/rules/aria-hidden-focus.json b/lib/rules/aria-hidden-focus.json index 4d3460eada..93aeada210 100755 --- a/lib/rules/aria-hidden-focus.json +++ b/lib/rules/aria-hidden-focus.json @@ -8,7 +8,11 @@ "description": "Ensures aria-hidden elements do not contain focusable elements", "help": "ARIA hidden element must not contain focusable elements" }, - "all": ["focusable-disabled", "focusable-not-tabbable"], + "all": [ + "focusable-modal-open", + "focusable-disabled", + "focusable-not-tabbable" + ], "any": [], "none": [] } diff --git a/test/checks/keyboard/focusable-disabled.js b/test/checks/keyboard/focusable-disabled.js index 1c023fe289..7bf3e3a2b7 100644 --- a/test/checks/keyboard/focusable-disabled.js +++ b/test/checks/keyboard/focusable-disabled.js @@ -129,4 +129,15 @@ describe('focusable-disabled', function() { var actual = check.evaluate.apply(checkContext, params); assert.isFalse(actual); }); + + it('returns true if there is a focusable element and modal is open', function() { + var params = checkSetup( + '' + + '
Modal
' + ); + var actual = check.evaluate.apply(checkContext, params); + assert.isTrue(actual); + }); }); diff --git a/test/checks/keyboard/focusable-modal-open.js b/test/checks/keyboard/focusable-modal-open.js new file mode 100644 index 0000000000..68dc7aa2f3 --- /dev/null +++ b/test/checks/keyboard/focusable-modal-open.js @@ -0,0 +1,55 @@ +describe('focusable-modal-open', function() { + 'use strict'; + + var check; + var fixture = document.getElementById('fixture'); + var checkContext = axe.testUtils.MockCheckContext(); + var checkSetup = axe.testUtils.checkSetup; + + before(function() { + check = checks['focusable-modal-open']; + }); + + afterEach(function() { + fixture.innerHTML = ''; + axe._tree = undefined; + axe._selectorData = undefined; + checkContext.reset(); + }); + + it('returns true when no modal is open', function() { + var params = checkSetup( + '' + ); + var actual = check.evaluate.apply(checkContext, params); + assert.isTrue(actual); + }); + + it('returns undefined if a modal is open', function() { + var params = checkSetup( + '' + + '
Modal
' + ); + var actual = check.evaluate.apply(checkContext, params); + assert.isUndefined(actual); + }); + + it('sets the tabbable elements as related nodes', function() { + var params = checkSetup( + '' + + '
Modal
' + ); + check.evaluate.apply(checkContext, params); + assert.lengthOf(checkContext._relatedNodes, 1); + assert.deepEqual( + checkContext._relatedNodes, + Array.from(fixture.querySelectorAll('button')) + ); + }); +}); diff --git a/test/checks/keyboard/focusable-not-tabbable.js b/test/checks/keyboard/focusable-not-tabbable.js index 686258baf7..dd552d1185 100644 --- a/test/checks/keyboard/focusable-not-tabbable.js +++ b/test/checks/keyboard/focusable-not-tabbable.js @@ -133,4 +133,15 @@ describe('focusable-not-tabbable', function() { var actual = check.evaluate.apply(checkContext, params); assert.isTrue(actual); }); + + it('returns true if there is a focusable element and modal is open', function() { + var params = checkSetup( + '' + + '
Modal
' + ); + var actual = check.evaluate.apply(checkContext, params); + assert.isTrue(actual); + }); }); diff --git a/test/commons/dom/is-modal-open.js b/test/commons/dom/is-modal-open.js new file mode 100644 index 0000000000..d0d0835f7f --- /dev/null +++ b/test/commons/dom/is-modal-open.js @@ -0,0 +1,89 @@ +describe('dom.isModalOpen', function() { + 'use strict'; + + var fixtureSetup = axe.testUtils.fixtureSetup; + var isModalOpen = axe.commons.dom.isModalOpen; + var dialogElSupport = + typeof document.createElement('dialog').open !== 'undefined'; + + afterEach(function() { + fixtureSetup(''); + }); + + it('returns true if there is a visible element with role=dialog', function() { + fixtureSetup('
Modal
'); + assert.isTrue(isModalOpen()); + }); + + it('returns true if there is a visible element with aria-modal=true', function() { + fixtureSetup('
Modal
'); + assert.isTrue(isModalOpen()); + }); + + (dialogElSupport ? it : xit)( + 'returns true if there is a visible dialog element', + function() { + fixtureSetup('
Modal
'); + assert.isTrue(isModalOpen()); + } + ); + + it('returns true if there is a visible absolutely positioned element with >= 75% width/height', function() { + fixtureSetup( + '
Modal
' + ); + assert.isTrue(isModalOpen()); + }); + + it('returns true if there is a visible absolutely positioned element with >= 75% width/height and is not the top most element', function() { + fixtureSetup( + '
' + + '
' + + '
Modal
' + + '
' + + '
' + ); + assert.isTrue(isModalOpen()); + }); + + it('returns true if modal opens like a drawer', function() { + fixtureSetup( + '
' + + '
' + + '
Modal
' + + '
' + + '
' + ); + assert.isTrue(isModalOpen()); + }); + + it('returns undefined if there is no modal', function() { + fixtureSetup('
Modal
'); + assert.isUndefined(isModalOpen()); + }); + + it('returns undefined if there is a hidden element with role=dialog', function() { + fixtureSetup('
Modal
'); + assert.isUndefined(isModalOpen()); + }); + + it('returns undefined if there is a hidden element with aria-modal=true', function() { + fixtureSetup('
Modal
'); + assert.isUndefined(isModalOpen()); + }); + + (dialogElSupport ? it : xit)( + 'returns undefined if there is a hidden dialog element', + function() { + fixtureSetup('
Modal
'); + assert.isUndefined(isModalOpen()); + } + ); + + it('returns undefined if there is a visible absolutely positioned element with < 75% width/height', function() { + fixtureSetup( + '
+ + + + + + + + + +
+

Contents of page

+ +
+
+
+ Subscribe to our newsletter! +
+
+ + + + + diff --git a/test/integration/full/aria-hidden-focus/modal.js b/test/integration/full/aria-hidden-focus/modal.js new file mode 100644 index 0000000000..ffac223990 --- /dev/null +++ b/test/integration/full/aria-hidden-focus/modal.js @@ -0,0 +1,38 @@ +describe('aria-hidden-focus test', function() { + 'use strict'; + var results; + before(function(done) { + axe.testUtils.awaitNestedLoad(function() { + axe.run( + { runOnly: { type: 'rule', values: ['aria-hidden-focus'] } }, + function(err, r) { + assert.isNull(err); + results = r; + done(); + } + ); + }); + }); + + describe('violations', function() { + it('should find 0 violations', function() { + assert.lengthOf(results.violations, 0); + }); + }); + + describe('passes', function() { + it('should find 0 passes', function() { + assert.lengthOf(results.violations, 0); + }); + }); + + describe('incomplete', function() { + it('should find 1', function() { + assert.lengthOf(results.incomplete[0].nodes, 1); + }); + + it('should find #incomplete1', function() { + assert.deepEqual(results.incomplete[0].nodes[0].target, ['#incomplete1']); + }); + }); +});