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

a11y checks #815

Merged
merged 14 commits into from
Sep 5, 2017
2 changes: 1 addition & 1 deletion src/interfaces.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export interface CompileOptions {
cascade?: boolean;
hydratable?: boolean;
legacy?: boolean;
customElement: CustomElementOptions | true;
customElement?: CustomElementOptions | true;

onerror?: (error: Error) => void;
onwarn?: (warning: Warning) => void;
Expand Down
27 changes: 4 additions & 23 deletions src/validate/html/index.ts
Original file line number Diff line number Diff line change
@@ -1,41 +1,24 @@
import * as namespaces from '../../utils/namespaces';
import validateElement from './validateElement';
import validateWindow from './validateWindow';
import fuzzymatch from '../utils/fuzzymatch'
import flattenReference from '../../utils/flattenReference';
import { Validator } from '../index';
import { Node } from '../../interfaces';

const svg = /^(?:altGlyph|altGlyphDef|altGlyphItem|animate|animateColor|animateMotion|animateTransform|circle|clipPath|color-profile|cursor|defs|desc|discard|ellipse|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feDropShadow|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|filter|font|font-face|font-face-format|font-face-name|font-face-src|font-face-uri|foreignObject|g|glyph|glyphRef|hatch|hatchpath|hkern|image|line|linearGradient|marker|mask|mesh|meshgradient|meshpatch|meshrow|metadata|missing-glyph|mpath|path|pattern|polygon|polyline|radialGradient|rect|set|solidcolor|stop|switch|symbol|text|textPath|title|tref|tspan|unknown|use|view|vkern)$/;

const meta = new Map([[':Window', validateWindow]]);

export default function validateHtml(validator: Validator, html: Node) {
let elementDepth = 0;

const refs = new Map();
const refCallees: Node[] = [];
const elementStack: Node[] = [];

function visit(node: Node) {
if (node.type === 'Element') {
if (
elementDepth === 0 &&
validator.namespace !== namespaces.svg &&
svg.test(node.name)
) {
validator.warn(
`<${node.name}> is an SVG element – did you forget to add { namespace: 'svg' } ?`,
node.start
);
}

if (meta.has(node.name)) {
return meta.get(node.name)(validator, node, refs, refCallees);
}

elementDepth += 1;

validateElement(validator, node, refs, refCallees);
validateElement(validator, node, refs, refCallees, elementStack);
} else if (node.type === 'EachBlock') {
if (validator.helpers.has(node.context)) {
let c = node.expression.end;
Expand All @@ -53,16 +36,14 @@ export default function validateHtml(validator: Validator, html: Node) {
}

if (node.children) {
if (node.type === 'Element') elementStack.push(node);
node.children.forEach(visit);
if (node.type === 'Element') elementStack.pop();
}

if (node.else) {
visit(node.else);
}

if (node.type === 'Element') {
elementDepth -= 1;
}
}

html.children.forEach(visit);
Expand Down
45 changes: 44 additions & 1 deletion src/validate/html/validateElement.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,17 @@
import * as namespaces from '../../utils/namespaces';
import validateEventHandler from './validateEventHandler';
import { Validator } from '../index';
import { Node } from '../../interfaces';

export default function validateElement(validator: Validator, node: Node, refs: Map<string, Node[]>, refCallees: Node[]) {
const svg = /^(?:altGlyph|altGlyphDef|altGlyphItem|animate|animateColor|animateMotion|animateTransform|circle|clipPath|color-profile|cursor|defs|desc|discard|ellipse|feBlend|feColorMatrix|feComponentTransfer|feComposite|feConvolveMatrix|feDiffuseLighting|feDisplacementMap|feDistantLight|feDropShadow|feFlood|feFuncA|feFuncB|feFuncG|feFuncR|feGaussianBlur|feImage|feMerge|feMergeNode|feMorphology|feOffset|fePointLight|feSpecularLighting|feSpotLight|feTile|feTurbulence|filter|font|font-face|font-face-format|font-face-name|font-face-src|font-face-uri|foreignObject|g|glyph|glyphRef|hatch|hatchpath|hkern|image|line|linearGradient|marker|mask|mesh|meshgradient|meshpatch|meshrow|metadata|missing-glyph|mpath|path|pattern|polygon|polyline|radialGradient|rect|set|solidcolor|stop|switch|symbol|text|textPath|title|tref|tspan|unknown|use|view|vkern)$/;

export default function validateElement(
validator: Validator,
node: Node,
refs: Map<string, Node[]>,
refCallees: Node[],
elementStack: Node[]
) {
const isComponent =
node.name === ':Self' || validator.components.has(node.name);

Expand All @@ -11,6 +20,13 @@ export default function validateElement(validator: Validator, node: Node, refs:
validator.warn(`${node.name} component is not defined`, node.start);
}

if (elementStack.length === 0 && validator.namespace !== namespaces.svg && svg.test(node.name)) {
validator.warn(
`<${node.name}> is an SVG element – did you forget to add { namespace: 'svg' } ?`,
node.start
);
}

if (node.name === 'slot') {
const nameAttribute = node.attributes.find((attribute: Node) => attribute.name === 'name');
if (nameAttribute) {
Expand Down Expand Up @@ -44,6 +60,8 @@ export default function validateElement(validator: Validator, node: Node, refs:
let hasOutro: boolean;
let hasTransition: boolean;

const attributeMap: Map<string, Node> = new Map();

node.attributes.forEach((attribute: Node) => {
if (attribute.type === 'Ref') {
if (!refs.has(attribute.name)) refs.set(attribute.name, []);
Expand Down Expand Up @@ -161,6 +179,8 @@ export default function validateElement(validator: Validator, node: Node, refs:
);
}
} else if (attribute.type === 'Attribute') {
attributeMap.set(attribute.name, attribute);

if (attribute.name === 'value' && node.name === 'textarea') {
if (node.children.length) {
validator.error(
Expand All @@ -178,6 +198,29 @@ export default function validateElement(validator: Validator, node: Node, refs:
}
}
});

// a11y
if (node.name === 'a' && !attributeMap.has('href')) {
validator.warn(`A11y: <a> element should have an href attribute`, node.start);
}

if (node.name === 'img' && !attributeMap.has('alt')) {
validator.warn(`A11y: <img> element should have an alt attribute`, node.start);
}

if (node.name === 'figcaption') {
const parent = elementStack[elementStack.length - 1];
if (parent) {
if (parent.name !== 'figure') {
validator.warn(`A11y: <figcaption> must be an immediate child of <figure>`, node.start);
} else {
const index = parent.children.indexOf(node);
if (index !== 0 && index !== parent.children.length - 1) {
validator.warn(`A11y: <figcaption> must be first or last child of <figure>`, node.start);
}
}
}
}
}

function checkTypeAttribute(validator: Validator, node: Node) {
Expand Down
2 changes: 1 addition & 1 deletion test/validator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as fs from "fs";
import assert from "assert";
import { svelte, tryToLoadJson } from "../helpers.js";

describe("validate", () => {
describe.only("validate", () => {
Copy link

Choose a reason for hiding this comment

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

You have an only here

Copy link
Member Author

Choose a reason for hiding this comment

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

yeah, forgot to remove it before pushing — it's just to make my life easier while I'm working on it (as I don't need to configure tests to run individually, or wait for the entire suite). will make sure I remove it before removing the WIP 😀

fs.readdirSync("test/validator/samples").forEach(dir => {
if (dir[0] === ".") return;

Expand Down
1 change: 1 addition & 0 deletions test/validator/samples/a11y-a-without-href/input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<a>not actually a link</a>
8 changes: 8 additions & 0 deletions test/validator/samples/a11y-a-without-href/warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[{
"message": "A11y: <a> element should have an href attribute",
"loc": {
"line": 1,
"column": 0
},
"pos": 0
}]
19 changes: 19 additions & 0 deletions test/validator/samples/a11y-figcaption-wrong-place/input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<figure>
<img src='foo.jpg' alt='a picture of a foo'>

<figcaption>
a foo in its natural habitat
</figcaption>

<p>this should not be here</p>
</figure>

<figure>
<img src='foo.jpg' alt='a picture of a foo'>

<div class='markup-for-styling'>
<figcaption>
this element should be a child of the figure
</figcaption>
</div>
</figure>
18 changes: 18 additions & 0 deletions test/validator/samples/a11y-figcaption-wrong-place/warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
[
{
"message": "A11y: <figcaption> must be first or last child of <figure>",
"loc": {
"line": 4,
"column": 1
},
"pos": 57
},
{
"message": "A11y: <figcaption> must be an immediate child of <figure>",
"loc": {
"line": 15,
"column": 2
},
"pos": 252
}
]
1 change: 1 addition & 0 deletions test/validator/samples/a11y-img-without-alt/input.html
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
<img src='foo.jpg'>
8 changes: 8 additions & 0 deletions test/validator/samples/a11y-img-without-alt/warnings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[{
"message": "A11y: <img> element should have an alt attribute",
"loc": {
"line": 1,
"column": 0
},
"pos": 0
}]