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

dequelabs · Jan 4, 2021 · 9903d7d · 9903d7d
1 parent fc6bbb2
commit 9903d7d
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 47 deletions.
diff --git a/lib/checks/aria/no-implicit-explicit-label-evaluate.js b/lib/checks/aria/no-implicit-explicit-label-evaluate.js
@@ -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;
diff --git a/lib/checks/label/explicit-evaluate.js b/lib/checks/label/explicit-evaluate.js
@@ -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)) {
@@ -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;
diff --git a/lib/checks/label/hidden-explicit-label-evaluate.js b/lib/checks/label/hidden-explicit-label-evaluate.js
@@ -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;
diff --git a/lib/checks/shared/svg-non-empty-title-evaluate.js b/lib/checks/shared/svg-non-empty-title-evaluate.js
@@ -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;