SIA-R111 and SIA-R113 applicability improvements

.changeset/slimy-socks-cry.md

@@ -0,0 +1,5 @@
+---
+"@siteimprove/alfa-rules": patch
+---
+
+**Fixed:** R113 is no longer applicable to invisible targets and targets inside a paragraph

packages/alfa-rules/src/common/applicability/targets-of-pointer-events.ts

@@ -1,38 +1,102 @@
 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 applicableTargetsOfPointerEvents(
+  document: Document,
+  device: Device,
+): Sequence<Element> {
+  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): Iterable<Element> {
+      if (Element.isElement(node)) {
+        if (isParagraph(node)) {
+          // If we encounter a paragraph, we can skip the entire subtree
+          return;
+        }
+
+        // TODO: It's not enough to reject paragraphs, we need to reject all text blocks in order to avoid false positives
+
+        if (targetOfPointerEvent(node)) {
+          yield node;
+        }
+      }
+
+      for (const child of node.children(Node.fullTree)) {
+        yield* visit(child);
+      }
+    }
+
+    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 targetsOfPointerEvents(
+export function allTargetsOfPointerEvents(
   document: Document,
   device: Device,
 ): Sequence<Element> {
-  return cache.get(document, Cache.empty).get(device, () =>
+  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();
+}

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) {

packages/alfa-rules/src/sia-r113/rule.ts

@@ -12,7 +12,10 @@ import { Page } from "@siteimprove/alfa-web";
 
 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";
 
@@ -25,7 +28,7 @@ export default Rule.Atomic.of<Page, Element>({
   evaluate({ device, document }) {
     return {
       applicability() {
-        return targetsOfPointerEvents(document, device);
+        return applicableTargetsOfPointerEvents(document, device);
       },
 
       expectations(target) {
@@ -126,7 +129,6 @@ const undersizedCache = Cache.empty<
   Document,
   Cache<Device, Sequence<Element>>
 >();
-
 /**
  * Yields all elements that have insufficient spacing to the target.
  *
@@ -148,15 +150,15 @@ function* findElementsWithInsufficientSpacingToTarget(
   const undersizedTargets = undersizedCache
     .get(document, Cache.empty)
     .get(device, () =>
-      targetsOfPointerEvents(document, device).reject(
+      allTargetsOfPointerEvents(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 (