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

Fix detection of empty titles #1573

Merged
merged 9 commits into from
Feb 14, 2024
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
5 changes: 5 additions & 0 deletions .changeset/sharp-chefs-glow.md
Original file line number Diff line number Diff line change
@@ -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).
2 changes: 2 additions & 0 deletions docs/review/api/alfa-string.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
11 changes: 6 additions & 5 deletions packages/alfa-dom/src/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
Expand All @@ -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(""));
}

/**
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/alfa-rules/src/common/predicate.ts
Original file line number Diff line number Diff line change
@@ -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";
4 changes: 0 additions & 4 deletions packages/alfa-rules/src/common/predicate/is-whitespace.ts

This file was deleted.

5 changes: 3 additions & 2 deletions packages/alfa-rules/src/sia-r1/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand All @@ -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>({
Expand Down Expand Up @@ -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,
),
Expand Down
3 changes: 1 addition & 2 deletions packages/alfa-rules/src/sia-r14/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -96,7 +95,7 @@ function getPerceivableInnerTextFromTextNode(

if (
and(not(isPerceivableForAll(device)), isRendered(device))(text) &&
isWhitespace(text.data)
String.isWhitespace(text.data, false)
) {
return " ";
}
Expand Down
8 changes: 3 additions & 5 deletions packages/alfa-rules/src/sia-r4/rule.ts
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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,
),
Expand Down
8 changes: 3 additions & 5 deletions packages/alfa-rules/src/sia-r5/rule.ts
Original file line number Diff line number Diff line change
@@ -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",
Expand All @@ -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())
);
Expand Down
8 changes: 3 additions & 5 deletions packages/alfa-rules/src/sia-r57/rule.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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;
Expand Down Expand Up @@ -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),
),
);
Expand Down
96 changes: 48 additions & 48 deletions packages/alfa-rules/src/sia-r62/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";

Expand Down Expand Up @@ -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()
Expand All @@ -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,
Expand Down Expand Up @@ -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)),
),
)
) {
Expand Down Expand Up @@ -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 = [
Expand Down
11 changes: 4 additions & 7 deletions packages/alfa-rules/src/sia-r7/rule.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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)) {
Expand Down
Loading