From aeb044c26908b44490bad160add8c3e6327ce759 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Tue, 2 Feb 2021 09:16:43 -0700 Subject: [PATCH] feat(frame-focusable-content): new rule to test iframes with tabindex=-1 do not have focusable content (#2785) * feat(frame-focusable-content): new rule to test iframes with tabindex=-1 do not have focusable content * add inapplicable * fix tests * after results * test * hmmm * fix * remove after * fix test --- doc/rule-descriptions.md | 5 +- .../keyboard/frame-focusable-content.json | 12 ++++ lib/core/base/metadata-function-map.js | 2 + lib/core/utils/collect-results-from-frames.js | 9 ++- lib/rules/frame-focusable-content-matches.js | 9 +++ lib/rules/frame-focusable-content.json | 13 ++++ .../frame-focusable-content.html | 43 ++++++++++++ .../frame-focusable-content.json | 9 +++ .../frames/focusable.html | 11 +++ .../frames/not-focusable.html | 11 +++ .../frame-focusable-content-matches.js | 68 +++++++++++++++++++ 11 files changed, 188 insertions(+), 4 deletions(-) create mode 100644 lib/checks/keyboard/frame-focusable-content.json create mode 100644 lib/rules/frame-focusable-content-matches.js create mode 100644 lib/rules/frame-focusable-content.json create mode 100644 test/integration/rules/frame-focusable-content/frame-focusable-content.html create mode 100644 test/integration/rules/frame-focusable-content/frame-focusable-content.json create mode 100644 test/integration/rules/frame-focusable-content/frames/focusable.html create mode 100644 test/integration/rules/frame-focusable-content/frames/not-focusable.html create mode 100644 test/rule-matches/frame-focusable-content-matches.js diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index 7d82c348c1..6440a01b39 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -41,6 +41,7 @@ | [duplicate-id-aria](https://dequeuniversity.com/rules/axe/4.1/duplicate-id-aria?application=RuleDescription) | Ensures every id attribute value used in ARIA and in labels is unique | Critical | cat.parsing, wcag2a, wcag411 | failure | | [duplicate-id](https://dequeuniversity.com/rules/axe/4.1/duplicate-id?application=RuleDescription) | Ensures every id attribute value is unique | Minor | cat.parsing, wcag2a, wcag411 | failure | | [form-field-multiple-labels](https://dequeuniversity.com/rules/axe/4.1/form-field-multiple-labels?application=RuleDescription) | Ensures form field does not have multiple label elements | Moderate | cat.forms, wcag2a, wcag332 | needs review | +| [frame-focusable-content](https://dequeuniversity.com/rules/axe/4.1/frame-focusable-content?application=RuleDescription) | Ensures <frame> and <iframe> elements with tabindex=-1 do not have focusable content | Serious | cat.keyboard, wcag2a, wcag211 | failure, needs review | | [frame-title](https://dequeuniversity.com/rules/axe/4.1/frame-title?application=RuleDescription) | Ensures <iframe> and <frame> elements have an accessible name | Serious | cat.text-alternatives, wcag2a, wcag241, wcag412, section508, section508.22.i | failure, needs review | | [html-has-lang](https://dequeuniversity.com/rules/axe/4.1/html-has-lang?application=RuleDescription) | Ensures every HTML document has a lang attribute | Serious | cat.language, wcag2a, wcag311, ACT | failure | | [html-lang-valid](https://dequeuniversity.com/rules/axe/4.1/html-lang-valid?application=RuleDescription) | Ensures the lang attribute of the <html> element has a valid value | Serious | cat.language, wcag2a, wcag311, ACT | failure | @@ -57,7 +58,7 @@ | [nested-interactive](https://dequeuniversity.com/rules/axe/4.1/nested-interactive?application=RuleDescription) | Nested interactive controls are not announced by screen readers | Serious | cat.keyboard, wcag2a, wcag412 | failure, needs review | | [object-alt](https://dequeuniversity.com/rules/axe/4.1/object-alt?application=RuleDescription) | Ensures <object> elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a | failure, needs review | | [role-img-alt](https://dequeuniversity.com/rules/axe/4.1/role-img-alt?application=RuleDescription) | Ensures [role='img'] elements have alternate text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a, ACT | failure, needs review | -| [scrollable-region-focusable](https://dequeuniversity.com/rules/axe/4.1/scrollable-region-focusable?application=RuleDescription) | Elements that have scrollable content should be accessible by keyboard | Moderate | cat.keyboard, wcag2a, wcag211 | failure | +| [scrollable-region-focusable](https://dequeuniversity.com/rules/axe/4.1/scrollable-region-focusable?application=RuleDescription) | Elements that have scrollable content must be accessible by keyboard | Moderate | cat.keyboard, wcag2a, wcag211 | failure | | [select-name](https://dequeuniversity.com/rules/axe/4.1/select-name?application=RuleDescription) | Ensures select element has an accessible name | Minor, Critical | cat.forms, wcag2a, wcag412, wcag131, section508, section508.22.n, ACT | failure, needs review | | [server-side-image-map](https://dequeuniversity.com/rules/axe/4.1/server-side-image-map?application=RuleDescription) | Ensures that server-side image maps are not used | Minor | cat.text-alternatives, wcag2a, wcag211, section508, section508.22.f | needs review | | [svg-img-alt](https://dequeuniversity.com/rules/axe/4.1/svg-img-alt?application=RuleDescription) | Ensures svg elements with an img, graphics-document or graphics-symbol role have an accessible text | Serious | cat.text-alternatives, wcag2a, wcag111, section508, section508.22.a, ACT | failure, needs review | @@ -99,7 +100,7 @@ Rules that do not necessarily conform to WCAG success criterion but are industry | [landmark-no-duplicate-contentinfo](https://dequeuniversity.com/rules/axe/4.1/landmark-no-duplicate-contentinfo?application=RuleDescription) | Ensures the document has at most one contentinfo landmark | Moderate | cat.semantics, best-practice | failure | | [landmark-no-duplicate-main](https://dequeuniversity.com/rules/axe/4.1/landmark-no-duplicate-main?application=RuleDescription) | Ensures the document has at most one main landmark | Moderate | cat.semantics, best-practice | failure | | [landmark-one-main](https://dequeuniversity.com/rules/axe/4.1/landmark-one-main?application=RuleDescription) | Ensures the document has a main landmark | Moderate | cat.semantics, best-practice | failure | -| [landmark-unique](https://dequeuniversity.com/rules/axe/4.1/landmark-unique?application=RuleDescription) | Landmarks must have a unique role or role/label/title (i.e. accessible name) combination | Moderate | cat.semantics, best-practice | failure | +| [landmark-unique](https://dequeuniversity.com/rules/axe/4.1/landmark-unique?application=RuleDescription) | Landmarks should have a unique role or role/label/title (i.e. accessible name) combination | Moderate | cat.semantics, best-practice | failure | | [meta-viewport-large](https://dequeuniversity.com/rules/axe/4.1/meta-viewport-large?application=RuleDescription) | Ensures <meta name="viewport"> can scale a significant amount | Minor | cat.sensory-and-visual-cues, best-practice | failure | | [meta-viewport](https://dequeuniversity.com/rules/axe/4.1/meta-viewport?application=RuleDescription) | Ensures <meta name="viewport"> does not disable text scaling and zooming | Critical | cat.sensory-and-visual-cues, best-practice, ACT | failure | | [page-has-heading-one](https://dequeuniversity.com/rules/axe/4.1/page-has-heading-one?application=RuleDescription) | Ensure that the page, or at least one of its frames contains a level-one heading | Moderate | cat.semantics, best-practice | failure | diff --git a/lib/checks/keyboard/frame-focusable-content.json b/lib/checks/keyboard/frame-focusable-content.json new file mode 100644 index 0000000000..5157665b2e --- /dev/null +++ b/lib/checks/keyboard/frame-focusable-content.json @@ -0,0 +1,12 @@ +{ + "id": "frame-focusable-content", + "evaluate": "no-focusable-content-evaluate", + "metadata": { + "impact": "serious", + "messages": { + "pass": "Element does not have focusable descendants", + "fail": "Element has focusable descendants", + "incomplete": "Could not determine if element has descendants" + } + } +} diff --git a/lib/core/base/metadata-function-map.js b/lib/core/base/metadata-function-map.js index 1a1ae7ce3f..cf1a17de4a 100644 --- a/lib/core/base/metadata-function-map.js +++ b/lib/core/base/metadata-function-map.js @@ -143,6 +143,7 @@ import dataTableMatches from '../../rules/data-table-matches'; import duplicateIdActiveMatches from '../../rules/duplicate-id-active-matches'; import duplicateIdAriaMatches from '../../rules/duplicate-id-aria-matches'; import duplicateIdMiscMatches from '../../rules/duplicate-id-misc-matches'; +import frameFocusableContentMatches from '../../rules/frame-focusable-content-matches'; import frameTitleHasTextMatches from '../../rules/frame-title-has-text-matches'; import headingMatches from '../../rules/heading-matches'; import htmlNamespaceMatches from '../../rules/html-namespace-matches'; @@ -317,6 +318,7 @@ const metadataFunctionMap = { 'duplicate-id-active-matches': duplicateIdActiveMatches, 'duplicate-id-aria-matches': duplicateIdAriaMatches, 'duplicate-id-misc-matches': duplicateIdMiscMatches, + 'frame-focusable-content-matches': frameFocusableContentMatches, 'frame-title-has-text-matches': frameTitleHasTextMatches, 'heading-matches': headingMatches, 'html-namespace-matches': htmlNamespaceMatches, diff --git a/lib/core/utils/collect-results-from-frames.js b/lib/core/utils/collect-results-from-frames.js index 493fc1972a..e216cf947f 100644 --- a/lib/core/utils/collect-results-from-frames.js +++ b/lib/core/utils/collect-results-from-frames.js @@ -27,7 +27,12 @@ function collectResultsFromFrames( frames.forEach(frame => { const tabindex = parseInt(frame.node.getAttribute('tabindex'), 10); const focusable = isNaN(tabindex) || tabindex >= 0; + const rect = frame.node.getBoundingClientRect(); + let width = parseInt(frame.node.getAttribute('width'), 10); + let height = parseInt(frame.node.getAttribute('height'), 10); + width = isNaN(width) ? rect.width : width; + height = isNaN(height) ? rect.height : height; var params = { options: options, @@ -41,8 +46,8 @@ function collectResultsFromFrames( // iframe has tabindex=0 on it) focusable: parentContent.focusable === false ? false : focusable, boundingClientRect: { - width: rect.width, - height: rect.height + width: width, + height: height }, page: parentContent.page, include: frame.include || [], diff --git a/lib/rules/frame-focusable-content-matches.js b/lib/rules/frame-focusable-content-matches.js new file mode 100644 index 0000000000..5195e41140 --- /dev/null +++ b/lib/rules/frame-focusable-content-matches.js @@ -0,0 +1,9 @@ +function frameFocusableContentMatches(node, virtualNode, context) { + return ( + !context.initiator && + !context.focusable && + context.boundingClientRect.width * context.boundingClientRect.height > 1 + ); +} + +export default frameFocusableContentMatches; diff --git a/lib/rules/frame-focusable-content.json b/lib/rules/frame-focusable-content.json new file mode 100644 index 0000000000..dc0b14cb51 --- /dev/null +++ b/lib/rules/frame-focusable-content.json @@ -0,0 +1,13 @@ +{ + "id": "frame-focusable-content", + "selector": "html", + "matches": "frame-focusable-content-matches", + "tags": ["cat.keyboard", "wcag2a", "wcag211"], + "metadata": { + "description": "Ensures and + + + + + + + + + diff --git a/test/integration/rules/frame-focusable-content/frame-focusable-content.json b/test/integration/rules/frame-focusable-content/frame-focusable-content.json new file mode 100644 index 0000000000..0dcfe65674 --- /dev/null +++ b/test/integration/rules/frame-focusable-content/frame-focusable-content.json @@ -0,0 +1,9 @@ +{ + "description": "frame-focusable-content tests", + "rule": "frame-focusable-content", + "violations": [ + ["#fail1", "#focusable"], + ["#fail2", "#focusable"] + ], + "passes": [["#pass1", "#not-focusable"]] +} diff --git a/test/integration/rules/frame-focusable-content/frames/focusable.html b/test/integration/rules/frame-focusable-content/frames/focusable.html new file mode 100644 index 0000000000..de5374ba35 --- /dev/null +++ b/test/integration/rules/frame-focusable-content/frames/focusable.html @@ -0,0 +1,11 @@ + + + + Hello + + + + + + + diff --git a/test/integration/rules/frame-focusable-content/frames/not-focusable.html b/test/integration/rules/frame-focusable-content/frames/not-focusable.html new file mode 100644 index 0000000000..6b3e413940 --- /dev/null +++ b/test/integration/rules/frame-focusable-content/frames/not-focusable.html @@ -0,0 +1,11 @@ + + + + Hello + + + + +

Hello

+ + diff --git a/test/rule-matches/frame-focusable-content-matches.js b/test/rule-matches/frame-focusable-content-matches.js new file mode 100644 index 0000000000..dc784072dd --- /dev/null +++ b/test/rule-matches/frame-focusable-content-matches.js @@ -0,0 +1,68 @@ +describe('frame-focusable-content-matches', function() { + 'use strict'; + var rule; + + beforeEach(function() { + rule = axe.utils.getRule('frame-focusable-content'); + }); + + it('returns false for the top-level context', function() { + var result = rule.matches(null, null, { + initiator: true, + focusable: false, + boundingClientRect: { + width: 100, + height: 100 + } + }); + assert.isFalse(result); + }); + + it('returns false for focusable iframes', function() { + var result = rule.matches(null, null, { + initiator: false, + focusable: true, + boundingClientRect: { + width: 100, + height: 100 + } + }); + assert.isFalse(result); + }); + + it('returns false for non-focusable iframes that are too small (1x1)', function() { + var result = rule.matches(null, null, { + initiator: false, + focusable: false, + boundingClientRect: { + width: 1, + height: 1 + } + }); + assert.isFalse(result); + }); + + it('returns false for non-focusable iframes that are too small (0x0)', function() { + var result = rule.matches(null, null, { + initiator: false, + focusable: false, + boundingClientRect: { + width: 0, + height: 0 + } + }); + assert.isFalse(result); + }); + + it('returns true for non-focusable iframes', function() { + var result = rule.matches(null, null, { + initiator: false, + focusable: false, + boundingClientRect: { + width: 2, + height: 1 + } + }); + assert.isTrue(result); + }); +});