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

chore: don't be so greedy in virtual checks with try/catch #2719

Merged
merged 1 commit into from
Jan 4, 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
30 changes: 16 additions & 14 deletions lib/checks/aria/no-implicit-explicit-label-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,26 +27,28 @@ function noImplicitExplicitLabelEvaluate(node, options, virtualNode) {
const role = getRole(virtualNode, { noImplicit: true });
this.data(role);

let label;
let accText;
try {
const label = sanitize(labelText(virtualNode)).toLowerCase();
const accText = sanitize(accessibleTextVirtual(virtualNode)).toLowerCase();

if (!accText && !label) {
return false;
}
label = sanitize(labelText(virtualNode)).toLowerCase();
accText = sanitize(accessibleTextVirtual(virtualNode)).toLowerCase();
} catch (e) {
return undefined;
}

if (!accText && label) {
return undefined;
}
if (!accText && !label) {
return false;
}

if (!accText.includes(label)) {
return undefined;
}
if (!accText && label) {
return undefined;
}

return false;
} catch (e) {
if (!accText.includes(label)) {
return undefined;
}

return false;
}

export default noImplicitExplicitLabelEvaluate;
23 changes: 14 additions & 9 deletions lib/checks/label/explicit-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,17 @@ import { accessibleText } from '../../commons/text';
import { escapeSelector } from '../../core/utils';

function explicitEvaluate(node, options, virtualNode) {
try {
if (virtualNode.attr('id')) {
const root = getRootNode(virtualNode.actualNode);
const id = escapeSelector(virtualNode.attr('id'));
const labels = Array.from(root.querySelectorAll(`label[for="${id}"]`));
if (virtualNode.attr('id')) {
if (!virtualNode.actualNode) {
return undefined;
}

const root = getRootNode(virtualNode.actualNode);
const id = escapeSelector(virtualNode.attr('id'));
const labels = Array.from(root.querySelectorAll(`label[for="${id}"]`));

if (labels.length) {
if (labels.length) {
try {
return labels.some(label => {
// defer to hidden-explicit-label check for better messaging
if (!isVisible(label)) {
Expand All @@ -18,12 +22,13 @@ function explicitEvaluate(node, options, virtualNode) {
return !!accessibleText(label);
}
});
} catch (e) {
return undefined;
}
}
return false;
} catch (e) {
return undefined;
}

return false;
}

export default explicitEvaluate;
30 changes: 18 additions & 12 deletions lib/checks/label/hidden-explicit-label-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,22 +3,28 @@ import { accessibleTextVirtual } from '../../commons/text';
import { escapeSelector } from '../../core/utils';

function hiddenExplicitLabelEvaluate(node, options, virtualNode) {
try {
if (virtualNode.hasAttr('id')) {
const root = getRootNode(node);
const id = escapeSelector(node.getAttribute('id'));
const label = root.querySelector(`label[for="${id}"]`);
if (virtualNode.hasAttr('id')) {
if (!virtualNode.actualNode) {
return undefined;
}

const root = getRootNode(node);
const id = escapeSelector(node.getAttribute('id'));
const label = root.querySelector(`label[for="${id}"]`);

if (label && !isVisible(label, true)) {
const name = accessibleTextVirtual(virtualNode).trim();
const isNameEmpty = name === '';
return isNameEmpty;
if (label && !isVisible(label, true)) {
let name;
try {
name = accessibleTextVirtual(virtualNode).trim();
} catch (e) {
return undefined;
}

const isNameEmpty = name === '';
return isNameEmpty;
}
return false;
} catch (e) {
return undefined;
}
return false;
}

export default hiddenExplicitLabelEvaluate;
28 changes: 16 additions & 12 deletions lib/checks/shared/svg-non-empty-title-evaluate.js
Original file line number Diff line number Diff line change
@@ -1,29 +1,33 @@
import visibleVirtual from '../../commons/text/visible-virtual';

function svgNonEmptyTitleEvaluate(node, options, virtualNode) {
try {
const titleNode = virtualNode.children.find(({ props }) => {
return props.nodeName === 'title';
});
if (!virtualNode.children) {
return undefined;
}

if (!titleNode) {
this.data({
messageKey: 'noTitle'
});
return false;
}
const titleNode = virtualNode.children.find(({ props }) => {
return props.nodeName === 'title';
});

if (!titleNode) {
this.data({
messageKey: 'noTitle'
});
return false;
}

try {
if (visibleVirtual(titleNode) === '') {
this.data({
messageKey: 'emptyTitle'
});
return false;
}

return true;
} catch (e) {
return undefined;
}

return true;
}

export default svgNonEmptyTitleEvaluate;