Add function for getting all targets needed for spacing calculation

Siteimprove · Apr 8, 2024 · b7fd020 · b7fd020
1 parent 279ec82
commit b7fd020
Show file tree

Hide file tree

Showing 3 changed files with 84 additions and 76 deletions.
diff --git a/packages/alfa-rules/src/common/applicability/targets-of-pointer-events.ts b/packages/alfa-rules/src/common/applicability/targets-of-pointer-events.ts
@@ -1,38 +1,104 @@
 import { DOM } from "@siteimprove/alfa-aria";
 import { Cache } from "@siteimprove/alfa-cache";
 import { Device } from "@siteimprove/alfa-device";
-import { Document, Element, Node, Query } from "@siteimprove/alfa-dom";
+import { Document, Element, Node } from "@siteimprove/alfa-dom";
 import { Predicate } from "@siteimprove/alfa-predicate";
 import { Sequence } from "@siteimprove/alfa-sequence";
 import { Style } from "@siteimprove/alfa-style";
+import { Query } from "@siteimprove/alfa-dom";
 
 const { hasRole } = DOM;
-const { hasComputedStyle, isFocusable } = Style;
+const { hasComputedStyle, isFocusable, isVisible } = Style;
 
 const { and } = Predicate;
+
 const { getElementDescendants } = Query;
 
-const cache = Cache.empty<Document, Cache<Device, Sequence<Element>>>();
+const applicabilityCache = Cache.empty<
+  Document,
+  Cache<Device, Sequence<Element>>
+>();
 
 /**
  * @internal
  */
-export function targetsOfPointerEvents(
+export function applicableTargetsOfPointerEvents(
   document: Document,
   device: Device,
 ): Sequence<Element> {
-  return cache.get(document, Cache.empty).get(device, () =>
+  return applicabilityCache.get(document, Cache.empty).get(device, () => {
+    const isParagraph = hasRole(device, "paragraph");
+    const targetOfPointerEvent = and(
+      hasComputedStyle(
+        "pointer-events",
+        (keyword) => keyword.value !== "none",
+        device,
+      ),
+      isFocusable(device),
+      isVisible(device),
+      // TODO: Exclude <area> elements
+      hasRole(device, (role) => role.isWidget()),
+      hasBoundingBox(device),
+    );
+
+    function visit(node: Node, result: Array<Element> = []): Iterable<Element> {
+      if (Element.isElement(node)) {
+        if (isParagraph(node)) {
+          // If we encounter a paragraph, we can skip the entire subtree
+          return result;
+        }
+
+        // TODO: It's not enough to reject paragraphs, we need to reject all text blocks in order to avoid false positives
+
+        if (targetOfPointerEvent(node)) {
+          result.push(node);
+        }
+      }
+
+      for (const child of node.children(Node.fullTree)) {
+        visit(child, result);
+      }
+
+      return result;
+    }
+
+    return Sequence.from(visit(document));
+  });
+}
+
+const allTargetsCache = Cache.empty<
+  Document,
+  Cache<Device, Sequence<Element>>
+>();
+
+/**
+ * @internal
+ *
+ * @privateRemarks
+ * This function is not used in the applicability of R111 or R113,
+ * but in the expectation of R113 since all other targets are needed
+ * to determine if an applicable target is underspaced.
+ * It's kept here since it's closely related to the applicability.
+ */
+export function allTargetsOfPointerEvents(
+  document: Document,
+  device: Device,
+): Sequence<Element> {
+  return allTargetsCache.get(document, Cache.empty).get(device, () =>
     getElementDescendants(document, Node.fullTree).filter(
       and(
         hasComputedStyle(
           "pointer-events",
           (keyword) => keyword.value !== "none",
           device,
         ),
-        isFocusable(device),
         hasRole(device, (role) => role.isWidget()),
-        (target) => target.getBoundingBox(device).isSome(),
+        hasBoundingBox(device),
       ),
     ),
   );
 }
+
+function hasBoundingBox(device: Device): Predicate<Element> {
+  return (element) => element.getBoundingBox(device).isSome();
+}
diff --git a/packages/alfa-rules/src/sia-r111/rule.ts b/packages/alfa-rules/src/sia-r111/rule.ts
@@ -8,7 +8,7 @@ import { Page } from "@siteimprove/alfa-web";
 
 import { expectation } from "../common/act/expectation";
 
-import { targetsOfPointerEvents } from "../common/applicability/targets-of-pointer-events";
+import { applicableTargetsOfPointerEvents } from "../common/applicability/targets-of-pointer-events";
 
 import { WithBoundingBox, WithName } from "../common/diagnostic";
 
@@ -21,7 +21,7 @@ export default Rule.Atomic.of<Page, Element>({
   evaluate({ device, document }) {
     return {
       applicability() {
-        return targetsOfPointerEvents(document, device);
+        return applicableTargetsOfPointerEvents(document, device);
       },
 
       expectations(target) {

diff --git a/packages/alfa-rules/src/sia-r113/rule.ts b/packages/alfa-rules/src/sia-r113/rule.ts
@@ -1,79 +1,34 @@
 import { Rule } from "@siteimprove/alfa-act";
-import { DOM } from "@siteimprove/alfa-aria";
 import { Cache } from "@siteimprove/alfa-cache";
 import { Device } from "@siteimprove/alfa-device";
-import { Document, Element, Node } from "@siteimprove/alfa-dom";
+import { Document, Element } from "@siteimprove/alfa-dom";
 import { Either } from "@siteimprove/alfa-either";
 import { Iterable } from "@siteimprove/alfa-iterable";
 import { Rectangle } from "@siteimprove/alfa-rectangle";
 import { Err, Ok } from "@siteimprove/alfa-result";
 import { Sequence } from "@siteimprove/alfa-sequence";
 import { Criterion } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
-import { Predicate } from "@siteimprove/alfa-predicate";
-import { Style } from "@siteimprove/alfa-style";
 
 import { expectation } from "../common/act/expectation";
 
-import { targetsOfPointerEvents } from "../common/applicability/targets-of-pointer-events";
+import {
+  applicableTargetsOfPointerEvents,
+  allTargetsOfPointerEvents,
+} from "../common/applicability/targets-of-pointer-events";
 
 import { WithBoundingBox, WithName } from "../common/diagnostic";
 
 import { hasSufficientSize } from "../common/predicate/has-sufficient-size";
 import { isUserAgentControlled } from "../common/predicate/is-user-agent-controlled";
-import { hasName } from "@siteimprove/alfa-aria/src/role/predicate";
-
-const { and } = Predicate;
-const { hasComputedStyle, isFocusable, isVisible } = Style;
-const { hasRole } = DOM;
 
 export default Rule.Atomic.of<Page, Element>({
   uri: "https://alfa.siteimprove.com/rules/sia-r113",
   requirements: [Criterion.of("2.5.8")],
   evaluate({ device, document }) {
     return {
       applicability() {
-        // Strategy: Traverse tree and
-        // 1) reject subtrees that are text blocks, see sia-r62
-        // 2) collect targets of pointer events
-
-        const isParagraph = hasRole(device, "paragraph");
-        const targetOfPointerEvent = and(
-          hasComputedStyle(
-            "pointer-events",
-            (keyword) => keyword.value !== "none",
-            device,
-          ),
-          isFocusable(device),
-          isVisible(device),
-          hasRole(device, (role) => role.isWidget()),
-          (target) => target.getBoundingBox(device).isSome(),
-        );
-
-        let targets: Array<Element> = [];
-
-        function visit(node: Node): void {
-          if (Element.isElement(node)) {
-            if (isParagraph(node)) {
-              // If we encounter a paragraph, we can skip the entire subtree
-              return;
-            }
-
-            if (targetOfPointerEvent(node)) {
-              targets.push(node);
-            }
-          }
-
-          for (const child of node.children(Node.fullTree)) {
-            visit(child);
-          }
-        }
-
-        visit(document);
-
-        return Sequence.from(targets);
-
-        // return targetsOfPointerEvents(document, device);
+        return applicableTargetsOfPointerEvents(document, device);
       },
 
       expectations(target) {
@@ -170,11 +125,6 @@ export namespace Outcomes {
     );
 }
 
-const undersizedCache = Cache.empty<
-  Document,
-  Cache<Device, Sequence<Element>>
->();
-
 /**
  * Yields all elements that have insufficient spacing to the target.
  *
@@ -193,18 +143,10 @@ function* findElementsWithInsufficientSpacingToTarget(
   // Existence of a bounding box is guaranteed by applicability
   const targetRect = target.getBoundingBox(device).getUnsafe();
 
-  const undersizedTargets = undersizedCache
-    .get(document, Cache.empty)
-    .get(device, () =>
-      targetsOfPointerEvents(document, device).reject(
-        hasSufficientSize(24, device),
-      ),
-    );
-
   // TODO: This needs to be optimized, we should be able to use some spatial data structure like a quadtree to reduce the number of comparisons
-  for (const candidate of targetsOfPointerEvents(document, device)) {
+  for (const candidate of allTargetsOfPointerEvents(document, device)) {
     if (target !== candidate) {
-      // Existence of a bounding box is guaranteed by applicability
+      // Existence of a bounding box should be guaranteed by implementation of allTargetsOfPointerEvents
       const candidateRect = candidate.getBoundingBox(device).getUnsafe();
 
       if (
@@ -213,7 +155,7 @@ function* findElementsWithInsufficientSpacingToTarget(
           targetRect.center.y,
           12,
         ) ||
-        (undersizedTargets.includes(candidate) &&
+        (!hasSufficientSize(24, device)(candidate) &&
           targetRect.distanceSquared(candidateRect) < 24 ** 2)
       ) {
         // The 24px diameter circle of the target must not intersect with the bounding box of any other target, or