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

core(audits): add handling of 'incomplete' results from axe-core #10072

Merged
merged 68 commits into from
Jan 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
68 commits
Select commit Hold shift + click to select a range
1fc1f1c
core(audits): remove form-field-multiple-labels audit
jayaddison Dec 6, 2019
2e93d84
Update per axe-core#1798: multiple-labels no longer fails
jayaddison Nov 30, 2019
0dd38c3
Nit: remove end-of-file newlines
jayaddison Dec 6, 2019
996d1f1
Update heading-order path expectation
jayaddison Dec 6, 2019
fb8e2ac
Run yarnpkg update:sample-json
jayaddison Dec 6, 2019
4430870
Update category-render-test score expectation
jayaddison Dec 6, 2019
abc5e5f
Coerce null summaries for all round-trip-JSON test suites
jayaddison Dec 6, 2019
ac1acfc
Merge branch 'master' into core-remove-form-field-multiple-labels
jayaddison Dec 8, 2019
f9975f8
Remove accidentally-committed vim .swp
jayaddison Dec 9, 2019
f630673
Remove null coercion for 'summary' field
jayaddison Dec 9, 2019
2abac27
Revert "Run yarnpkg update:sample-json"
jayaddison Dec 9, 2019
e910079
Revert "Update category-render-test score expectation"
jayaddison Dec 9, 2019
d48a36d
Revert "Revert "Update category-render-test score expectation""
jayaddison Dec 9, 2019
6f53962
Update a11y score expectations (minus 'summary' field value updates)
jayaddison Dec 9, 2019
ea29b71
Merge branch 'master' into core-remove-form-field-multiple-labels
jayaddison Dec 11, 2019
35666fe
Temporarily disable Travis cache
jayaddison Dec 11, 2019
330926b
Revert "Temporarily disable Travis cache"
jayaddison Dec 11, 2019
70fc34f
Revert form-field-multiple-labels audit removal
jayaddison Dec 11, 2019
47a607b
Set scoring mode for form-field-multiple-labels to 'informative'
jayaddison Dec 17, 2019
3596ace
Merge branch 'master' into core-remove-form-field-multiple-labels
jayaddison Dec 17, 2019
20ae5c6
Update per axe-core#1798: multiple-labels no longer fails
jayaddison Nov 30, 2019
9eef895
Restore form-field-multiple-labels expectation, with updated score: null
jayaddison Dec 18, 2019
942b0a1
Additionally gather audit results from axe-core 'incomplete' entries
jayaddison Dec 19, 2019
8d9dee4
Avoid duplicate {violations,incomplete} type definitions by merging i…
jayaddison Dec 19, 2019
eed7090
Extract violations and incomplete results separately
jayaddison Dec 19, 2019
857d03d
Restore binary scoring
jayaddison Dec 19, 2019
8a0b870
Fix linting error
jayaddison Dec 19, 2019
81f695f
Add test coverage on incomplete result handling
jayaddison Dec 19, 2019
745b397
Update incomplete, violations handling
jayaddison Dec 19, 2019
2edd6e4
Cleanup: Remove redundant array.concat
jayaddison Dec 19, 2019
fada34b
Add explanatory comment re: incomplete results
jayaddison Dec 19, 2019
589fd4e
Extract common augmentAxeNodes function
jayaddison Dec 19, 2019
879d84a
Extract common FailureCase result type
jayaddison Dec 19, 2019
b56fcd0
Add TODO pending correct error handling
jayaddison Dec 19, 2019
58fbf28
Nit: test string correction
jayaddison Dec 19, 2019
f0fb016
Add error-handling test case
jayaddison Dec 19, 2019
210b525
Update test to match type specifications
jayaddison Dec 19, 2019
d66fe31
Add RuleExecutionError type, child of FailureCase
jayaddison Dec 19, 2019
77c391c
Add error handling logic
jayaddison Dec 19, 2019
7b949eb
Restore automatic pass for no-error, no-nodes-found case
jayaddison Dec 19, 2019
acbfac0
Apply linting fixes
jayaddison Dec 19, 2019
1eb32eb
Apply 'not applicable' scoring to cases where 'incomplete' results fi…
jayaddison Dec 19, 2019
57e609c
Update logic commentary
jayaddison Dec 19, 2019
168d93f
Remove redundant code path
jayaddison Dec 20, 2019
cfc1aa3
Remove irrelevant test cases, descriptions and fields
jayaddison Dec 20, 2019
f0df4f8
Cleanup and comment update
jayaddison Dec 20, 2019
6c5a59e
Remove accidentally-committed devtoolslog file
jayaddison Dec 20, 2019
c792787
Only consider axe 'incomplete' results for informative audits
jayaddison Dec 20, 2019
42ba2a2
Add axe-core result type documentation references
jayaddison Dec 20, 2019
5cd5cab
Use 'informative' scoring mode for form-field-multiple-labels audit
jayaddison Dec 20, 2019
693dfea
Rephrase test descriptions
jayaddison Dec 20, 2019
bdc0893
Determine informative audit applicability based on axe passes
jayaddison Dec 20, 2019
e6d8bf6
Comment update
jayaddison Dec 20, 2019
2e0920d
Simplify logic: mark passing informative audits as not applicable
jayaddison Dec 20, 2019
1c42aba
Consolidate axe documentation references
jayaddison Dec 20, 2019
645daea
Rectify type naming for axe result sets
jayaddison Dec 20, 2019
4cd6031
Handle axe results containing both passes and incomplete matches
jayaddison Dec 20, 2019
7fef3c1
Move conditional check to ensure axe-core v3.3.0 backcompat
jayaddison Dec 20, 2019
fb4042c
Simplify logic; no need to collect, inspect 'passes' set
jayaddison Dec 21, 2019
3a6d1b9
Update LHR
jayaddison Dec 21, 2019
0faeed8
Clean up test case
jayaddison Dec 23, 2019
234b7dc
Rename: isIncomplete -> incompleteResult
jayaddison Dec 23, 2019
6ca94c7
Cleanup: remove duplicate 'incomplete' list access
jayaddison Dec 23, 2019
9782cbe
Add 'scoreDisplayMode' expectation
jayaddison Dec 23, 2019
35de51a
Merge branch 'master' into core-remove-form-field-multiple-labels
jayaddison Dec 23, 2019
e131519
Merge branch 'master' into core-remove-form-field-multiple-labels
jayaddison Jan 2, 2020
43b69f7
Cleanup: types
jayaddison Jan 6, 2020
2169144
Cleanup: further arrayification
jayaddison Jan 6, 2020
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
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,8 @@ const expectations = [
},
},
'form-field-multiple-labels': {
score: 0,
score: null,
jayaddison marked this conversation as resolved.
Show resolved Hide resolved
scoreDisplayMode: 'informative',
details: {
items: [
{
Expand All @@ -322,7 +323,6 @@ const expectations = [
'selector': '#form-field-multiple-labels',
'path': '2,HTML,1,BODY,35,SECTION,2,INPUT',
'snippet': '<input type="checkbox" id="form-field-multiple-labels">',
'explanation': 'Fix all of the following:\n Multiple label elements is not widely supported in assistive technologies',
'nodeLabel': 'input',
},
},
Expand Down
26 changes: 25 additions & 1 deletion lighthouse-core/audits/accessibility/axe-audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings);

class AxeAudit extends Audit {
/**
* Base class for audit rules which reflect assessment performed by the aXe accessibility library
* See https://github.com/dequelabs/axe-core/blob/6b444546cff492a62a70a74a8fc3c62bd4729400/doc/API.md#results-object for result type and format details
*
* @param {LH.Artifacts} artifacts Accessibility gatherer artifacts. Note that AxeAudit
* expects the meta name for the class to match the rule id from aXe.
* @return {LH.Audit.Product}
Expand All @@ -39,11 +42,32 @@ class AxeAudit extends Audit {
};
}

// Detect errors reported within aXe 'incomplete' results
// aXe uses this result type to indicate errors, or rules which require manual investigation
// If aXe reports an error, then bubble it up to the caller
const incomplete = artifacts.Accessibility.incomplete || [];
const incompleteResult = incomplete.find(result => result.id === this.meta.id);
if (incompleteResult && incompleteResult.error) {
return Audit.generateErrorAuditResult(this, incompleteResult.error.message);
}

const isInformative = this.meta.scoreDisplayMode === Audit.SCORING_MODES.INFORMATIVE;
const violations = artifacts.Accessibility.violations || [];
const rule = violations.find(result => result.id === this.meta.id);
const failureCases = isInformative ? violations.concat(incomplete) : violations;
const rule = failureCases.find(result => result.id === this.meta.id);
const impact = rule && rule.impact;
const tags = rule && rule.tags;

// Handle absence of aXe failure results for informative rules as 'not applicable'
jayaddison marked this conversation as resolved.
Show resolved Hide resolved
// This scenario indicates that no action is required by the web property owner
// Since there is no score impact from informative rules, display the rule as not applicable
if (isInformative && !rule) {
return {
score: 1,
notApplicable: true,
};
}

/** @type {LH.Audit.Details.Table['items']}>} */
let items = [];
if (rule && rule.nodes) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ class FormFieldMultipleLabels extends AxeAudit {
title: str_(UIStrings.title),
failureTitle: str_(UIStrings.failureTitle),
description: str_(UIStrings.description),
scoreDisplayMode: AxeAudit.SCORING_MODES.INFORMATIVE,
requiredArtifacts: ['Accessibility'],
};
}
Expand Down
34 changes: 22 additions & 12 deletions lighthouse-core/gather/gatherers/accessibility.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,21 +47,31 @@ function runA11yChecks() {
},
// @ts-ignore
}).then(axeResult => {
// Augment the node objects with outerHTML snippet & custom path string
// @ts-ignore
axeResult.violations.forEach(v => v.nodes.forEach(node => {
// @ts-ignore - getNodePath put into scope via stringification
node.path = getNodePath(node.element);
// @ts-ignore - getOuterHTMLSnippet put into scope via stringification
node.snippet = getOuterHTMLSnippet(node.element);
// @ts-ignore - getNodeLabel put into scope via stringification
node.nodeLabel = getNodeLabel(node.element);
// avoid circular JSON concerns
node.element = node.any = node.all = node.none = undefined;
}));
const augmentAxeNodes = result => {
// @ts-ignore
result.nodes.forEach(node => {
// @ts-ignore - getNodePath put into scope via stringification
node.path = getNodePath(node.element);
// @ts-ignore - getOuterHTMLSnippet put into scope via stringification
node.snippet = getOuterHTMLSnippet(node.element);
// @ts-ignore - getNodeLabel put into scope via stringification
node.nodeLabel = getNodeLabel(node.element);
// avoid circular JSON concerns
node.element = node.any = node.all = node.none = undefined;
});
};

// Augment the node objects with outerHTML snippet & custom path string
axeResult.violations.forEach(augmentAxeNodes);
axeResult.incomplete.forEach(augmentAxeNodes);

// We only need violations, and circular references are possible outside of violations
axeResult = {violations: axeResult.violations, notApplicable: axeResult.inapplicable};
axeResult = {
violations: axeResult.violations,
notApplicable: axeResult.inapplicable,
incomplete: axeResult.incomplete,
};
return axeResult;
});
}
Expand Down
108 changes: 108 additions & 0 deletions lighthouse-core/test/audits/accessibility/axe-audit-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,5 +35,113 @@ describe('Accessibility: axe-audit', () => {
const output = FakeA11yAudit.audit(artifacts);
assert.equal(output.score, 0);
});

it('returns axe error message to the caller when present', () => {
class FakeA11yAudit extends AxeAudit {
static get meta() {
return {
id: 'fake-incomplete-error',
title: 'Example title',
requiredArtifacts: ['Accessibility'],
};
}
}
const artifacts = {
Accessibility: {
incomplete: [{
id: 'fake-incomplete-error',
nodes: [],
help: 'http://example.com/',
error: {
name: 'SupportError',
message: 'Feature is not supported on your platform',
},
}],
},
};

const output = FakeA11yAudit.audit(artifacts);
assert.equal(output.errorMessage, 'Feature is not supported on your platform');
});

it('considers passing axe result as not applicable for informative audit', () => {
class FakeA11yAudit extends AxeAudit {
static get meta() {
return {
id: 'fake-axe-pass',
title: 'Example title',
scoreDisplayMode: 'informative',
requiredArtifacts: ['Accessibility'],
};
}
}
const artifacts = {
Accessibility: {
passes: [{
id: 'fake-axe-pass',
help: 'http://example.com/',
}],
},
};

const output = FakeA11yAudit.audit(artifacts);
assert.ok(output.notApplicable);
});

it('considers failing axe result as failure for informative audit', () => {
class FakeA11yAudit extends AxeAudit {
static get meta() {
return {
id: 'fake-axe-failure-case',
title: 'Example title',
scoreDisplayMode: 'informative',
requiredArtifacts: ['Accessibility'],
};
}
}
const artifacts = {
Accessibility: {
incomplete: [{
id: 'fake-axe-failure-case',
nodes: [{html: '<input id="multi-label-form-element" />'}],
help: 'http://example.com/',
}],
// TODO: remove: axe-core v3.3.0 backwards-compatibility test
violations: [{
id: 'fake-axe-failure-case',
nodes: [{html: '<input id="multi-label-form-element" />'}],
help: 'http://example.com/',
}],
},
};

const output = FakeA11yAudit.audit(artifacts);
assert.ok(!output.notApplicable);
assert.equal(output.score, 0);
});

it('considers error-free incomplete axe result as failure for informative audit', () => {
class FakeA11yAudit extends AxeAudit {
static get meta() {
return {
id: 'fake-incomplete-fail',
title: 'Example title',
scoreDisplayMode: 'informative',
requiredArtifacts: ['Accessibility'],
};
}
}
const artifacts = {
Accessibility: {
incomplete: [{
id: 'fake-incomplete-fail',
help: 'http://example.com/',
}],
},
};

const output = FakeA11yAudit.audit(artifacts);
assert.equal(output.score, 0);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ describe('CategoryRenderer', () => {
const categoryDOM = renderer.render(category, sampleResults.categoryGroups);

const gauge = categoryDOM.querySelector('.lh-gauge__percentage');
assert.equal(gauge.textContent.trim(), '66', 'score is 0-100');
assert.equal(gauge.textContent.trim(), '65', 'score is 0-100');

const score = categoryDOM.querySelector('.lh-category-header');
const value = categoryDOM.querySelector('.lh-gauge__percentage');
Expand Down
13 changes: 4 additions & 9 deletions lighthouse-core/test/results/sample_v2.json
Original file line number Diff line number Diff line change
Expand Up @@ -1801,13 +1801,8 @@
"id": "form-field-multiple-labels",
"title": "No form fields have multiple labels",
"description": "Form fields with multiple labels can be confusingly announced by assistive technologies like screen readers which use either the first, the last, or all of the labels. [Learn more](https://web.dev/form-field-multiple-labels/).",
"score": 1,
"scoreDisplayMode": "binary",
"details": {
"type": "table",
"headings": [],
"items": []
}
"score": null,
"scoreDisplayMode": "notApplicable"
},
"frame-title": {
"id": "frame-title",
Expand Down Expand Up @@ -3819,7 +3814,7 @@
},
{
"id": "form-field-multiple-labels",
"weight": 2,
"weight": 0,
"group": "a11y-names-labels"
},
{
Expand Down Expand Up @@ -3964,7 +3959,7 @@
}
],
"id": "accessibility",
"score": 0.66
"score": 0.65
},
"best-practices": {
"title": "Best Practices",
Expand Down
13 changes: 4 additions & 9 deletions proto/sample_v2_round_trip.json
Original file line number Diff line number Diff line change
Expand Up @@ -860,14 +860,9 @@
},
"form-field-multiple-labels": {
"description": "Form fields with multiple labels can be confusingly announced by assistive technologies like screen readers which use either the first, the last, or all of the labels. [Learn more](https://web.dev/form-field-multiple-labels/).",
"details": {
"headings": [],
"items": [],
"type": "table"
},
"id": "form-field-multiple-labels",
"score": 1.0,
"scoreDisplayMode": "binary",
"score": null,
"scoreDisplayMode": "notApplicable",
"title": "No form fields have multiple labels"
},
"frame-title": {
Expand Down Expand Up @@ -3443,7 +3438,7 @@
{
"group": "a11y-names-labels",
"id": "form-field-multiple-labels",
"weight": 2.0
"weight": 0.0
},
{
"group": "a11y-names-labels",
Expand Down Expand Up @@ -3589,7 +3584,7 @@
"description": "These checks highlight opportunities to [improve the accessibility of your web app](https://developers.google.com/web/fundamentals/accessibility). Only a subset of accessibility issues can be automatically detected so manual testing is also encouraged.",
"id": "accessibility",
"manualDescription": "These items address areas which an automated testing tool cannot cover. Learn more in our guide on [conducting an accessibility review](https://developers.google.com/web/fundamentals/accessibility/how-to-review).",
"score": 0.66,
"score": 0.65,
"title": "Accessibility"
},
"best-practices": {
Expand Down
39 changes: 23 additions & 16 deletions types/artifacts.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,23 +144,30 @@ declare global {
export type TaskNode = _TaskNode;
export type MetaElement = LH.Artifacts['MetaElements'][0];

export interface RuleExecutionError {
name: string;
message: string;
}

export interface AxeResult {
id: string;
impact: string;
tags: Array<string>;
nodes: Array<{
path: string;
html: string;
snippet: string;
target: Array<string>;
failureSummary?: string;
nodeLabel?: string;
}>;
error?: RuleExecutionError;
}

export interface Accessibility {
violations: {
id: string;
impact: string;
tags: string[];
nodes: {
path: string;
html: string;
snippet: string;
target: string[];
failureSummary?: string;
nodeLabel?: string;
}[];
}[];
notApplicable: {
id: string
}[];
violations: Array<AxeResult>;
notApplicable: Array<Pick<AxeResult, 'id'>>;
incomplete: Array<AxeResult>;
}

export interface CSSStyleSheetInfo {
Expand Down