From f9aa94c88f08b5f048f564ca9e411642e876dc6b Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Fri, 17 Jul 2020 16:05:29 +0200 Subject: [PATCH 1/2] feat(rule): optional impact on rules --- axe.d.ts | 1 + build/configure.js | 3 + build/tasks/validate.js | 4 ++ doc/API.md | 2 +- doc/rule-descriptions.md | 2 +- .../shared/has-accessible-text-evaluate.js | 7 --- lib/checks/shared/has-accessible-text.json | 11 ---- lib/core/base/metadata-function-map.js | 2 - lib/core/base/rule.js | 16 ++++- lib/core/utils/finalize-result.js | 11 ++++ lib/rules/empty-heading.json | 8 ++- test/core/base/rule.js | 21 +++++++ test/core/utils/finalize-result.js | 63 +++++++++++++++++++ 13 files changed, 127 insertions(+), 24 deletions(-) delete mode 100644 lib/checks/shared/has-accessible-text-evaluate.js delete mode 100644 lib/checks/shared/has-accessible-text.json diff --git a/axe.d.ts b/axe.d.ts index 26e4f2f247..1b72075138 100644 --- a/axe.d.ts +++ b/axe.d.ts @@ -141,6 +141,7 @@ declare namespace axe { interface Rule { id: string; selector?: string; + impact?: ImpactValue; excludeHidden?: boolean; enabled?: boolean; pageLevel?: boolean; diff --git a/build/configure.js b/build/configure.js index e05ea62ea0..a5cf76c0ae 100644 --- a/build/configure.js +++ b/build/configure.js @@ -224,6 +224,9 @@ function buildRules(grunt, options, commons, callback) { function capitalize(s) { return s.charAt(0).toUpperCase() + s.slice(1); } + if (rule.impact) { + return capitalize(rule.impact); + } function getUniqueArr(arr) { return arr.filter(function(value, index, self) { diff --git a/build/tasks/validate.js b/build/tasks/validate.js index d5007d43bb..7d6b7f6920 100644 --- a/build/tasks/validate.js +++ b/build/tasks/validate.js @@ -133,6 +133,10 @@ function createSchemas() { selector: { type: 'string' }, + impact: { + type: 'string', + enum: ['minor', 'moderate', 'serious', 'critical'] + }, excludeHidden: { type: 'boolean' }, diff --git a/doc/API.md b/doc/API.md index fb40c90063..2a26d50acb 100644 --- a/doc/API.md +++ b/doc/API.md @@ -202,7 +202,7 @@ axe.configure({ - `rules` - Used to add rules to the existing set of rules, or to override the properties of existing rules - The rules attribute is an Array of rule objects - each rule object can contain the following attributes - - `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules. + - `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules. - `impact` - string(optional). Override the impact defined by checks - `selector` - string(optional, default `*`). A [CSS selector](./developer-guide.md#supported-css-selectors) used to identify the elements that are passed into the rule for evaluation. - `excludeHidden` - boolean(optional, default `true`). This indicates whether elements that are hidden from all users are to be passed into the rule for evaluation. - `enabled` - boolean(optional, default `true`). Whether the rule is turned on. This is a common attribute for overriding. diff --git a/doc/rule-descriptions.md b/doc/rule-descriptions.md index fe97bff0b6..5b1e3515ac 100644 --- a/doc/rule-descriptions.md +++ b/doc/rule-descriptions.md @@ -75,7 +75,7 @@ Rules that do not necessarily conform to WCAG success criterion but are industry | :----------------------------------------------------------------------------------------------------------------------------------------------- | :--------------------------------------------------------------------------------------------------- | :----------------- | :----------------------------------------- | :------------------------- | | [accesskeys](https://dequeuniversity.com/rules/axe/3.5/accesskeys?application=RuleDescription) | Ensures every accesskey attribute value is unique | Serious | best-practice, cat.keyboard | failure | | [aria-allowed-role](https://dequeuniversity.com/rules/axe/3.5/aria-allowed-role?application=RuleDescription) | Ensures role attribute has an appropriate value for the element | Minor | cat.aria, best-practice | failure, needs review | -| [empty-heading](https://dequeuniversity.com/rules/axe/3.5/empty-heading?application=RuleDescription) | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | failure | +| [empty-heading](https://dequeuniversity.com/rules/axe/3.5/empty-heading?application=RuleDescription) | Ensures headings have discernible text | Minor | cat.name-role-value, best-practice | failure, needs review | | [frame-tested](https://dequeuniversity.com/rules/axe/3.5/frame-tested?application=RuleDescription) | Ensures <iframe> and <frame> elements contain the axe-core script | Critical | cat.structure, review-item, best-practice | failure, needs review | | [frame-title-unique](https://dequeuniversity.com/rules/axe/3.5/frame-title-unique?application=RuleDescription) | Ensures <iframe> and <frame> elements contain a unique title attribute | Serious | cat.text-alternatives, best-practice | failure | | [heading-order](https://dequeuniversity.com/rules/axe/3.5/heading-order?application=RuleDescription) | Ensures the order of headings is semantically correct | Moderate | cat.semantics, best-practice | failure | diff --git a/lib/checks/shared/has-accessible-text-evaluate.js b/lib/checks/shared/has-accessible-text-evaluate.js deleted file mode 100644 index 19222e8c49..0000000000 --- a/lib/checks/shared/has-accessible-text-evaluate.js +++ /dev/null @@ -1,7 +0,0 @@ -import { accessibleTextVirtual } from '../../commons/text'; - -function hasAccessibleTextEvaluate(node, options, virtualNode) { - return accessibleTextVirtual(virtualNode).length > 0; -} - -export default hasAccessibleTextEvaluate; diff --git a/lib/checks/shared/has-accessible-text.json b/lib/checks/shared/has-accessible-text.json deleted file mode 100644 index 8aa07402e5..0000000000 --- a/lib/checks/shared/has-accessible-text.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "id": "has-accessible-text", - "evaluate": "has-accessible-text-evaluate", - "metadata": { - "impact": "minor", - "messages": { - "pass": "Element has text that is visible to screen readers", - "fail": "Element does not have text that is visible to screen readers" - } - } -} diff --git a/lib/core/base/metadata-function-map.js b/lib/core/base/metadata-function-map.js index db99a49263..72914a876c 100644 --- a/lib/core/base/metadata-function-map.js +++ b/lib/core/base/metadata-function-map.js @@ -66,7 +66,6 @@ import ariaLabelledbyEvaluate from '../../checks/shared/aria-labelledby-evaluate import avoidInlineSpacingEvaluate from '../../checks/shared/avoid-inline-spacing-evaluate'; import docHasTitleEvaluate from '../../checks/shared/doc-has-title-evaluate'; import existsEvaluate from '../../checks/shared/exists-evaluate'; -import hasAccessibleTextEvaluate from '../../checks/shared/has-accessible-text-evaluate'; import hasAltEvaluate from '../../checks/shared/has-alt-evaluate'; import isOnScreenEvaluate from '../../checks/shared/is-on-screen-evaluate'; import nonEmptyIfPresentEvaluate from '../../checks/shared/non-empty-if-present-evaluate'; @@ -228,7 +227,6 @@ const metadataFunctionMap = { 'avoid-inline-spacing-evaluate': avoidInlineSpacingEvaluate, 'doc-has-title-evaluate': docHasTitleEvaluate, 'exists-evaluate': existsEvaluate, - 'has-accessible-text-evaluate': hasAccessibleTextEvaluate, 'has-alt-evaluate': hasAltEvaluate, 'is-on-screen-evaluate': isOnScreenEvaluate, 'non-empty-if-present-evaluate': nonEmptyIfPresentEvaluate, diff --git a/lib/core/base/rule.js b/lib/core/base/rule.js index d794ee1366..a2d7220229 100644 --- a/lib/core/base/rule.js +++ b/lib/core/base/rule.js @@ -8,8 +8,10 @@ import { queue, DqElement, select, - isHidden + isHidden, + assert } from '../utils'; +import constants from '../constants'; function Rule(spec, parentAudit) { this._audit = parentAudit; @@ -26,6 +28,18 @@ function Rule(spec, parentAudit) { */ this.selector = spec.selector || '*'; + /** + * Impact of the rule (optional) + * @type {"minor" | "moderate" | "serious" | "critical"} + */ + if (spec.impact) { + assert( + constants.impact.includes(spec.impact), + `Impact ${spec.impact} is not a valid impact` + ); + this.impact = spec.impact; + } + /** * Whether to exclude hiddden elements form analysis. Defaults to true. * @type {Boolean} diff --git a/lib/core/utils/finalize-result.js b/lib/core/utils/finalize-result.js index d09bea915a..bc36f71743 100644 --- a/lib/core/utils/finalize-result.js +++ b/lib/core/utils/finalize-result.js @@ -6,6 +6,17 @@ import aggregateNodeResults from './aggregate-node-results'; * @return {object} */ function finalizeRuleResult(ruleResult) { + const rule = axe._audit.rules.find(rule => rule.id === ruleResult.id); + if (rule && rule.impact) { + ruleResult.nodes.forEach(node => { + ['any', 'all', 'none'].forEach(checkType => { + (node[checkType] || []).forEach(checkResult => { + checkResult.impact = rule.impact; + }); + }); + }); + } + Object.assign(ruleResult, aggregateNodeResults(ruleResult.nodes)); delete ruleResult.nodes; diff --git a/lib/rules/empty-heading.json b/lib/rules/empty-heading.json index 391e041c8a..9efd7159e0 100644 --- a/lib/rules/empty-heading.json +++ b/lib/rules/empty-heading.json @@ -3,11 +3,17 @@ "selector": "h1, h2, h3, h4, h5, h6, [role=\"heading\"]", "matches": "heading-matches", "tags": ["cat.name-role-value", "best-practice"], + "impact": "minor", "metadata": { "description": "Ensures headings have discernible text", "help": "Headings must not be empty" }, "all": [], - "any": ["has-accessible-text"], + "any": [ + "has-visible-text", + "aria-label", + "aria-labelledby", + "non-empty-title" + ], "none": [] } diff --git a/test/core/base/rule.js b/test/core/base/rule.js index 282255503f..4ec6e3160f 100644 --- a/test/core/base/rule.js +++ b/test/core/base/rule.js @@ -1729,6 +1729,27 @@ describe('Rule', function() { }); }); + describe('.impact', function() { + it('should be set', function() { + var spec = { + impact: 'critical' + }; + assert.equal(new Rule(spec).impact, spec.impact); + }); + + it('should have no default', function() { + var spec = {}; + assert.isUndefined(new Rule(spec).impact); + }); + + it('throws if impact is invalid', function() { + assert.throws(function() { + // eslint-disable-next-line no-new + new Rule({ impact: 'hello' }); + }); + }); + }); + describe('.any', function() { it('should be set', function() { var spec = { diff --git a/test/core/utils/finalize-result.js b/test/core/utils/finalize-result.js index 5459bbb556..4233c8953d 100644 --- a/test/core/utils/finalize-result.js +++ b/test/core/utils/finalize-result.js @@ -1,5 +1,16 @@ describe('axe.utils.finalizeRuleResult', function() { 'use strict'; + var original = axe._audit; + + beforeEach(function() { + axe._audit = { + rules: [] + }; + }); + + after(function() { + axe._audit = original; + }); it('should be a function', function() { assert.isFunction(axe.utils.finalizeRuleResult); @@ -13,4 +24,56 @@ describe('axe.utils.finalizeRuleResult', function() { assert.equal(goingIn, comingOut); }); + + it('assigns impact if rule.impact is defined', function() { + axe._audit = { + rules: [{ id: 'foo', impact: 'critical' }] + }; + + var output = axe.utils.finalizeRuleResult({ + id: 'foo', + nodes: [ + { + any: [ + { + result: false, + impact: 'minor' + } + ], + all: [], + none: [] + } + ] + }); + + assert.equal(output.impact, 'critical'); + assert.equal(output.violations[0].impact, 'critical'); + assert.equal(output.violations[0].any[0].impact, 'critical'); + }); + + it('leaves impact as null when rule.impact is defined', function() { + axe._audit = { + rules: [{ id: 'foo', impact: 'critical' }] + }; + + var output = axe.utils.finalizeRuleResult({ + id: 'foo', + nodes: [ + { + any: [ + { + result: true, + impact: 'minor' + } + ], + all: [], + none: [] + } + ] + }); + + assert.isNull(output.impact); + assert.isNull(output.passes[0].impact); + assert.equal(output.passes[0].any[0].impact, 'critical'); + }); }); From c0e5740aa67477a56bb0ef047a73f0d8912f8c0e Mon Sep 17 00:00:00 2001 From: Wilco Fiers Date: Mon, 20 Jul 2020 11:41:02 +0200 Subject: [PATCH 2/2] Update API.md --- doc/API.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/API.md b/doc/API.md index 2a26d50acb..065c2f1f5b 100644 --- a/doc/API.md +++ b/doc/API.md @@ -202,7 +202,8 @@ axe.configure({ - `rules` - Used to add rules to the existing set of rules, or to override the properties of existing rules - The rules attribute is an Array of rule objects - each rule object can contain the following attributes - - `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules. - `impact` - string(optional). Override the impact defined by checks + - `id` - string(required). This uniquely identifies the rule. If the rule already exists, it will be overridden with any of the attributes supplied. The attributes below that are marked required, are only required for new rules. + - `impact` - string(optional). Override the impact defined by checks - `selector` - string(optional, default `*`). A [CSS selector](./developer-guide.md#supported-css-selectors) used to identify the elements that are passed into the rule for evaluation. - `excludeHidden` - boolean(optional, default `true`). This indicates whether elements that are hidden from all users are to be passed into the rule for evaluation. - `enabled` - boolean(optional, default `true`). Whether the rule is turned on. This is a common attribute for overriding.