Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(frame-focusable-content): new rule to test iframes with tabindex=-1 do not have focusable content #2785

Merged
merged 9 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions doc/rule-descriptions.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
Expand All @@ -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 |
Expand Down Expand Up @@ -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 |
Expand Down
12 changes: 12 additions & 0 deletions lib/checks/keyboard/frame-focusable-content.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
}
2 changes: 2 additions & 0 deletions lib/core/base/metadata-function-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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,
Expand Down
9 changes: 7 additions & 2 deletions lib/core/utils/collect-results-from-frames.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Contributor Author

@straker straker Jan 29, 2021

Choose a reason for hiding this comment

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

This prevents margin/border/padding CSS from affecting the width/height (otherwise in Chrome a width and height of 1 would result in a rect size of 5x5).


var params = {
options: options,
Expand All @@ -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 || [],
Expand Down
9 changes: 9 additions & 0 deletions lib/rules/frame-focusable-content-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
function frameFocusableContentMatches(node, virtualNode, context) {
return (
!context.initiator &&
!context.focusable &&
context.boundingClientRect.width * context.boundingClientRect.height > 1
);
}

export default frameFocusableContentMatches;
13 changes: 13 additions & 0 deletions lib/rules/frame-focusable-content.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"id": "frame-focusable-content",
"selector": "html",
"matches": "frame-focusable-content-matches",
"tags": ["cat.keyboard", "wcag2a", "wcag211"],
"metadata": {
"description": "Ensures <frame> and <iframe> elements with tabindex=-1 do not have focusable content",
"help": "Frames with tabindex=-1 must not have focusable content"
},
"all": [],
"any": ["frame-focusable-content"],
"none": []
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<iframe
src="/integration/rules/frame-focusable-content/frames/not-focusable.html"
tabindex="-1"
id="pass1"
></iframe>

<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
tabindex="-1"
id="fail1"
></iframe>
<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
tabindex="-1"
width="2"
height="1"
id="fail2"
></iframe>

<!-- inapplicable -->
<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
id="inapplicable-1"
></iframe>
<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
tabindex="0"
id="inapplicable-2"
></iframe>
<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
tabindex="-1"
width="1"
height="1"
id="inapplicable-3"
></iframe>
<iframe
src="/integration/rules/frame-focusable-content/frames/focusable.html"
tabindex="-1"
width="0"
height="0"
id="inapplicable-3"
></iframe>
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"description": "frame-focusable-content tests",
"rule": "frame-focusable-content",
"violations": [
["#fail1", "#focusable"],
["#fail2", "#focusable"]
],
"passes": [["#pass1", "#not-focusable"]]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html id="focusable">
<head>
<title>Hello</title>
<meta charset="utf8" />
<script src="/axe.js"></script>
</head>
<body>
<button>Click</button>
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<!DOCTYPE html>
<html id="not-focusable">
<head>
<title>Hello</title>
<meta charset="utf8" />
<script src="/axe.js"></script>
</head>
<body>
<p>Hello</p>
</body>
</html>
68 changes: 68 additions & 0 deletions test/rule-matches/frame-focusable-content-matches.js
Original file line number Diff line number Diff line change
@@ -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);
});
});