Fix detection of empty titles (#1573)

* Add breaking case * Flatten text content * Add isWhitespace predicate * Fix discarding of whitespace title * Use String.isWhitespace * Fix usage of hasTextContent * Add changeset * Clean up * Extract API --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Siteimprove · Feb 14, 2024 · e437385 · e437385
1 parent e449c31
commit e437385
Show file tree

Hide file tree

Showing 18 changed files with 124 additions and 96 deletions.
diff --git a/.changeset/sharp-chefs-glow.md b/.changeset/sharp-chefs-glow.md
@@ -0,0 +1,5 @@
+---
+"@siteimprove/alfa-rules": patch
+---
+
+**Fixed:** A regression on R1 for `<title>` with whitespace but not empty content has been fixed (was introduced in v0.73.0).
diff --git a/docs/review/api/alfa-string.api.md b/docs/review/api/alfa-string.api.md
@@ -9,7 +9,9 @@ type String_2 = globalThis.String;
 
 // @public (undocumented)
 namespace String_2 {
+    function flatten(input: string): string;
     function indent(input: string): string;
+    function isWhitespace(input: string, allowEmpty?: boolean): boolean;
     function normalize(input: string, toLowerCase?: boolean): string;
 }
 export { String_2 as String }

diff --git a/packages/alfa-dom/src/node.ts b/packages/alfa-dom/src/node.ts
@@ -29,16 +29,17 @@ import {
 
 import * as predicate from "./node/predicate";
 import * as traversal from "./node/traversal";
+import { String } from "@siteimprove/alfa-string";
 
 /**
  * @public
  */
 export abstract class Node<T extends string = string>
   extends tree.Node<Node.Traversal.Flag, T>
   implements
-    earl.Serializable<Node.EARL>,
-    json.Serializable<tree.Node.JSON<T>, Node.SerializationOptions>,
-    sarif.Serializable<sarif.Location>
+  earl.Serializable<Node.EARL>,
+  json.Serializable<tree.Node.JSON<T>, Node.SerializationOptions>,
+  sarif.Serializable<sarif.Location>
 {
   protected constructor(
     children: Array<Node>,
@@ -53,7 +54,7 @@ export abstract class Node<T extends string = string>
    * {@link https://dom.spec.whatwg.org/#concept-descendant-text-content}
    */
   public textContent(options: Node.Traversal = Node.Traversal.empty): string {
-    return this.descendants(options).filter(Text.isText).join("");
+    return String.flatten(this.descendants(options).filter(Text.isText).join(""));
   }
 
   /**
@@ -277,7 +278,7 @@ export interface Node {
  * @public
  */
 export namespace Node {
-  export interface JSON<T extends string = string> extends tree.Node.JSON<T> {}
+  export interface JSON<T extends string = string> extends tree.Node.JSON<T> { }
 
   export interface SerializationOptions {
     device: Device;

diff --git a/packages/alfa-rules/src/common/predicate.ts b/packages/alfa-rules/src/common/predicate.ts
@@ -1,4 +1,3 @@
 export * from "./predicate/is-large-text";
 export * from "./predicate/is-at-the-start";
-export * from "./predicate/is-whitespace";
 export * from "./predicate/reference-same-resource";
diff --git a/packages/alfa-rules/src/common/predicate/is-whitespace.ts b/packages/alfa-rules/src/common/predicate/is-whitespace.ts
diff --git a/packages/alfa-rules/src/sia-r1/rule.ts b/packages/alfa-rules/src/sia-r1/rule.ts
@@ -8,6 +8,7 @@ import {
 } from "@siteimprove/alfa-dom";
 import { Predicate } from "@siteimprove/alfa-predicate";
 import { Err, Ok } from "@siteimprove/alfa-result";
+import { String } from "@siteimprove/alfa-string";
 import { Criterion, Technique } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
 
@@ -18,7 +19,7 @@ import { Scope, Stability } from "../tags";
 
 const { hasName, hasNamespace } = Element;
 const { hasTextContent } = Node;
-const { and } = Predicate;
+const { and, not } = Predicate;
 const { getElementDescendants } = Query;
 
 export default Rule.Atomic.of<Page, Document>({
@@ -48,7 +49,7 @@ export default Rule.Atomic.of<Page, Document>({
           ),
 
           2: expectation(
-            title.some(hasTextContent()),
+            title.some(hasTextContent(not(String.isWhitespace))),
             () => Outcomes.HasNonEmptyTitle,
             () => Outcomes.HasEmptyTitle,
           ),

diff --git a/packages/alfa-rules/src/sia-r14/rule.ts b/packages/alfa-rules/src/sia-r14/rule.ts
@@ -12,7 +12,6 @@ import { Page } from "@siteimprove/alfa-web";
 
 import { expectation } from "../common/act/expectation";
 
-import { isWhitespace } from "../common/predicate";
 import { Scope, Stability } from "../tags";
 
 const { hasAccessibleName, hasRole, isPerceivableForAll } = DOM;
@@ -96,7 +95,7 @@ function getPerceivableInnerTextFromTextNode(
 
   if (
     and(not(isPerceivableForAll(device)), isRendered(device))(text) &&
-    isWhitespace(text.data)
+    String.isWhitespace(text.data, false)
   ) {
     return " ";
   }

diff --git a/packages/alfa-rules/src/sia-r4/rule.ts b/packages/alfa-rules/src/sia-r4/rule.ts
@@ -1,19 +1,17 @@
 import { Diagnostic, Rule } from "@siteimprove/alfa-act";
 import { Element } from "@siteimprove/alfa-dom";
-import { Iterable } from "@siteimprove/alfa-iterable";
 import { Predicate } from "@siteimprove/alfa-predicate";
 import { Err, Ok } from "@siteimprove/alfa-result";
+import { String } from "@siteimprove/alfa-string";
 import { Criterion, Technique } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
 
 import { expectation } from "../common/act/expectation";
 
-import { isWhitespace } from "../common/predicate";
 import { Scope, Stability } from "../tags";
 
 const { hasAttribute, isDocumentElement } = Element;
-const { isEmpty } = Iterable;
-const { nor } = Predicate;
+const { not } = Predicate;
 
 export default Rule.Atomic.of<Page, Element>({
   uri: "https://alfa.siteimprove.com/rules/sia-r4",
@@ -28,7 +26,7 @@ export default Rule.Atomic.of<Page, Element>({
       expectations(target) {
         return {
           1: expectation(
-            hasAttribute("lang", nor(isEmpty, isWhitespace))(target),
+            hasAttribute("lang", not(String.isWhitespace))(target),
             () => Outcomes.HasLanguage,
             () => Outcomes.HasNoLanguage,
           ),

diff --git a/packages/alfa-rules/src/sia-r5/rule.ts b/packages/alfa-rules/src/sia-r5/rule.ts
@@ -1,20 +1,18 @@
 import { Diagnostic, Rule } from "@siteimprove/alfa-act";
 import { Attribute, Element } from "@siteimprove/alfa-dom";
 import { Language } from "@siteimprove/alfa-iana";
-import { Iterable } from "@siteimprove/alfa-iterable";
 import { Predicate } from "@siteimprove/alfa-predicate";
 import { Err, Ok } from "@siteimprove/alfa-result";
+import { String } from "@siteimprove/alfa-string";
 import { Criterion, Technique } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
 
 import { expectation } from "../common/act/expectation";
 
-import { isWhitespace } from "../common/predicate";
 import { Scope, Stability } from "../tags";
 
 const { hasAttribute, isDocumentElement } = Element;
-const { isEmpty } = Iterable;
-const { nor } = Predicate;
+const { not } = Predicate;
 
 export default Rule.Atomic.of<Page, Attribute>({
   uri: "https://alfa.siteimprove.com/rules/sia-r5",
@@ -27,7 +25,7 @@ export default Rule.Atomic.of<Page, Attribute>({
           document
             .children()
             .filter(isDocumentElement)
-            .filter(hasAttribute("lang", nor(isEmpty, isWhitespace)))
+            .filter(hasAttribute("lang", not(String.isWhitespace)))
             // The previous filter ensures that lang exists
             .map((element) => element.attribute("lang").getUnsafe())
         );

diff --git a/packages/alfa-rules/src/sia-r57/rule.ts b/packages/alfa-rules/src/sia-r57/rule.ts
@@ -1,18 +1,17 @@
 import { Diagnostic, Rule } from "@siteimprove/alfa-act";
 import { DOM, Node } from "@siteimprove/alfa-aria";
 import { Element, Query, Text } from "@siteimprove/alfa-dom";
-import { Iterable } from "@siteimprove/alfa-iterable";
 import { Predicate } from "@siteimprove/alfa-predicate";
 import { Refinement } from "@siteimprove/alfa-refinement";
 import { Err, Ok } from "@siteimprove/alfa-result";
+import { String } from "@siteimprove/alfa-string";
 import { Style } from "@siteimprove/alfa-style";
 import { Page } from "@siteimprove/alfa-web";
 
 import * as dom from "@siteimprove/alfa-dom";
 
 import { expectation } from "../common/act/expectation";
 
-import { isWhitespace } from "../common/predicate";
 import { Scope, Stability } from "../tags";
 
 const {
@@ -21,8 +20,7 @@ const {
   isIncludedInTheAccessibilityTree,
 } = DOM;
 const { isElement } = Element;
-const { isEmpty } = Iterable;
-const { nor, not, or, property, test } = Predicate;
+const { not, or, property, test } = Predicate;
 const { and } = Refinement;
 const { isTabbable } = Style;
 const { isText } = Text;
@@ -53,7 +51,7 @@ export default Rule.Atomic.of<Page, Text>({
             .filter(isText)
             .filter(
               and(
-                property("data", nor(isEmpty, isWhitespace)),
+                property("data", not(String.isWhitespace)),
                 isIncludedInTheAccessibilityTree(device),
               ),
             );

diff --git a/packages/alfa-rules/src/sia-r62/rule.ts b/packages/alfa-rules/src/sia-r62/rule.ts
@@ -15,6 +15,7 @@ import { Err, Ok, Result } from "@siteimprove/alfa-result";
 import { Context } from "@siteimprove/alfa-selector";
 import { Sequence } from "@siteimprove/alfa-sequence";
 import { Set } from "@siteimprove/alfa-set";
+import { String } from "@siteimprove/alfa-string";
 import { Style } from "@siteimprove/alfa-style";
 import { Criterion } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
@@ -25,7 +26,6 @@ import { Contrast } from "../common/diagnostic/contrast";
 import { contrast } from "../common/expectation/contrast";
 
 import { getForeground } from "../common/dom/get-colors";
-import { isWhitespace } from "../common/predicate";
 
 import { Scope, Stability, Version } from "../tags";
 
@@ -230,25 +230,25 @@ export default Rule.Atomic.of<Page, Element>({
 
               return hasDistinguishableStyle
                 ? Ok.of(
-                    ElementDistinguishable.from(
-                      link,
-                      device,
-                      target,
-                      context,
-                      properties,
-                      distinguishableContrast,
-                    ),
-                  )
+                  ElementDistinguishable.from(
+                    link,
+                    device,
+                    target,
+                    context,
+                    properties,
+                    distinguishableContrast,
+                  ),
+                )
                 : Err.of(
-                    ElementDistinguishable.from(
-                      link,
-                      device,
-                      target,
-                      context,
-                      properties,
-                      distinguishableContrast,
-                    ),
-                  );
+                  ElementDistinguishable.from(
+                    link,
+                    device,
+                    target,
+                    context,
+                    properties,
+                    distinguishableContrast,
+                  ),
+                );
             }),
           )
             .toArray()
@@ -272,8 +272,8 @@ export default Rule.Atomic.of<Page, Element>({
             // If at least one link element is good, this is enough. The sorting
             // guarantees it is first in the array.
             isDefaultDistinguishable[0].isOk() &&
-              isHoverDistinguishable[0].isOk() &&
-              isFocusDistinguishable[0].isOk(),
+            isHoverDistinguishable[0].isOk() &&
+            isFocusDistinguishable[0].isOk(),
             () =>
               Outcomes.IsDistinguishable(
                 isDefaultDistinguishable,
@@ -354,7 +354,7 @@ function hasNonLinkText(device: Device): Predicate<Element> {
         children.some(
           and(
             isText,
-            and<Text>(isVisible(device), (text) => !isWhitespace(text.data)),
+            and<Text>(isVisible(device), (text) => !String.isWhitespace(text.data, false)),
           ),
         )
       ) {
@@ -387,32 +387,32 @@ namespace Distinguishable {
     let predicates: Array<
       readonly [ElementDistinguishable.Property, Predicate<Element>]
     > = [
-      // Things like text decoration and backgrounds risk blending with the
-      // container element. We therefore need to check if these can be distinguished
-      // from what the container element might itself set.
-      ["background", hasDistinguishableBackground(container, device, context)],
-      ["contrast", hasDistinguishableContrast(container, device, context)],
-      ["font", hasDistinguishableFont(container, device, context)],
-      [
-        "text-decoration",
-        hasDistinguishableTextDecoration(container, device, context),
-      ],
-      [
-        "vertical-align",
-        hasDistinguishableVerticalAlign(container, device, context),
-      ],
-      // We consider the mere presence of borders, box-shadows or outlines on the element as
-      // distinguishable features. There's of course a risk of these blending with
-      // other features of the container element, such as its background, but this
-      // should hopefully not happen (too often) in practice. When it does, we
-      // risk false negatives.
-      ["border", hasBorder(device, context)],
-      [
-        "box-shadow",
-        hasBoxShadow(device, context), //Checks for color != transparent and spread => 0
-      ],
-      ["outline", hasOutline(device, context)],
-    ];
+        // Things like text decoration and backgrounds risk blending with the
+        // container element. We therefore need to check if these can be distinguished
+        // from what the container element might itself set.
+        ["background", hasDistinguishableBackground(container, device, context)],
+        ["contrast", hasDistinguishableContrast(container, device, context)],
+        ["font", hasDistinguishableFont(container, device, context)],
+        [
+          "text-decoration",
+          hasDistinguishableTextDecoration(container, device, context),
+        ],
+        [
+          "vertical-align",
+          hasDistinguishableVerticalAlign(container, device, context),
+        ],
+        // We consider the mere presence of borders, box-shadows or outlines on the element as
+        // distinguishable features. There's of course a risk of these blending with
+        // other features of the container element, such as its background, but this
+        // should hopefully not happen (too often) in practice. When it does, we
+        // risk false negatives.
+        ["border", hasBorder(device, context)],
+        [
+          "box-shadow",
+          hasBoxShadow(device, context), //Checks for color != transparent and spread => 0
+        ],
+        ["outline", hasOutline(device, context)],
+      ];
 
     if (context.isHovered(target)) {
       predicates = [

diff --git a/packages/alfa-rules/src/sia-r7/rule.ts b/packages/alfa-rules/src/sia-r7/rule.ts
@@ -15,16 +15,16 @@ import { Predicate } from "@siteimprove/alfa-predicate";
 import { Refinement } from "@siteimprove/alfa-refinement";
 import { Err, Ok } from "@siteimprove/alfa-result";
 import { Sequence } from "@siteimprove/alfa-sequence";
+import { String } from "@siteimprove/alfa-string";
 import { Style } from "@siteimprove/alfa-style";
 import { Criterion, Technique } from "@siteimprove/alfa-wcag";
 import { Page } from "@siteimprove/alfa-web";
 
 import { expectation } from "../common/act/expectation";
 
-import { isWhitespace } from "../common/predicate";
 import { Scope, Stability } from "../tags";
 
-const { hasAccessibleName, isIncludedInTheAccessibilityTree } = DOM;
+const { hasNonEmptyAccessibleName, isIncludedInTheAccessibilityTree } = DOM;
 const { hasAttribute, hasName, hasNamespace, isElement } = Element;
 const { isEmpty } = Iterable;
 const { not, or } = Predicate;
@@ -52,16 +52,13 @@ export default Rule.Atomic.of<Page, Attribute>({
               isText,
               and(
                 or(isVisible(device), isIncludedInTheAccessibilityTree(device)),
-                (text: Text) => !isWhitespace(text.data),
+                (text: Text) => !String.isWhitespace(text.data, false),
               ),
             );
 
             const isElementWithAccessibleName = and(
               isElement,
-              hasAccessibleName(
-                device,
-                (accessibleName) => !isWhitespace(accessibleName.value),
-              ),
+              hasNonEmptyAccessibleName(device),
             );
 
             if (test(or(isVisibleText, isElementWithAccessibleName), node)) {