Skip to content

Commit

Permalink
fix(checks): do not normalize options for custom checks (#2435)
Browse files Browse the repository at this point in the history
  • Loading branch information
straker authored Aug 4, 2020
1 parent b315904 commit 83056ad
Show file tree
Hide file tree
Showing 6 changed files with 134 additions and 24 deletions.
2 changes: 0 additions & 2 deletions lib/core/base/audit.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import Rule from './rule';
import Check from './check';
import standards from '../../standards';
import metadataFunctionMap from './metadata-function-map';
import RuleResult from './rule-result';
import {
clone,
Expand Down Expand Up @@ -157,7 +156,6 @@ class Audit {
this.lang = 'en';
this.defaultConfig = audit;
this.standards = standards;
this.metadataFunctionMap = metadataFunctionMap;
this._init();
// A copy of the "default" locale. This will be set if the user
// provides a new locale to `axe.configure()` and used to undo
Expand Down
22 changes: 19 additions & 3 deletions lib/core/base/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,21 +180,37 @@ Check.prototype.runSync = function(node, options, context) {
*/

Check.prototype.configure = function(spec) {
// allow test specs (without evaluate functions) to work as
// internal checks
if (!spec.evaluate || metadataFunctionMap[spec.evaluate]) {
this._internalCheck = true;
}

if (spec.hasOwnProperty('enabled')) {
this.enabled = spec.enabled;
}

if (spec.hasOwnProperty('options')) {
this.options = normalizeOptions(spec.options);
// only normalize options for internal checks
if (this._internalCheck) {
this.options = normalizeOptions(spec.options);
} else {
this.options = spec.options;
}
}

['evaluate', 'after']
.filter(prop => spec.hasOwnProperty(prop))
.forEach(prop => (this[prop] = createExecutionContext(spec[prop])));
};

Check.prototype.getOptions = function getOptions(options = {}) {
return deepMerge(this.options, normalizeOptions(options));
Check.prototype.getOptions = function getOptions(options) {
// only merge and normalize options for internal checks
if (this._internalCheck) {
return deepMerge(this.options, normalizeOptions(options || {}));
} else {
return options || this.options;
}
};

export default Check;
4 changes: 3 additions & 1 deletion lib/core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Audit from './base/audit';
import CheckResult from './base/check-result';
import Check from './base/check';
import Context from './base/context';
import metadataFunctionMap from './base/metadata-function-map';
import RuleResult from './base/rule-result';
import Rule from './base/rule';

Expand Down Expand Up @@ -53,7 +54,8 @@ axe._thisWillBeDeletedDoNotUse.base = {
Check,
Context,
RuleResult,
Rule
Rule,
metadataFunctionMap
};

axe.imports = imports;
Expand Down
54 changes: 42 additions & 12 deletions test/core/base/check.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ describe('Check', function() {

var Check = axe._thisWillBeDeletedDoNotUse.base.Check;
var CheckResult = axe._thisWillBeDeletedDoNotUse.base.CheckResult;
var metadataFunctionMap =
axe._thisWillBeDeletedDoNotUse.base.metadataFunctionMap;
var noop = function() {};

var fixture = document.getElementById('fixture');
Expand Down Expand Up @@ -99,8 +101,7 @@ describe('Check', function() {
delete Check.prototype.test;
});
it('should override evaluate as ID', function() {
axe._load({});
axe._audit.metadataFunctionMap['custom-evaluate'] = function() {
metadataFunctionMap['custom-evaluate'] = function() {
return 'fong';
};

Expand All @@ -113,11 +114,10 @@ describe('Check', function() {
check.configure({ evaluate: 'custom-evaluate' });
assert.equal('fong', check.test());
delete Check.prototype.test;
delete axe._audit.metadataFunctionMap['custom-evaluate'];
delete metadataFunctionMap['custom-evaluate'];
});
it('should override after as ID', function() {
axe._load({});
axe._audit.metadataFunctionMap['custom-after'] = function() {
metadataFunctionMap['custom-after'] = function() {
return 'fong';
};

Expand All @@ -130,7 +130,7 @@ describe('Check', function() {
check.configure({ after: 'custom-after' });
assert.equal('fong', check.test());
delete Check.prototype.test;
delete axe._audit.metadataFunctionMap['custom-after'];
delete metadataFunctionMap['custom-after'];
});
it('should error if evaluate does not match an ID', function() {
function fn() {
Expand Down Expand Up @@ -226,10 +226,21 @@ describe('Check', function() {
}).run(fixture, { options: expected }, {}, noop);
});

it('should normalize non-object options', function(done) {
it('should normalize non-object options for internal checks', function(done) {
metadataFunctionMap['custom-check'] = function(node, options) {
assert.deepEqual(options, { value: 'foo' });
done();
};
new Check({
evaluate: 'custom-check'
}).run(fixture, { options: 'foo' }, {}, noop);
delete metadataFunctionMap['custom-check'];
});

it('should not normalize non-object options for external checks', function(done) {
new Check({
evaluate: function(node, options) {
assert.deepEqual(options, { value: 'foo' });
assert.deepEqual(options, 'foo');
done();
}
}).run(fixture, { options: 'foo' }, {}, noop);
Expand Down Expand Up @@ -388,13 +399,24 @@ describe('Check', function() {
}).runSync(fixture, { options: expected }, {});
});

it('should normalize non-object options', function(done) {
it('should normalize non-object options for internal checks', function(done) {
metadataFunctionMap['custom-check'] = function(node, options) {
assert.deepEqual(options, { value: 'foo' });
done();
};
new Check({
evaluate: 'custom-check'
}).runSync(fixture, { options: 'foo' }, {});
delete metadataFunctionMap['custom-check'];
});

it('should not normalize non-object options for external checks', function(done) {
new Check({
evaluate: function(node, options) {
assert.deepEqual(options, { value: 'foo' });
assert.deepEqual(options, 'foo');
done();
}
}).run(fixture, { options: 'foo' }, {}, noop);
}).runSync(fixture, { options: 'foo' }, {});
});

it('should pass the context through to check evaluate call', function() {
Expand Down Expand Up @@ -569,12 +591,20 @@ describe('Check', function() {
assert.equal(new Check(spec).options, spec.options);
});

it('should normalize non-object options', function() {
it('should normalize non-object options for internal checks', function() {
var spec = {
options: 'foo'
};
assert.deepEqual(new Check(spec).options, { value: 'foo' });
});

it('should not normalize non-object options for external checks', function() {
var spec = {
options: 'foo',
evaluate: function() {}
};
assert.deepEqual(new Check(spec).options, 'foo');
});
});

describe('.evaluate', function() {
Expand Down
10 changes: 6 additions & 4 deletions test/core/base/rule.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ describe('Rule', function() {

var Rule = axe._thisWillBeDeletedDoNotUse.base.Rule;
var Check = axe._thisWillBeDeletedDoNotUse.base.Check;
var metadataFunctionMap =
axe._thisWillBeDeletedDoNotUse.base.metadataFunctionMap;
var fixture = document.getElementById('fixture');
var noop = function() {};
var isNotCalled = function(err) {
Expand Down Expand Up @@ -1940,10 +1942,10 @@ describe('Rule', function() {
});
it('should override matches (metadata function name)', function() {
axe._load({});
axe._audit.metadataFunctionMap['custom-matches'] = function() {
metadataFunctionMap['custom-matches'] = function() {
return 'custom-matches';
};
axe._audit.metadataFunctionMap['other-matches'] = function() {
metadataFunctionMap['other-matches'] = function() {
return 'other-matches';
};

Expand All @@ -1953,8 +1955,8 @@ describe('Rule', function() {
rule.configure({ matches: 'other-matches' });
assert.equal(rule._get('matches')(), 'other-matches');

delete axe._audit.metadataFunctionMap['custom-matches'];
delete axe._audit.metadataFunctionMap['other-matches'];
delete metadataFunctionMap['custom-matches'];
delete metadataFunctionMap['other-matches'];
});
it('should error if matches does not match an ID', function() {
function fn() {
Expand Down
66 changes: 64 additions & 2 deletions test/integration/full/configure-options/configure-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,68 @@ describe('Configure Options', function() {
}
);
});

it('should not normalize external check options', function(done) {
target.setAttribute('lang', 'en');

axe.configure({
checks: [
{
id: 'dylang',
options: ['dylan'],
evaluate:
'function (node, options) {\n var lang = (node.getAttribute("lang") || "").trim().toLowerCase();\n var xmlLang = (node.getAttribute("xml:lang") || "").trim().toLowerCase();\n var invalid = [];\n (options || []).forEach(function(cc) {\n cc = cc.toLowerCase();\n if (lang && (lang === cc || lang.indexOf(cc.toLowerCase() + "-") === 0)) {\n lang = null;\n }\n if (xmlLang && (xmlLang === cc || xmlLang.indexOf(cc.toLowerCase() + "-") === 0)) {\n xmlLang = null;\n }\n });\n if (xmlLang) {\n invalid.push(\'xml:lang="\' + xmlLang + \'"\');\n }\n if (lang) {\n invalid.push(\'lang="\' + lang + \'"\');\n }\n if (invalid.length) {\n this.data(invalid);\n return true;\n }\n return false;\n }',
messages: {
pass: 'Good language',
fail: 'You mst use the DYLAN language'
}
}
],
rules: [
{
id: 'dylang',
metadata: {
description:
"Ensures lang attributes have the value of 'dylan'",
help: "lang attribute must have the value of 'dylan'"
},
selector: '#target',
any: [],
all: [],
none: ['dylang'],
tags: ['wcag2aa']
}
],
data: {
rules: {
dylang: {
description:
"Ensures lang attributes have the value of 'dylan'",
help: "lang attribute must have the value of 'dylan'"
}
}
}
});

axe.run(
'#target',
{
runOnly: {
type: 'rule',
values: ['dylang']
}
},
function(err, results) {
try {
assert.isNull(err);
assert.lengthOf(results.violations, 1, 'violations');
done();
} catch (e) {
done(e);
}
}
);
});
});

describe('aria-required-attr', function() {
Expand Down Expand Up @@ -67,8 +129,8 @@ describe('Configure Options', function() {
});
});

describe('disableOtherRules', function(done) {
it('disables rules that are not in the `rules` array', function() {
describe('disableOtherRules', function() {
it('disables rules that are not in the `rules` array', function(done) {
axe.configure({
disableOtherRules: true,
rules: [
Expand Down

0 comments on commit 83056ad

Please sign in to comment.