From a0b4449797881796a9f8afe86984556a602df819 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 09:22:15 +0100 Subject: [PATCH 01/22] Clean up unused imports --- packages/alfa-cascade/src/selector-map.ts | 6 ------ 1 file changed, 6 deletions(-) diff --git a/packages/alfa-cascade/src/selector-map.ts b/packages/alfa-cascade/src/selector-map.ts index 5cf302e62d..27372e6b6a 100644 --- a/packages/alfa-cascade/src/selector-map.ts +++ b/packages/alfa-cascade/src/selector-map.ts @@ -16,15 +16,12 @@ import { Option } from "@siteimprove/alfa-option"; import { Predicate } from "@siteimprove/alfa-predicate"; import { Refinement } from "@siteimprove/alfa-refinement"; import { - Attribute, Class, Combinator, Complex, Compound, Context, Id, - PseudoClass, - PseudoElement, Selector, Type, } from "@siteimprove/alfa-selector"; @@ -37,13 +34,10 @@ import { AncestorFilter } from "./ancestor-filter"; const { equals, property } = Predicate; const { and } = Refinement; -const { isAttribute } = Attribute; const { isClass } = Class; const { isComplex } = Complex; const { isCompound } = Compound; const { isId } = Id; -const { isPseudoClass } = PseudoClass; -const { isPseudoElement } = PseudoElement; const { isType } = Type; const isDescendantSelector = and( From 6f877bb05360d3cc87f3fa9744e85fe25051bb0b Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 13:26:17 +0100 Subject: [PATCH 02/22] Add tests for ancestor filters --- packages/alfa-cascade/package.json | 3 + packages/alfa-cascade/src/ancestor-filter.ts | 48 ++- .../test/ancestor-filter.spec.tsx | 308 ++++++++++++++++++ packages/alfa-cascade/tsconfig.json | 10 +- 4 files changed, 361 insertions(+), 8 deletions(-) create mode 100644 packages/alfa-cascade/test/ancestor-filter.spec.tsx diff --git a/packages/alfa-cascade/package.json b/packages/alfa-cascade/package.json index 98ca25552e..6a58884aec 100644 --- a/packages/alfa-cascade/package.json +++ b/packages/alfa-cascade/package.json @@ -31,6 +31,9 @@ "@siteimprove/alfa-refinement": "workspace:^0.69.0", "@siteimprove/alfa-selector": "workspace:^0.69.0" }, + "devDependencies": { + "@siteimprove/alfa-test": "workspace:^0.69.0" + }, "publishConfig": { "access": "public", "registry": "https://npm.pkg.github.com/" diff --git a/packages/alfa-cascade/src/ancestor-filter.ts b/packages/alfa-cascade/src/ancestor-filter.ts index 19125a0693..4ea1105bcc 100644 --- a/packages/alfa-cascade/src/ancestor-filter.ts +++ b/packages/alfa-cascade/src/ancestor-filter.ts @@ -1,6 +1,9 @@ import { Element } from "@siteimprove/alfa-dom"; +import { Serializable } from "@siteimprove/alfa-json"; import { Class, Id, Selector, Type } from "@siteimprove/alfa-selector"; +import * as json from "@siteimprove/alfa-json"; + /** * The ancestor filter is a data structure used for optimising selector matching * in the case of descendant selectors. When traversing down through the DOM @@ -18,7 +21,7 @@ import { Class, Id, Selector, Type } from "@siteimprove/alfa-selector"; * type, in order to evict these from the filter when the last element with a * given ID, class, or type is removed from the filter. * - * For example, consider the following tree: + * For example, consider the following DOM tree: * * section#content * +-- blockquote @@ -43,7 +46,7 @@ import { Class, Id, Selector, Type } from "@siteimprove/alfa-selector"; * * @internal */ -export class AncestorFilter { +export class AncestorFilter implements Serializable { public static empty(): AncestorFilter { return new AncestorFilter(); } @@ -79,20 +82,40 @@ export class AncestorFilter { } public matches(selector: Selector): boolean { - if (selector instanceof Id) { + if (Id.isId(selector)) { return this._ids.has(selector.name); } - if (selector instanceof Class) { + if (Class.isClass(selector)) { return this._classes.has(selector.name); } - if (selector instanceof Type) { + if (Type.isType(selector)) { return this._types.has(selector.name); } return false; } + + public toJSON(): AncestorFilter.JSON { + return { + ids: this._ids.toJSON(), + classes: this._classes.toJSON(), + types: this._types.toJSON(), + }; + } +} + +/** + * @internal + */ +export namespace AncestorFilter { + export interface JSON { + [key: string]: json.JSON; + ids: Bucket.JSON; + classes: Bucket.JSON; + types: Bucket.JSON; + } } /** @@ -106,8 +129,10 @@ export class AncestorFilter { * as we only ever compute cascade once for every context, and native maps are * actually much faster than any bloom filter we might be able to cook up in * plain JavaScript. + * + * @internal */ -class Bucket { +export class Bucket implements Serializable { public static empty(): Bucket { return new Bucket(); } @@ -143,4 +168,15 @@ class Bucket { this._entries.set(entry, count - 1); } } + + public toJSON(): Bucket.JSON { + return [...this._entries]; + } +} + +/** + * @internal + */ +export namespace Bucket { + export type JSON = Array<[string, number]>; } diff --git a/packages/alfa-cascade/test/ancestor-filter.spec.tsx b/packages/alfa-cascade/test/ancestor-filter.spec.tsx new file mode 100644 index 0000000000..f80a916fc6 --- /dev/null +++ b/packages/alfa-cascade/test/ancestor-filter.spec.tsx @@ -0,0 +1,308 @@ +import { Lexer } from "@siteimprove/alfa-css"; +import { Selector } from "@siteimprove/alfa-selector"; +import { Assertions, test } from "@siteimprove/alfa-test"; + +import { AncestorFilter, Bucket } from "../src/ancestor-filter"; + +test("Buckets behave as expected", (t) => { + const bucket = Bucket.empty(); + + t.deepEqual(bucket.toJSON(), []); + t(!bucket.has("a")); + t(!bucket.has("b")); + + bucket.add("a"); + t.deepEqual(bucket.toJSON(), [["a", 1]]); + t(bucket.has("a")); + t(!bucket.has("b")); + + bucket.add("b"); + t.deepEqual(bucket.toJSON(), [ + ["a", 1], + ["b", 1], + ]); + t(bucket.has("a")); + t(bucket.has("b")); + + bucket.add("a"); + t.deepEqual(bucket.toJSON(), [ + ["a", 2], + ["b", 1], + ]); + t(bucket.has("a")); + t(bucket.has("b")); + + bucket.add("a"); + t.deepEqual(bucket.toJSON(), [ + ["a", 3], + ["b", 1], + ]); + t(bucket.has("a")); + t(bucket.has("b")); + + bucket.remove("a"); + t.deepEqual(bucket.toJSON(), [ + ["a", 2], + ["b", 1], + ]); + t(bucket.has("a")); + t(bucket.has("b")); + + bucket.remove("a"); + t.deepEqual(bucket.toJSON(), [ + ["a", 1], + ["b", 1], + ]); + t(bucket.has("a")); + t(bucket.has("b")); + + bucket.remove("b"); + t.deepEqual(bucket.toJSON(), [["a", 1]]); + t(bucket.has("a")); + t(!bucket.has("b")); + + bucket.remove("a"); + t.deepEqual(bucket.toJSON(), []); + t(!bucket.has("a")); + t(!bucket.has("b")); + + bucket.remove("a"); + t.deepEqual(bucket.toJSON(), []); + t(!bucket.has("a")); + t(!bucket.has("b")); +}); + +function parse(selector: string): Selector { + return Selector.parse(Lexer.lex(selector)).getUnsafe()[1]; +} + +const selectors = { + divSel: parse("div"), + spanSel: parse("span"), + dotFooSel: parse(".foo"), + dotBarSel: parse(".bar"), + hashFooSel: parse("#foo"), + hashBarSel: parse("#bar"), +}; + +function match( + t: Assertions, + filter: AncestorFilter, + expected: AncestorFilter.JSON, + matching: Array, +) { + t.deepEqual(filter.toJSON(), expected); + for (const sel of [ + "divSel", + "spanSel", + "dotFooSel", + "dotBarSel", + "hashFooSel", + "hashBarSel", + ] as const) { + t.equal(filter.matches(selectors[sel]), matching.includes(sel)); + } +} + +/** + * In this test, we are walking the following DOM tree: + * div + * +-- div.foo + * | +-- div + * | | +-- span#foo + * | | +-- span.foo + * | | +-- span.bar + * +-- div + * | +-- span#bar + * +-- div.bar + * | +-- span.foo + * | +-- span.bar + * span + * + * and check the composition of the ancestor filter at each step. + * Since only element's name, id and class matter (not the actual element), we + * create them on the fly and remove a structurally identical one rather than the + * exact one that has been added. This simplifies the test a bit. Actual cascading + * walk through the actual DOM tree and remove the exact same element when moving up. + */ +test("Ancestor filter behaves as expected", (t) => { + const filter = AncestorFilter.empty(); + match(t, filter, { ids: [], classes: [], types: [] }, []); + + filter.add(
); + match(t, filter, { ids: [], classes: [], types: [["div", 1]] }, ["divSel"]); + + filter.add(
); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 2]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.add(
); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 3]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.add(); + match( + t, + filter, + { + ids: [["foo", 1]], + classes: [["foo", 1]], + types: [ + ["div", 3], + ["span", 1], + ], + }, + ["divSel", "dotFooSel", "spanSel", "hashFooSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 3]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.add(); + match( + t, + filter, + { + ids: [], + classes: [["foo", 2]], + types: [ + ["div", 3], + ["span", 1], + ], + }, + ["divSel", "dotFooSel", "spanSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 3]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.add(); + match( + t, + filter, + { + ids: [], + classes: [ + ["foo", 1], + ["bar", 1], + ], + types: [ + ["div", 3], + ["span", 1], + ], + }, + ["divSel", "dotFooSel", "spanSel", "dotBarSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 3]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.remove(
); + match(t, filter, { ids: [], classes: [["foo", 1]], types: [["div", 2]] }, [ + "divSel", + "dotFooSel", + ]); + + filter.remove(
); + match(t, filter, { ids: [], classes: [], types: [["div", 1]] }, ["divSel"]); + + filter.add(
); + match(t, filter, { ids: [], classes: [], types: [["div", 2]] }, ["divSel"]); + + filter.add(); + match( + t, + filter, + { + ids: [["bar", 1]], + classes: [], + types: [ + ["div", 2], + ["span", 1], + ], + }, + ["divSel", "spanSel", "hashBarSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [], types: [["div", 2]] }, ["divSel"]); + + filter.remove(
); + match(t, filter, { ids: [], classes: [], types: [["div", 1]] }, ["divSel"]); + + filter.add(
); + match(t, filter, { ids: [], classes: [["bar", 1]], types: [["div", 2]] }, [ + "divSel", + "dotBarSel", + ]); + + filter.add(); + match( + t, + filter, + { + ids: [], + classes: [ + ["bar", 1], + ["foo", 1], + ], + types: [ + ["div", 2], + ["span", 1], + ], + }, + ["divSel", "dotFooSel", "spanSel", "dotBarSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [["bar", 1]], types: [["div", 2]] }, [ + "divSel", + "dotBarSel", + ]); + + filter.add(); + match( + t, + filter, + { + ids: [], + classes: [["bar", 2]], + types: [ + ["div", 2], + ["span", 1], + ], + }, + ["divSel", "spanSel", "dotBarSel"], + ); + + filter.remove(); + match(t, filter, { ids: [], classes: [["bar", 1]], types: [["div", 2]] }, [ + "divSel", + "dotBarSel", + ]); + + filter.remove(
); + match(t, filter, { ids: [], classes: [], types: [["div", 1]] }, ["divSel"]); + + filter.remove(
); + match(t, filter, { ids: [], classes: [], types: [] }, []); + + filter.add(); + match(t, filter, { ids: [], classes: [], types: [["span", 1]] }, ["spanSel"]); + + filter.remove(); + match(t, filter, { ids: [], classes: [], types: [] }, []); +}); diff --git a/packages/alfa-cascade/tsconfig.json b/packages/alfa-cascade/tsconfig.json index 594fff846f..23289b7d88 100644 --- a/packages/alfa-cascade/tsconfig.json +++ b/packages/alfa-cascade/tsconfig.json @@ -1,13 +1,18 @@ { "$schema": "http://json.schemastore.org/tsconfig", "extends": "../tsconfig.json", + "compilerOptions": { + "jsx": "react-jsx", + "jsxImportSource": "@siteimprove/alfa-dom" + }, "files": [ "src/ancestor-filter.ts", "src/cascade.ts", "src/index.ts", "src/rule-tree.ts", "src/selector-map.ts", - "src/user-agent.ts" + "src/user-agent.ts", + "test/ancestor-filter.spec.tsx" ], "references": [ { "path": "../alfa-cache" }, @@ -21,6 +26,7 @@ { "path": "../alfa-option" }, { "path": "../alfa-predicate" }, { "path": "../alfa-refinement" }, - { "path": "../alfa-selector" } + { "path": "../alfa-selector" }, + { "path": "../alfa-test" } ] } From 1e3cf28ad57a1557dcd1b17633f98bc798dfb5a3 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 13:44:33 +0100 Subject: [PATCH 03/22] Improve comment --- packages/alfa-cascade/src/ancestor-filter.ts | 40 +++++++++++--------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/packages/alfa-cascade/src/ancestor-filter.ts b/packages/alfa-cascade/src/ancestor-filter.ts index 4ea1105bcc..f0c26d99d3 100644 --- a/packages/alfa-cascade/src/ancestor-filter.ts +++ b/packages/alfa-cascade/src/ancestor-filter.ts @@ -6,20 +6,23 @@ import * as json from "@siteimprove/alfa-json"; /** * The ancestor filter is a data structure used for optimising selector matching - * in the case of descendant selectors. When traversing down through the DOM - * tree during selector matching, the ancestor filter stores information about - * the ancestor elements that are found up the path from the element that is - * currently being visited. Given an element and a descendant selector, we can - * therefore quickly determine if the selector might match an ancestor of the - * current element. + * in the case of descendant selectors. * - * The information stored about elements includes their ID, classes, and type - * which are what the majority of selectors make use of. A bucket exists for - * each of these and whenever an element is added to the filter, its associated - * ID, classes, and type are added to the three buckets. The buckets also keep - * count of how many elements in the current path match a given ID, class, or - * type, in order to evict these from the filter when the last element with a - * given ID, class, or type is removed from the filter. + * @remarks + * When traversing down through the DOM tree during selector matching, the + * ancestor filter stores information about the ancestor elements that are + * found up the path from the element that is currently being visited. + * Given an element and a descendant selector, we can therefore quickly + * determine if the selector might match an ancestor of the current element. + * + * The ancestor filter simply count the number of each ID, class, and type + * amongst the path walked so far. When a descendant selector is encountered, we + * can quickly see if the ancestor filter contains the ID, class, or type of the + * ancestor bit, without walking up the full tree again. + * + * We need to remember exact count rather than just existence because the + * initial build of the cascade traverses the tree in depth-first order and + * therefore needs to be able to *remove* item from the filter when going up. * * For example, consider the following DOM tree: * @@ -28,11 +31,12 @@ import * as json from "@siteimprove/alfa-json"; * +-- p.highlight * +-- b * - * If we assume that we're currently visiting the `` element, the ancestor - * filter would contain the `section` and `p` types, the `#content` ID, - * and the `.highlight` class. Given a selector `main b`, we can therefore - * reject that the selector would match `` as the ancestor filter does not - * contain an entry for the type `main`. + * For the `` element, the ancestor filter would be: + * { ids: [["content", 1]], + * classes: [["highlight", 1]], + * types: [["p", 1], ["section", 1]]} + * Given a selector `main b`, we can therefore reject that the selector would + * match the `` as the ancestor filter does not contain the type `main`. * * NB: None of the operations of the ancestor filter are idempotent to avoid * keeping track of more information than strictly necessary. This is however From 5b9f36f75a6cb6cefe8d419933776cf54704f88b Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 16:21:28 +0100 Subject: [PATCH 04/22] Compute key selector at parse time --- .../test/ancestor-filter.spec.tsx | 1 - packages/alfa-cascade/test/rule-tree.spec.ts | 19 ++ packages/alfa-cascade/tsconfig.json | 3 +- .../alfa-selector/src/selector/complex.ts | 6 +- .../alfa-selector/src/selector/compound.ts | 6 +- .../alfa-selector/src/selector/selector.ts | 44 +++- .../src/selector/simple/class.ts | 5 + .../alfa-selector/src/selector/simple/id.ts | 5 + .../alfa-selector/src/selector/simple/type.ts | 3 + packages/alfa-selector/test/basic.spec.ts | 7 + packages/alfa-selector/test/complex.spec.ts | 188 ++++++++++++++++-- packages/alfa-selector/test/compound.spec.ts | 42 +++- packages/alfa-selector/test/list.spec.ts | 69 ++++++- 13 files changed, 359 insertions(+), 39 deletions(-) create mode 100644 packages/alfa-cascade/test/rule-tree.spec.ts diff --git a/packages/alfa-cascade/test/ancestor-filter.spec.tsx b/packages/alfa-cascade/test/ancestor-filter.spec.tsx index f80a916fc6..1e18455feb 100644 --- a/packages/alfa-cascade/test/ancestor-filter.spec.tsx +++ b/packages/alfa-cascade/test/ancestor-filter.spec.tsx @@ -6,7 +6,6 @@ import { AncestorFilter, Bucket } from "../src/ancestor-filter"; test("Buckets behave as expected", (t) => { const bucket = Bucket.empty(); - t.deepEqual(bucket.toJSON(), []); t(!bucket.has("a")); t(!bucket.has("b")); diff --git a/packages/alfa-cascade/test/rule-tree.spec.ts b/packages/alfa-cascade/test/rule-tree.spec.ts new file mode 100644 index 0000000000..68fa1502f8 --- /dev/null +++ b/packages/alfa-cascade/test/rule-tree.spec.ts @@ -0,0 +1,19 @@ +/// +import { h } from "@siteimprove/alfa-dom"; +import { None } from "@siteimprove/alfa-option"; +import { Type } from "@siteimprove/alfa-selector"; +import { test } from "@siteimprove/alfa-test"; + +import { RuleTree } from "../src"; + +test(".of() builds a node", (t) => { + const node = RuleTree.Node.of( + h.rule.style("div", { color: "red" }), + Type.of(None, "div"), + [], + [], + None, + ); + + console.dir(node.toJSON(), { depth: null }); +}); diff --git a/packages/alfa-cascade/tsconfig.json b/packages/alfa-cascade/tsconfig.json index 23289b7d88..28454a283a 100644 --- a/packages/alfa-cascade/tsconfig.json +++ b/packages/alfa-cascade/tsconfig.json @@ -12,7 +12,8 @@ "src/rule-tree.ts", "src/selector-map.ts", "src/user-agent.ts", - "test/ancestor-filter.spec.tsx" + "test/ancestor-filter.spec.tsx", + "test/rule-tree.spec.ts" ], "references": [ { "path": "../alfa-cache" }, diff --git a/packages/alfa-selector/src/selector/complex.ts b/packages/alfa-selector/src/selector/complex.ts index 4f0a8bc46b..fd5ef53f85 100644 --- a/packages/alfa-selector/src/selector/complex.ts +++ b/packages/alfa-selector/src/selector/complex.ts @@ -1,6 +1,7 @@ import type { Parser as CSSParser } from "@siteimprove/alfa-css"; import { Element } from "@siteimprove/alfa-dom"; import { Iterable } from "@siteimprove/alfa-iterable"; +import { Option } from "@siteimprove/alfa-option"; import { Parser } from "@siteimprove/alfa-parser"; import { Thunk } from "@siteimprove/alfa-thunk"; @@ -11,7 +12,7 @@ import { Specificity } from "../specificity"; import { Combinator } from "./combinator"; import { Compound } from "./compound"; import { Selector } from "./selector"; -import type { Simple } from "./simple"; +import type { Class, Id, Simple, Type } from "./simple"; const { isElement } = Element; const { map, pair, zeroOrMore } = Parser; @@ -33,6 +34,7 @@ export class Complex extends Selector<"complex"> { private readonly _combinator: Combinator; private readonly _left: Simple | Compound | Complex; private readonly _right: Simple | Compound; + protected readonly _key: Option; private constructor( combinator: Combinator, @@ -43,6 +45,8 @@ export class Complex extends Selector<"complex"> { this._combinator = combinator; this._left = left; this._right = right; + + this._key = right.key; } public get combinator(): Combinator { diff --git a/packages/alfa-selector/src/selector/compound.ts b/packages/alfa-selector/src/selector/compound.ts index 34ccfa19a6..5a1f8f8d96 100644 --- a/packages/alfa-selector/src/selector/compound.ts +++ b/packages/alfa-selector/src/selector/compound.ts @@ -2,6 +2,7 @@ import { Array } from "@siteimprove/alfa-array"; import { Token } from "@siteimprove/alfa-css"; import type { Element } from "@siteimprove/alfa-dom"; import { Iterable } from "@siteimprove/alfa-iterable"; +import { None, Option } from "@siteimprove/alfa-option"; import { Parser } from "@siteimprove/alfa-parser"; import { Slice } from "@siteimprove/alfa-slice"; @@ -10,7 +11,7 @@ import { Specificity } from "../specificity"; import type { Absolute } from "./index"; import { Selector } from "./selector"; -import { Simple } from "./simple"; +import { type Class, type Id, Simple, type Type } from "./simple"; const { map, oneOrMore } = Parser; @@ -26,6 +27,7 @@ export class Compound extends Selector<"compound"> { private readonly _selectors: Array; private readonly _length: number; + protected readonly _key: Option; private constructor(selectors: Array) { super( @@ -34,6 +36,8 @@ export class Compound extends Selector<"compound"> { ); this._selectors = selectors; this._length = selectors.length; + + this._key = selectors[0]?.key ?? None; } public get selectors(): Iterable { diff --git a/packages/alfa-selector/src/selector/selector.ts b/packages/alfa-selector/src/selector/selector.ts index 0d26c02f9d..e3dc2d63a1 100644 --- a/packages/alfa-selector/src/selector/selector.ts +++ b/packages/alfa-selector/src/selector/selector.ts @@ -4,6 +4,7 @@ import { Iterable } from "@siteimprove/alfa-iterable"; import { Serializable } from "@siteimprove/alfa-json"; import * as json from "@siteimprove/alfa-json"; +import { None, Option } from "@siteimprove/alfa-option"; import type { Context } from "../context"; import { Specificity } from "../specificity"; @@ -11,7 +12,7 @@ import { Specificity } from "../specificity"; import type { Complex } from "./complex"; import type { Compound } from "./compound"; import type { Relative } from "./relative"; -import type { Simple } from "./simple"; +import type { Class, Id, Simple, Type } from "./simple"; /** * @internal @@ -25,6 +26,37 @@ export abstract class Selector private readonly _type: T; private readonly _specificity: Specificity; + /** + * The key selector is used to optimise matching of complex (and compound) + * selectors. + * + * @remarks + * The key selector is the rightmost simple selector in a complex selector, + * or the leftmost simple selector in a compound selector. In order for an + * element to match a complex selector, it must match the key selector. + * + * For example, consider selector `main .foo + div`. Any element matching it + * must necessarily be a `
`, and for other elements there is no need to + * waste time traversing the DOM tree to check siblings or ancestors. + * + * For compound selectors, e.g. `.foo.bar`, any part could be taken, and we + * arbitrarily pick the leftmost. + * + * Conversely, an `` can only match selectors + * whose key selector is `img`, `#image`, `.foo`, or `.bar`. So we can + * pre-filter these when attempting matching. + * + * @privateRemarks + * Key selectors are not part of the CSS specification, but are a useful tool + * for optimising selector matching. + * + * Key selectors relate to cascading more than selector syntax and matching, + * but they only depend on selector and thus make sense as instance properties. + * + * {@link http://doc.servo.org/style/selector_map/struct.SelectorMap.html} + */ + protected readonly _key: Option = None; + protected constructor(type: T, specificity: Specificity) { this._type = type; this._specificity = specificity; @@ -39,6 +71,10 @@ export abstract class Selector return this._specificity; } + public get key(): Option { + return this._key; + } + /** * {@link https://drafts.csswg.org/selectors/#match} */ @@ -65,16 +101,20 @@ export abstract class Selector return { type: this._type, specificity: this._specificity.toJSON(), + ...(this._key.isSome() ? { key: `${this._key.get()}` } : {}), }; } } export namespace Selector { export interface JSON { - [key: string]: json.JSON; + [key: string]: json.JSON | undefined; type: T; specificity: Specificity.JSON; + // Since the key selector may be the selector itself, we only return its + // string representation to avoid infinite recursion. + key?: string; } } diff --git a/packages/alfa-selector/src/selector/simple/class.ts b/packages/alfa-selector/src/selector/simple/class.ts index 04ac9325e8..f574fb9c14 100644 --- a/packages/alfa-selector/src/selector/simple/class.ts +++ b/packages/alfa-selector/src/selector/simple/class.ts @@ -1,6 +1,7 @@ import { Token } from "@siteimprove/alfa-css"; import type { Element } from "@siteimprove/alfa-dom"; import { Iterable } from "@siteimprove/alfa-iterable"; +import { Option } from "@siteimprove/alfa-option"; import { Parser } from "@siteimprove/alfa-parser"; import { Specificity } from "../../specificity"; @@ -18,8 +19,12 @@ export class Class extends WithName<"class"> { public static of(name: string): Class { return new Class(name); } + + protected readonly _key: Option; private constructor(name: string) { super("class", name, Specificity.of(0, 1, 0)); + + this._key = Option.of(this); } public matches(element: Element): boolean { diff --git a/packages/alfa-selector/src/selector/simple/id.ts b/packages/alfa-selector/src/selector/simple/id.ts index 6c8d2a882d..bde2d124d4 100644 --- a/packages/alfa-selector/src/selector/simple/id.ts +++ b/packages/alfa-selector/src/selector/simple/id.ts @@ -1,5 +1,6 @@ import { Token } from "@siteimprove/alfa-css"; import type { Element } from "@siteimprove/alfa-dom"; +import { Option } from "@siteimprove/alfa-option"; import { Parser } from "@siteimprove/alfa-parser"; import { Specificity } from "../../specificity"; @@ -18,8 +19,12 @@ export class Id extends WithName<"id"> { return new Id(name); } + protected readonly _key: Option; + private constructor(name: string) { super("id", name, Specificity.of(1, 0, 0)); + + this._key = Option.of(this); } public matches(element: Element): boolean { diff --git a/packages/alfa-selector/src/selector/simple/type.ts b/packages/alfa-selector/src/selector/simple/type.ts index 45196a3506..2839afb9ee 100644 --- a/packages/alfa-selector/src/selector/simple/type.ts +++ b/packages/alfa-selector/src/selector/simple/type.ts @@ -21,10 +21,13 @@ export class Type extends WithName<"type"> { } private readonly _namespace: Option; + protected readonly _key: Option; private constructor(namespace: Option, name: string) { super("type", name, Specificity.of(0, 0, 1)); this._namespace = namespace; + + this._key = Option.of(this); } public get namespace(): Option { diff --git a/packages/alfa-selector/test/basic.spec.ts b/packages/alfa-selector/test/basic.spec.ts index feeeb034d6..ff60af7ed9 100644 --- a/packages/alfa-selector/test/basic.spec.ts +++ b/packages/alfa-selector/test/basic.spec.ts @@ -9,6 +9,7 @@ test(".parse() parses a type selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }); }); @@ -18,6 +19,7 @@ test(".parse() parses an uppercase type selector", (t) => { name: "DIV", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "DIV", }); }); @@ -27,6 +29,7 @@ test(".parse() parses a type selector with a namespace", (t) => { name: "a", namespace: "svg", specificity: { a: 0, b: 0, c: 1 }, + key: "svg|a", }); }); @@ -36,6 +39,7 @@ test(".parse() parses a type selector with an empty namespace", (t) => { name: "a", namespace: "", specificity: { a: 0, b: 0, c: 1 }, + key: "|a", }); }); @@ -45,6 +49,7 @@ test(".parse() parses a type selector with the universal namespace", (t) => { name: "a", namespace: "*", specificity: { a: 0, b: 0, c: 1 }, + key: "*|a", }); }); @@ -77,6 +82,7 @@ test(".parse() parses a class selector", (t) => { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }); }); @@ -85,6 +91,7 @@ test(".parse() parses an ID selector", (t) => { type: "id", name: "foo", specificity: { a: 1, b: 0, c: 0 }, + key: "#foo", }); }); diff --git a/packages/alfa-selector/test/complex.spec.ts b/packages/alfa-selector/test/complex.spec.ts index 6059241165..ca85ea55a9 100644 --- a/packages/alfa-selector/test/complex.spec.ts +++ b/packages/alfa-selector/test/complex.spec.ts @@ -12,9 +12,16 @@ test(".parse() parses a single descendant selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }); }); @@ -27,14 +34,17 @@ test(".parse() parses a single descendant selector with a right-hand type select name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "type", name: "span", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "span", }, specificity: { a: 0, b: 0, c: 2 }, + key: "span", }); }); @@ -50,12 +60,25 @@ test(".parse() parses a double descendant selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", + }, + right: { + type: "id", + name: "bar", + specificity: { a: 1, b: 0, c: 0 }, + key: "#bar", }, - right: { type: "id", name: "bar", specificity: { a: 1, b: 0, c: 0 } }, specificity: { a: 1, b: 1, c: 1 }, + key: "#bar", }); }); @@ -68,9 +91,16 @@ test(".parse() parses a direct descendant selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }); }); @@ -83,9 +113,16 @@ test(".parse() parses a sibling selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }); }); @@ -98,9 +135,16 @@ test(".parse() parses a direct sibling selector", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }); }); @@ -108,7 +152,12 @@ test(".parse() parses a compound selector relative to a class selector", (t) => t.deepEqual(serialize(".foo div.bar"), { type: "complex", combinator: Combinator.Descendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, right: { type: "compound", selectors: [ @@ -117,12 +166,20 @@ test(".parse() parses a compound selector relative to a class selector", (t) => name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 2, c: 1 }, + key: "div", }); }); @@ -138,10 +195,17 @@ test(".parse() parses a compound selector relative to a compound selector", (t) name: "span", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "span", + }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "span", }, right: { type: "compound", @@ -151,12 +215,20 @@ test(".parse() parses a compound selector relative to a compound selector", (t) name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 2, c: 2 }, + key: "div", }); }); @@ -172,17 +244,26 @@ test(".parse() parses a descendant selector relative to a sibling selector", (t) name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "type", name: "span", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "span", }, specificity: { a: 0, b: 0, c: 2 }, + key: "span", + }, + right: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, specificity: { a: 0, b: 1, c: 2 }, + key: ".foo", }); }); @@ -195,6 +276,7 @@ test(".parse() parses an attribute selector when part of a descendant selector", name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "attribute", @@ -217,6 +299,7 @@ test(".parse() parses an attribute selector when part of a compound selector rel type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, right: { type: "compound", @@ -226,6 +309,7 @@ test(".parse() parses an attribute selector when part of a compound selector rel name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, { type: "attribute", @@ -238,8 +322,10 @@ test(".parse() parses an attribute selector when part of a compound selector rel }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 2, c: 1 }, + key: "div", }); }); @@ -252,6 +338,7 @@ test(".parse() parses a pseudo-element selector when part of a descendant select name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "pseudo-element", @@ -266,7 +353,12 @@ test(".parse() parses a pseudo-element selector when part of a compound selector t.deepEqual(serialize(".foo div::before"), { type: "complex", combinator: Combinator.Descendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, right: { type: "compound", selectors: [ @@ -275,6 +367,7 @@ test(".parse() parses a pseudo-element selector when part of a compound selector name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, { type: "pseudo-element", @@ -283,8 +376,10 @@ test(".parse() parses a pseudo-element selector when part of a compound selector }, ], specificity: { a: 0, b: 0, c: 2 }, + key: "div", }, specificity: { a: 0, b: 1, c: 2 }, + key: "div", }); }); @@ -292,7 +387,12 @@ test(".parse() parses a pseudo-class selector when part of a compound selector r t.deepEqual(serialize(".foo div:hover"), { type: "complex", combinator: Combinator.Descendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, right: { type: "compound", selectors: [ @@ -301,6 +401,7 @@ test(".parse() parses a pseudo-class selector when part of a compound selector r name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, { type: "pseudo-class", @@ -309,8 +410,10 @@ test(".parse() parses a pseudo-class selector when part of a compound selector r }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 2, c: 1 }, + key: "div", }); }); @@ -318,7 +421,12 @@ test(".parse() parses a compound type, class, and pseudo-class selector relative t.deepEqual(serialize(".foo div.bar:hover"), { type: "complex", combinator: Combinator.Descendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, right: { type: "compound", selectors: [ @@ -327,8 +435,14 @@ test(".parse() parses a compound type, class, and pseudo-class selector relative name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, { type: "pseudo-class", name: "hover", @@ -336,8 +450,10 @@ test(".parse() parses a compound type, class, and pseudo-class selector relative }, ], specificity: { a: 0, b: 2, c: 1 }, + key: "div", }, specificity: { a: 0, b: 3, c: 1 }, + key: "div", }); }); @@ -345,7 +461,12 @@ test(".parse() parses a simple selector relative to a compound selector", (t) => t.deepEqual(serialize(".foo > div.bar"), { type: "complex", combinator: Combinator.DirectDescendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, right: { type: "compound", selectors: [ @@ -354,12 +475,20 @@ test(".parse() parses a simple selector relative to a compound selector", (t) => name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 2, c: 1 }, + key: "div", }); }); @@ -370,9 +499,20 @@ test(".parse() parses a relative selector relative to a compound selector", (t) left: { type: "complex", combinator: Combinator.DirectDescendant, - left: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, - right: { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, + left: { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, + right: { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", + }, specificity: { a: 0, b: 2, c: 0 }, + key: ".bar", }, right: { type: "compound", @@ -382,11 +522,19 @@ test(".parse() parses a relative selector relative to a compound selector", (t) name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "baz", + specificity: { a: 0, b: 1, c: 0 }, + key: ".baz", }, - { type: "class", name: "baz", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }, specificity: { a: 0, b: 3, c: 1 }, + key: "div", }); }); diff --git a/packages/alfa-selector/test/compound.spec.ts b/packages/alfa-selector/test/compound.spec.ts index b8104062d7..c414e34621 100644 --- a/packages/alfa-selector/test/compound.spec.ts +++ b/packages/alfa-selector/test/compound.spec.ts @@ -6,10 +6,21 @@ test(".parse() parses a compound selector", (t) => { t.deepEqual(serialize("#foo.bar"), { type: "compound", selectors: [ - { type: "id", name: "foo", specificity: { a: 1, b: 0, c: 0 } }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "id", + name: "foo", + specificity: { a: 1, b: 0, c: 0 }, + key: "#foo", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", + }, ], specificity: { a: 1, b: 1, c: 0 }, + key: "#foo", }); }); @@ -22,10 +33,17 @@ test(".parse() parses a compound selector with a type in prefix position", (t) = name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", + }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }); }); @@ -33,7 +51,12 @@ test(".parse() parses an attribute selector when part of a compound selector", ( t.deepEqual(serialize(".foo[foo]"), { type: "compound", selectors: [ - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, { type: "attribute", name: "foo", @@ -45,6 +68,7 @@ test(".parse() parses an attribute selector when part of a compound selector", ( }, ], specificity: { a: 0, b: 2, c: 0 }, + key: ".foo", }); }); @@ -52,7 +76,12 @@ test(".parse() parses a pseudo-element selector when part of a compound selector t.deepEqual(serialize(".foo::before"), { type: "compound", selectors: [ - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, { type: "pseudo-element", name: "before", @@ -60,6 +89,7 @@ test(".parse() parses a pseudo-element selector when part of a compound selector }, ], specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }); }); @@ -72,6 +102,7 @@ test(".parse() parses a pseudo-class selector when part of a compound selector", name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, { type: "pseudo-class", @@ -80,5 +111,6 @@ test(".parse() parses a pseudo-class selector when part of a compound selector", }, ], specificity: { a: 0, b: 1, c: 1 }, + key: "div", }); }); diff --git a/packages/alfa-selector/test/list.spec.ts b/packages/alfa-selector/test/list.spec.ts index 4020dea579..dd1e3761cb 100644 --- a/packages/alfa-selector/test/list.spec.ts +++ b/packages/alfa-selector/test/list.spec.ts @@ -7,9 +7,24 @@ test(".parse() parses a list of simple selectors", (t) => { t.deepEqual(serialize(".foo, .bar, .baz"), { type: "list", selectors: [ - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, - { type: "class", name: "baz", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", + }, + { + type: "class", + name: "baz", + specificity: { a: 0, b: 1, c: 0 }, + key: ".baz", + }, ], specificity: { a: 0, b: 1, c: 0 }, }); @@ -19,14 +34,30 @@ test(".parse() parses a list of simple and compound selectors", (t) => { t.deepEqual(serialize(".foo, #bar.baz"), { type: "list", selectors: [ - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, { type: "compound", selectors: [ - { type: "id", name: "bar", specificity: { a: 1, b: 0, c: 0 } }, - { type: "class", name: "baz", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "id", + name: "bar", + specificity: { a: 1, b: 0, c: 0 }, + key: "#bar", + }, + { + type: "class", + name: "baz", + specificity: { a: 0, b: 1, c: 0 }, + key: ".baz", + }, ], specificity: { a: 1, b: 1, c: 0 }, + key: "#bar", }, ], specificity: { a: 1, b: 1, c: 0 }, @@ -45,13 +76,16 @@ test(".parse() parses a list of descendant selectors", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }, { type: "complex", @@ -61,13 +95,16 @@ test(".parse() parses a list of descendant selectors", (t) => { name: "span", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "span", }, right: { type: "class", name: "baz", specificity: { a: 0, b: 1, c: 0 }, + key: ".baz", }, specificity: { a: 0, b: 1, c: 1 }, + key: ".baz", }, ], specificity: { a: 0, b: 1, c: 1 }, @@ -86,13 +123,16 @@ test(".parse() parses a list of sibling selectors", (t) => { name: "div", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, right: { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, specificity: { a: 0, b: 1, c: 1 }, + key: ".foo", }, { type: "complex", @@ -102,13 +142,16 @@ test(".parse() parses a list of sibling selectors", (t) => { name: "span", namespace: null, specificity: { a: 0, b: 0, c: 1 }, + key: "span", }, right: { type: "class", name: "baz", specificity: { a: 0, b: 1, c: 0 }, + key: ".baz", }, specificity: { a: 0, b: 1, c: 1 }, + key: ".baz", }, ], specificity: { a: 0, b: 1, c: 1 }, @@ -119,8 +162,18 @@ test(".parse() parses a list of selectors with no whitespace", (t) => { t.deepEqual(serialize(".foo,.bar"), { type: "list", selectors: [ - { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 } }, - { type: "class", name: "bar", specificity: { a: 0, b: 1, c: 0 } }, + { + type: "class", + name: "foo", + specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", + }, + { + type: "class", + name: "bar", + specificity: { a: 0, b: 1, c: 0 }, + key: ".bar", + }, ], specificity: { a: 0, b: 1, c: 0 }, }); From e0be86269eefc06cba8ae3de605d072b035fe02d Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 16:22:38 +0100 Subject: [PATCH 05/22] Fix key selector test --- packages/alfa-selector/test/pseudo-class.spec.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/alfa-selector/test/pseudo-class.spec.tsx b/packages/alfa-selector/test/pseudo-class.spec.tsx index 51b3be6d4f..6faecf0b3f 100644 --- a/packages/alfa-selector/test/pseudo-class.spec.tsx +++ b/packages/alfa-selector/test/pseudo-class.spec.tsx @@ -27,6 +27,7 @@ test(".parse() parses a functional pseudo-class selector", (t) => { type: "class", name: "foo", specificity: { a: 0, b: 1, c: 0 }, + key: ".foo", }, specificity: { a: 0, b: 1, c: 0 }, }); From 9999736a75a96c91a79945583f7a5a501e9d8591 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 16:30:24 +0100 Subject: [PATCH 06/22] Move key selector calculation at parse time --- packages/alfa-cascade/src/selector-map.ts | 36 ++++------------------- 1 file changed, 5 insertions(+), 31 deletions(-) diff --git a/packages/alfa-cascade/src/selector-map.ts b/packages/alfa-cascade/src/selector-map.ts index 27372e6b6a..ab505f6367 100644 --- a/packages/alfa-cascade/src/selector-map.ts +++ b/packages/alfa-cascade/src/selector-map.ts @@ -202,16 +202,14 @@ export namespace SelectorMap { ): void => { const node = Node.of(rule, selector, declarations, origin, order); - const keySelector = getKeySelector(selector); + const keySelector = selector.key; - if (keySelector === null) { + if (!keySelector.isSome()) { other.push(node); - } else if (isId(keySelector)) { - ids.add(keySelector.name, node); - } else if (isClass(keySelector)) { - classes.add(keySelector.name, node); } else { - types.add(keySelector.name, node); + const key = keySelector.get(); + const buckets = { id: ids, class: classes, type: types }; + buckets[key.type].add(key.name, node); } }; @@ -424,30 +422,6 @@ export namespace SelectorMap { } } -/** - * Given a selector, get the right-most ID, class, or type selector, i.e. the - * key selector. If the right-most selector is a compound selector, then the - * left-most ID, class, or type selector of the compound selector is returned. - */ -function getKeySelector(selector: Selector): Id | Class | Type | null { - if (isId(selector) || isClass(selector) || isType(selector)) { - return selector; - } - - if (isCompound(selector)) { - return Iterable.find( - Iterable.map(selector.selectors, getKeySelector), - (selector) => selector !== null, - ).getOr(null); - } - - if (isComplex(selector)) { - return getKeySelector(selector.right); - } - - return null; -} - /** * Check if a selector can be rejected based on an ancestor filter. */ From dfa3036ec97c68002102daa2dece6103e8522461 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 6 Dec 2023 16:32:27 +0100 Subject: [PATCH 07/22] Extract API --- docs/review/api/alfa-selector.api.md | 16 +++++++++++++--- packages/alfa-cascade/src/ancestor-filter.ts | 4 ++-- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/docs/review/api/alfa-selector.api.md b/docs/review/api/alfa-selector.api.md index d8a9c3e574..6b5c8c1b26 100644 --- a/docs/review/api/alfa-selector.api.md +++ b/docs/review/api/alfa-selector.api.md @@ -109,6 +109,8 @@ export class Class extends WithName<"class"> { // (undocumented) equals(value: unknown): value is this; // (undocumented) + protected readonly _key: Option; + // (undocumented) matches(element: Element): boolean; // (undocumented) static of(name: string): Class; @@ -160,6 +162,8 @@ export class Complex extends Selector_2<"complex"> { // (undocumented) equals(value: unknown): value is this; // (undocumented) + protected readonly _key: Option; + // (undocumented) get left(): Simple | Compound | Complex; // (undocumented) matches(element: Element, context?: Context): boolean; @@ -187,7 +191,7 @@ export namespace Complex { right: Simple.JSON | Compound.JSON; } const // @internal (undocumented) - parseComplex: (parseSelector: Thunk>) => Parser, Simple | Compound | Complex, string, []>; + parseComplex: (parseSelector: Thunk>) => Parser, Compound | Simple | Complex, string, []>; } // @public (undocumented) @@ -199,6 +203,8 @@ export class Compound extends Selector_2<"compound"> { // (undocumented) equals(value: unknown): value is this; // (undocumented) + protected readonly _key: Option; + // (undocumented) get length(): number; // (undocumented) matches(element: Element, context?: Context): boolean; @@ -222,7 +228,7 @@ export namespace Compound { selectors: Array_2; } const // @internal (undocumented) - parseCompound: (parseSelector: () => Parser, Absolute, string>) => Parser, Simple | Compound, string, []>; + parseCompound: (parseSelector: () => Parser, Absolute, string>) => Parser, Compound | Simple, string, []>; } // @public (undocumented) @@ -295,6 +301,8 @@ export class Id extends WithName<"id"> { // (undocumented) equals(value: unknown): value is this; // (undocumented) + protected readonly _key: Option; + // (undocumented) matches(element: Element): boolean; // (undocumented) static of(name: string): Id; @@ -347,7 +355,7 @@ export namespace List { selectors: Array_2>; } const // @internal (undocumented) - parseList: (parseSelector: Thunk>) => Parser, List, string, []>; + parseList: (parseSelector: Thunk>) => Parser, List, string, []>; } // Warning: (ae-forgotten-export) The symbol "Active" needs to be exported by the entry point index.d.ts @@ -494,6 +502,8 @@ export class Type extends WithName<"type"> { // (undocumented) equals(value: unknown): value is this; // (undocumented) + protected readonly _key: Option; + // (undocumented) matches(element: Element): boolean; // (undocumented) get namespace(): Option; diff --git a/packages/alfa-cascade/src/ancestor-filter.ts b/packages/alfa-cascade/src/ancestor-filter.ts index f0c26d99d3..58e981d0f5 100644 --- a/packages/alfa-cascade/src/ancestor-filter.ts +++ b/packages/alfa-cascade/src/ancestor-filter.ts @@ -32,9 +32,9 @@ import * as json from "@siteimprove/alfa-json"; * +-- b * * For the `` element, the ancestor filter would be: - * { ids: [["content", 1]], + * \{ ids: [["content", 1]], * classes: [["highlight", 1]], - * types: [["p", 1], ["section", 1]]} + * types: [["p", 1], ["section", 1]]\} * Given a selector `main b`, we can therefore reject that the selector would * match the `` as the ancestor filter does not contain the type `main`. * From 4677db1ad9b344e30b8f171cfde385ac4af5295b Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Fri, 8 Dec 2023 13:12:07 +0100 Subject: [PATCH 08/22] Update lockfile --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index da9d82d063..c57416c688 100644 --- a/yarn.lock +++ b/yarn.lock @@ -991,6 +991,7 @@ __metadata: "@siteimprove/alfa-predicate": "workspace:^0.69.0" "@siteimprove/alfa-refinement": "workspace:^0.69.0" "@siteimprove/alfa-selector": "workspace:^0.69.0" + "@siteimprove/alfa-test": "workspace:^0.69.0" languageName: unknown linkType: soft From 9e240e3d5a85f1d3a0c867f4c18ece8372727b02 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Fri, 8 Dec 2023 15:43:21 +0100 Subject: [PATCH 09/22] Add some documentation --- .changeset/pink-walls-remain.md | 5 +++++ packages/alfa-cascade/README.md | 37 +++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 .changeset/pink-walls-remain.md create mode 100644 packages/alfa-cascade/README.md diff --git a/.changeset/pink-walls-remain.md b/.changeset/pink-walls-remain.md new file mode 100644 index 0000000000..4cedf75880 --- /dev/null +++ b/.changeset/pink-walls-remain.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-selector": minor +--- + +**Added:** Selectors now contain a "key selector" which is the leftmost simple selector in a compound one, or the rightmost in a complex one. diff --git a/packages/alfa-cascade/README.md b/packages/alfa-cascade/README.md new file mode 100644 index 0000000000..7e7f725d63 --- /dev/null +++ b/packages/alfa-cascade/README.md @@ -0,0 +1,37 @@ +# Alfa Cascade + +This package builds the cascade which is then used by `@siteimprove/alfa-style` in order to find the cascaded value of each property. + +While resolving the cascade is in theory somewhat easy (for each element and property, find the highest precedence declaration), it requires a lot of work. A typical page contain hundreds of element and possibly thousands of style rules, and we support nearly 150 properties. So, we cannot just brute force our way through this and need structures to help eliminate quickly most of the selectors. This is especially true for the descendants and sibling selectors whose matching require traversing the DOM tree, potentially at a far away place for something like `main .foo` + +## Ancestor filter + +The ancestor filter is a structure to optimize matching of descendants selectors. It is build during a depth-first traversal of the DOM tree. While inspecting each element (and trying to match selectors), we keep a list of the ancestors we've found. + +In order to be compact and efficient, we just count the number of each type, class, and id on the path to the element. So, for example, a `div.foo .bar` selector cannot match if there is no `div` type or `.foo` class along the path. We cannot just keep a boolean because we want to be able to update the ancestor filter during the "upward moves" of the traversal, which require removing elements from it, so we need a precise count to figure out when it reaches 0. + +The ancestor filter only allow for guaranteed "won't match" answers, because the type, class and id have been separated. For example, a `div.foo .bar` selector won't match if the `div` and `.foo` ancestors are different, but the ancestor filter doesn't hold that information. However, the filter greatly reduce the actual number of elements to test against each descendant selector and thus the amount of work to be done. + +## Key selector and Selector map + +The other tool to speed up matching of complex (and compound) selectors is the selector map. + +Each selector is associated to a _key selector_ which is the part which is going to be matched against the element itself (not against its siblings or ancestors). For complex selector, the key selector is thus the rightmost bit. For compound selector, it could be any bit; we take the leftmost bit assuming that they are ordered significantly by authors. + +That is, in a `div.foo .bar` selector, the key selector is `.bar`. Any element that matches the full `div.foo .bar` selector must necessarily be a `.bar` itself (plus some DOM tree context). For anything else, we don't need to look at DOM structure. Similarly, in the `div.foo` selector, the key selector is `div`. + +Conversely, an element can only match selectors if it matches their key selector. So, a `` can only match selectors whose key selector is either `span`, `.bar`, `.baz`, or `#my-id`. + +The selector map stores selectors by their key selector. Thus, when searching for selectors that may match a given element, we only ask the selector map for selectors that have one of the possible key selectors and greatly reduce the search space. + +## Rule tree + +The rule tree (actually a forest, although rooted at `*`) is a representation of the join-semilattice of selectors (and their associated style rules). + +Selectors can be ordered by inclusion of the sets of matched elements. This forms a join-semilattice. For example, `div.foo` is smaller than both `div` and `.foo` because anything matching the former must necessarily match both of the later ones. However, `div` and `.foo` are not comparable. The semilattice of selectors existing in the current document is represented as a tree, which allow to share identical parts of otherwise incomparable selectors. For example, `div.foo.bar` and `div.foo.baz` are both smaller than `div.foo` and therefore can share any information related to it. + +The rule tree is dependent of the order in which it can is built (TO VERIFY). + +## Cascade + +The cascade itself is a rule tree associated with a map from elements to nodes in it. Each element is mapped to its highest priority selector in the rule tree. Thus, in order to find the cascaded value of any property for a given element, we can simply walk up the rule tree until we find a selector (and associated rule) declaring that property. Since we've walk up the lattice from the smallest possible selector (highest priority), this will be the cascaded value, no matter if more rules up the tree also define this property. From be889e8d3e5ec20e402209e03bc7939e0561dcd4 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Tue, 12 Dec 2023 15:36:45 +0100 Subject: [PATCH 10/22] Add documentation --- packages/alfa-cascade/README.md | 14 ++- packages/alfa-cascade/src/cascade.ts | 8 +- packages/alfa-cascade/src/rule-tree.ts | 103 ++++++++++++++++++---- packages/alfa-cascade/src/selector-map.ts | 6 +- 4 files changed, 99 insertions(+), 32 deletions(-) diff --git a/packages/alfa-cascade/README.md b/packages/alfa-cascade/README.md index 7e7f725d63..f729182a1e 100644 --- a/packages/alfa-cascade/README.md +++ b/packages/alfa-cascade/README.md @@ -8,9 +8,9 @@ While resolving the cascade is in theory somewhat easy (for each element and pro The ancestor filter is a structure to optimize matching of descendants selectors. It is build during a depth-first traversal of the DOM tree. While inspecting each element (and trying to match selectors), we keep a list of the ancestors we've found. -In order to be compact and efficient, we just count the number of each type, class, and id on the path to the element. So, for example, a `div.foo .bar` selector cannot match if there is no `div` type or `.foo` class along the path. We cannot just keep a boolean because we want to be able to update the ancestor filter during the "upward moves" of the traversal, which require removing elements from it, so we need a precise count to figure out when it reaches 0. +In order to be compact and efficient, we just count the number of each type, class, and id on the path to the element. So, for example, a `div.foo .bar` selector cannot match if there is no `div` type or `.foo` class along the path. We cannot just keep a boolean because we want to be able to update the ancestor filter during the "upward moves" of the traversal, which requires removing elements from it, so we need a precise count to figure out when it reaches 0. -The ancestor filter only allow for guaranteed "won't match" answers, because the type, class and id have been separated. For example, a `div.foo .bar` selector won't match if the `div` and `.foo` ancestors are different, but the ancestor filter doesn't hold that information. However, the filter greatly reduce the actual number of elements to test against each descendant selector and thus the amount of work to be done. +The ancestor filter only allow for guaranteed "won't match" answers, because the type, class and id have been separated for the sake of compactness. For example, a `div.foo .bar` selector won't match if the `div` and `.foo` ancestors are different, but the ancestor filter doesn't hold that information. However, the filter greatly reduce the actual number of elements to test against each descendant selector and thus the amount of work to be done. ## Key selector and Selector map @@ -22,16 +22,14 @@ That is, in a `div.foo .bar` selector, the key selector is `.bar`. Any element t Conversely, an element can only match selectors if it matches their key selector. So, a `` can only match selectors whose key selector is either `span`, `.bar`, `.baz`, or `#my-id`. -The selector map stores selectors by their key selector. Thus, when searching for selectors that may match a given element, we only ask the selector map for selectors that have one of the possible key selectors and greatly reduce the search space. +The selector map groups selectors by their key selector. Thus, when searching for selectors that may match a given element, we only ask the selector map for selectors that have one of the possible key selectors and greatly reduce the search space. ## Rule tree -The rule tree (actually a forest, although rooted at `*`) is a representation of the join-semilattice of selectors (and their associated style rules). +The rule tree (actually a forest) is a representation of the association between elements and the list of selectors (actually, rules) that they match, in decreasing precedence (according to cascade sorting). -Selectors can be ordered by inclusion of the sets of matched elements. This forms a join-semilattice. For example, `div.foo` is smaller than both `div` and `.foo` because anything matching the former must necessarily match both of the later ones. However, `div` and `.foo` are not comparable. The semilattice of selectors existing in the current document is represented as a tree, which allow to share identical parts of otherwise incomparable selectors. For example, `div.foo.bar` and `div.foo.baz` are both smaller than `div.foo` and therefore can share any information related to it. - -The rule tree is dependent of the order in which it can is built (TO VERIFY). +Using a tree, rather than a separated list for each element allows to share the selectors that are matching several elements and reduce the memory usage. ## Cascade -The cascade itself is a rule tree associated with a map from elements to nodes in it. Each element is mapped to its highest priority selector in the rule tree. Thus, in order to find the cascaded value of any property for a given element, we can simply walk up the rule tree until we find a selector (and associated rule) declaring that property. Since we've walk up the lattice from the smallest possible selector (highest priority), this will be the cascaded value, no matter if more rules up the tree also define this property. +The cascade itself is a rule tree associated with a map from elements to nodes in it. Each element is mapped to its highest precedence selector in the rule tree. Thus, in order to find the cascaded value of any property for a given element, we can simply walk up the rule tree until we find a selector (and associated rule) declaring that property. Since we've walk up the lattice from the smallest possible selector (highest precedence), this will be the cascaded value, no matter if more rules up the tree also define this property. diff --git a/packages/alfa-cascade/src/cascade.ts b/packages/alfa-cascade/src/cascade.ts index 7400213d90..90743ea759 100644 --- a/packages/alfa-cascade/src/cascade.ts +++ b/packages/alfa-cascade/src/cascade.ts @@ -14,7 +14,7 @@ import { SelectorMap } from "./selector-map"; import { UserAgent } from "./user-agent"; /** - * {@link https://drafts.csswg.org/css-cascade/} + * {@link https://drafts.csswg.org/css-cascade-5/} * * @public */ @@ -110,7 +110,7 @@ export namespace Cascade { } /** - * {@link https://drafts.csswg.org/css-cascade/#cascade-sort} + * {@link https://drafts.csswg.org/css-cascade-5/#cascade-sort} */ const compare: Comparer = (a, b) => { // First priority: Origin @@ -123,8 +123,8 @@ const compare: Comparer = (a, b) => { return a.specificity < b.specificity ? -1 : a.specificity > b.specificity - ? 1 - : 0; + ? 1 + : 0; } // Third priority: Order. diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index 60c3db108f..53fae84b13 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -8,35 +8,38 @@ import * as json from "@siteimprove/alfa-json"; /** * The rule tree is a data structure used for storing the rules that match each - * element when computing cascade for a document. Rules are stored in order - * from most to least specific; rules lower in the tree are therefore more - * specific than rules higher in the tree. Each element gets a pointer to the - * most specific rule it matched and can then follow pointers up the rule tree - * to locate decreasingly specific rules that the element also matches. This - * allows elements that share matched rules to also share a path in the rule - * tree. + * element when computing cascade for a document. * - * As an example, consider elements A and B. Element A matches rules `div`, - * `.foo` and `.foo[href]` whereas element B matches rules `div`, `.foo` and - * `.bar`. The naïve approach to associating these matched rules with elements + * @remarks + * Rules are stored in order from most to least precedence (according to cascade + * sorting order); rules lower in the tree have therefore higher precedence than + * rules higher in the tree. Each element gets a pointer to the highest + * precedence rule it matched and can then follow pointers up the rule tree to + * locate rules of decreasing precedence that the element also matches. This + * allows elements that share matched rules to also share a path in the rule tree. + * + * As an example, consider elements `A =
` and + * `B =
`. Element A matches rules `div`, `.foo` and + * `.foo[href]` whereas element B matches rules `div`, `.foo` and `.bar`. The + * naïve approach to associating these matched rules with elements * would be to associate an array of `[".foo[href]", ".foo", "div"]` with - * element A and an array of `[".foo", "div"]` with element B. With the rule - * tree, we instead start by inserting the matched rules for element A into the - * tree: + * element A and an array of `[".bar", ".foo", "div"]` with element B. With the + * rule tree, we instead start by inserting the matched rules for element A into + * the tree: * * "div" * +-- ".foo" - * +-- ".foo[href]" + * +-- ".foo[href]" <- A * * We then associate rule `".foo[href]"` with element A and insert the matched * rules for element B into the tree: * * "div" * +-- ".foo" - * +-- ".foo[href]" - * +-- ".bar" + * +-- ".foo[href]" <- A + * +-- ".bar" <- B * - * We then associate the rule `".bar"` with element B and we're done. Notice how + * We then associate the rule `".bar"` with element B, and we're done. Notice how * the tree branches at rule `".foo"`, allowing the two elements to share the * path in the rule tree that they have in common. This approach is conceptually * similar to associating arrays of matched rules with elements with the @@ -46,8 +49,33 @@ import * as json from "@siteimprove/alfa-json"; * rules that match most elements, such as the universal selector or `html` and * `body`. * + * Note that the resulting rule tree depends greatly on the order in which + * rules are inserted, which must then be by increasing precedence. The `.foo` + * and `.bar` selectors are not directly comparable, the example above assumes + * that the `.bar` rule came later in the style sheet order and therefore wins + * the cascade sort by "Order of Appearance". This information is not available + * for the rule tree which relies on rules being fed to it in increasing + * precedence for each element. If `.bar` came before `.foo`, the resulting tree + * would be: + * + * "div" + * +-- ".foo" + * +-- ".foo[href]" <- A + * +-- ".bar" + * +-- ".foo" <- B + * * {@link http://doc.servo.org/style/rule_tree/struct.RuleTree.html} * + * @privateRemarks + * The rules tree is actually a forest of nodes since many elements do not share + * any matched selector. We do not artificially root it as it would add little + * value, there is no natural root, and creating a fake root would actually + * make processing the tree harder (as we would need to handle that fake node). + * As a consequence, when inserting new rules in the tree, we may start a + * completely new tree in that forest. This means that rules may be inserted + * without a parent, and adding a single rule must be a static method of the + * Node class, rater than an instance method. + * * @public */ export class RuleTree implements Serializable { @@ -59,6 +87,20 @@ export class RuleTree implements Serializable { private constructor() {} + /** + * Add a bunch of rules to the tree. + * + * @remarks + * The rules are assumed to be: + * 1. all matching the same element; and + * 2. ordered in increasing cascade sort order (lower precedence rule first); and + * 3. be all the rules matching that element (this is not problematic). + * + * It is up to the caller to ensure this is true, as the tree itself cannot + * check that (notably, it has no access to the DOM tree to ensure the rule + * match the same element; nor to the origin or order of the rules to check + * cascade order). + */ public add( rules: Iterable<{ rule: Rule; @@ -74,6 +116,9 @@ export class RuleTree implements Serializable { // entry as the parent of the next rule to insert. This way, we gradually // build up a path of rule entries and then return the final entry to the // caller. + // Because all rules match the same element (by calling assumption), we + // do want to build them as a single path into the tree (baring some sharing). + // So each rule essentially creates a child of the preceding one. parent = Option.of( RuleTree.Node.add(rule, selector, declarations, children, parent), ); @@ -159,6 +204,14 @@ export namespace RuleTree { yield* this.ancestors(); } + /** + * Adds style rule to a potential node in the tree. Returns the node where + * the rule was added. + * + * @remarks Initially (for each element), the potential parent is None as + * it is possible to create a new tree in the forest. The forest itself + * is the children. + */ public static add( rule: Rule, selector: Selector, @@ -166,10 +219,23 @@ export namespace RuleTree { children: Array, parent: Option, ): Node { + // If we have already encountered the exact same selector (physical identity), + // we're done. + // This occurs when the exact same style rule matches several elements. + // The first element added to the rule tree will add that rule, subsequent + // one will just reuse it. (this also only occurs if the path so far in + // the rule tree has completely been shared so far). + // Notably, because it is the exact same selector, it controls the exact + // same rules, so all the information is already in the tree. if (parent.some((parent) => parent._selector === selector)) { return parent.get(); } + // Otherwise, if there is a child with a selector that looks the same, + // recursively add to it. + // This happens, e.g., when encountering two ".foo" selectors. They are + // then sorted by order of appearance (by assumption) and the later must + // be a descendant of the former as it has higher precedence. for (const child of children) { if (child._selector.equals(selector)) { return this.add( @@ -182,6 +248,9 @@ export namespace RuleTree { } } + // Otherwise, the selector is brand new (for this branch of the tree). + // Add it as a new child and return it (further rules in the same batch, + // matching the same element, should be added as its child. const node = Node.of(rule, selector, declarations, [], parent); children.push(node); diff --git a/packages/alfa-cascade/src/selector-map.ts b/packages/alfa-cascade/src/selector-map.ts index ab505f6367..f72df8416e 100644 --- a/packages/alfa-cascade/src/selector-map.ts +++ b/packages/alfa-cascade/src/selector-map.ts @@ -52,16 +52,16 @@ const isDescendantSelector = and( * Cascading origins defined in ascending order; origins defined first have * lower precedence than origins defined later. * - * {@link https://www.w3.org/TR/css-cascade/#cascading-origins} + * {@link https://www.w3.org/TR/css-cascade-5/#cascading-origins} */ enum Origin { /** - * {@link https://www.w3.org/TR/css-cascade/#cascade-origin-ua} + * {@link https://www.w3.org/TR/css-cascade-5/#cascade-origin-ua} */ UserAgent = 1, /** - * {@link https://www.w3.org/TR/css-cascade/#cascade-origin-author} + * {@link https://www.w3.org/TR/css-cascade-5/#cascade-origin-author} */ Author = 2, } From e3cf5727afc0d1520d7926c0d1828d8002ea68d6 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Tue, 12 Dec 2023 16:03:39 +0100 Subject: [PATCH 11/22] Improve rule tree structure --- packages/alfa-cascade/src/rule-tree.ts | 84 ++++++++++--------- packages/alfa-cascade/test/rule-tree.spec.ts | 17 ++-- .../test/nth-[last]-child.spec.tsx | 1 + 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index 53fae84b13..bd7963ff85 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -83,12 +83,14 @@ export class RuleTree implements Serializable { return new RuleTree(); } + // Keeping this allow a more streamlined tree vocabulary later on. + private readonly _root: Option = None; private readonly _children: Array = []; private constructor() {} /** - * Add a bunch of rules to the tree. + * Add a bunch of items to the tree. * * @remarks * The rules are assumed to be: @@ -101,17 +103,11 @@ export class RuleTree implements Serializable { * match the same element; nor to the origin or order of the rules to check * cascade order). */ - public add( - rules: Iterable<{ - rule: Rule; - selector: Selector; - declarations: Iterable; - }>, - ): Option { - let parent: Option = None; + public add(rules: Iterable): Option { + let parent = this._root; let children = this._children; - for (const { rule, selector, declarations } of rules) { + for (const item of rules) { // Insert the next rule into the current parent, using the returned rule // entry as the parent of the next rule to insert. This way, we gradually // build up a path of rule entries and then return the final entry to the @@ -119,9 +115,7 @@ export class RuleTree implements Serializable { // Because all rules match the same element (by calling assumption), we // do want to build them as a single path into the tree (baring some sharing). // So each rule essentially creates a child of the preceding one. - parent = Option.of( - RuleTree.Node.add(rule, selector, declarations, children, parent), - ); + parent = Option.of(RuleTree.Node.add(item, children, parent)); // parent was just build as a non-None Option. children = parent.getUnsafe().children; @@ -141,11 +135,31 @@ export class RuleTree implements Serializable { export namespace RuleTree { export type JSON = Array; + /** + * Items stored in rule tree nodes. + * + * @remarks + * Only the selector is used to actually build the structure. The rule and + * declarations are just data passed along to be used when resolving style. + */ + export interface Item { + rule: Rule; + selector: Selector; + declarations: Iterable; + } + + export namespace Item { + export interface JSON { + [key: string]: json.JSON; + rule: Rule.JSON; + selector: Selector.JSON; + declarations: Array; + } + } + export class Node implements Serializable { public static of( - rule: Rule, - selector: Selector, - declarations: Iterable, + { rule, selector, declarations }: Item, children: Array, parent: Option, ): Node { @@ -208,14 +222,14 @@ export namespace RuleTree { * Adds style rule to a potential node in the tree. Returns the node where * the rule was added. * - * @remarks Initially (for each element), the potential parent is None as + * @remarks + * + * Initially (for each element), the potential parent is None as * it is possible to create a new tree in the forest. The forest itself * is the children. */ public static add( - rule: Rule, - selector: Selector, - declarations: Iterable, + item: Item, children: Array, parent: Option, ): Node { @@ -227,7 +241,7 @@ export namespace RuleTree { // the rule tree has completely been shared so far). // Notably, because it is the exact same selector, it controls the exact // same rules, so all the information is already in the tree. - if (parent.some((parent) => parent._selector === selector)) { + if (parent.some((parent) => parent._selector === item.selector)) { return parent.get(); } @@ -237,21 +251,15 @@ export namespace RuleTree { // then sorted by order of appearance (by assumption) and the later must // be a descendant of the former as it has higher precedence. for (const child of children) { - if (child._selector.equals(selector)) { - return this.add( - rule, - selector, - declarations, - child._children, - Option.of(child), - ); + if (child._selector.equals(item.selector)) { + return this.add(item, child._children, Option.of(child)); } } // Otherwise, the selector is brand new (for this branch of the tree). // Add it as a new child and return it (further rules in the same batch, // matching the same element, should be added as its child. - const node = Node.of(rule, selector, declarations, [], parent); + const node = Node.of(item, [], parent); children.push(node); @@ -260,11 +268,13 @@ export namespace RuleTree { public toJSON(): Node.JSON { return { - rule: this._rule.toJSON(), - selector: this._selector.toJSON(), - declarations: [...this._declarations].map((declaration) => - declaration.toJSON(), - ), + item: { + rule: this._rule.toJSON(), + selector: this._selector.toJSON(), + declarations: [...this._declarations].map((declaration) => + declaration.toJSON(), + ), + }, children: this._children.map((node) => node.toJSON()), }; } @@ -273,9 +283,7 @@ export namespace RuleTree { export namespace Node { export interface JSON { [key: string]: json.JSON; - rule: Rule.JSON; - selector: Selector.JSON; - declarations: Array; + item: Item.JSON; children: Array; } } diff --git a/packages/alfa-cascade/test/rule-tree.spec.ts b/packages/alfa-cascade/test/rule-tree.spec.ts index 68fa1502f8..be42aefa71 100644 --- a/packages/alfa-cascade/test/rule-tree.spec.ts +++ b/packages/alfa-cascade/test/rule-tree.spec.ts @@ -1,16 +1,23 @@ /// -import { h } from "@siteimprove/alfa-dom"; +import { test } from "@siteimprove/alfa-test"; + +import { h, StyleRule } from "@siteimprove/alfa-dom"; import { None } from "@siteimprove/alfa-option"; import { Type } from "@siteimprove/alfa-selector"; -import { test } from "@siteimprove/alfa-test"; import { RuleTree } from "../src"; +function fakeRule(selector: string): StyleRule { + return h.rule.style(selector, []); +} + test(".of() builds a node", (t) => { const node = RuleTree.Node.of( - h.rule.style("div", { color: "red" }), - Type.of(None, "div"), - [], + { + rule: h.rule.style("div", { color: "red" }), + selector: Type.of(None, "div"), + declarations: [], + }, [], None, ); diff --git a/packages/alfa-selector/test/nth-[last]-child.spec.tsx b/packages/alfa-selector/test/nth-[last]-child.spec.tsx index 5508432abe..472139ac9c 100644 --- a/packages/alfa-selector/test/nth-[last]-child.spec.tsx +++ b/packages/alfa-selector/test/nth-[last]-child.spec.tsx @@ -23,6 +23,7 @@ test(".parse() accepts the `of selector` syntax", (t) => { namespace: null, name: "div", specificity: { a: 0, b: 0, c: 1 }, + key: "div", }, specificity: { a: 0, b: 1, c: 1 }, }); From 04223a0b00d1e595456a4d1d9d188321fef944d6 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 09:02:41 +0100 Subject: [PATCH 12/22] Improve root handling --- packages/alfa-cascade/src/rule-tree.ts | 35 +++++++++++++++++--------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index bd7963ff85..3a1bc14a56 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -106,8 +106,23 @@ export class RuleTree implements Serializable { public add(rules: Iterable): Option { let parent = this._root; let children = this._children; + let needNewBranch = true; for (const item of rules) { + // If the lowest precedence item has the same selector as one of the + // existing tree in the forest, then we can insert the items in this + // tree, so we select that child as parent for the next step. + for (const child of children) { + if (child.selector.equals(item.selector)) { + parent = Option.of(RuleTree.Node.add(item, child)); + needNewBranch = false; + } + } + + if (needNewBranch) { + parent = Option.of(RuleTree.Node.of(item, children, parent)); + } + // Insert the next rule into the current parent, using the returned rule // entry as the parent of the next rule to insert. This way, we gradually // build up a path of rule entries and then return the final entry to the @@ -115,7 +130,7 @@ export class RuleTree implements Serializable { // Because all rules match the same element (by calling assumption), we // do want to build them as a single path into the tree (baring some sharing). // So each rule essentially creates a child of the preceding one. - parent = Option.of(RuleTree.Node.add(item, children, parent)); + // parent = Option.of(RuleTree.Node.add(item, children, parent)); // parent was just build as a non-None Option. children = parent.getUnsafe().children; @@ -228,11 +243,7 @@ export namespace RuleTree { * it is possible to create a new tree in the forest. The forest itself * is the children. */ - public static add( - item: Item, - children: Array, - parent: Option, - ): Node { + public static add(item: Item, parent: Node): Node { // If we have already encountered the exact same selector (physical identity), // we're done. // This occurs when the exact same style rule matches several elements. @@ -241,8 +252,8 @@ export namespace RuleTree { // the rule tree has completely been shared so far). // Notably, because it is the exact same selector, it controls the exact // same rules, so all the information is already in the tree. - if (parent.some((parent) => parent._selector === item.selector)) { - return parent.get(); + if (parent._selector === item.selector) { + return parent; } // Otherwise, if there is a child with a selector that looks the same, @@ -250,18 +261,18 @@ export namespace RuleTree { // This happens, e.g., when encountering two ".foo" selectors. They are // then sorted by order of appearance (by assumption) and the later must // be a descendant of the former as it has higher precedence. - for (const child of children) { + for (const child of parent._children) { if (child._selector.equals(item.selector)) { - return this.add(item, child._children, Option.of(child)); + return Node.add(item, child); } } // Otherwise, the selector is brand new (for this branch of the tree). // Add it as a new child and return it (further rules in the same batch, // matching the same element, should be added as its child. - const node = Node.of(item, [], parent); + const node = Node.of(item, [], Option.of(parent)); - children.push(node); + parent._children.push(node); return node; } From 9261ecf377673bc4a05e0f11529b139ca895cb38 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 11:39:43 +0100 Subject: [PATCH 13/22] Update packages --- yarn.lock | 1 + 1 file changed, 1 insertion(+) diff --git a/yarn.lock b/yarn.lock index 86a34fd7c6..70bd78dfec 100644 --- a/yarn.lock +++ b/yarn.lock @@ -956,6 +956,7 @@ __metadata: "@siteimprove/alfa-predicate": "workspace:^0.69.0" "@siteimprove/alfa-refinement": "workspace:^0.69.0" "@siteimprove/alfa-selector": "workspace:^0.69.0" + "@siteimprove/alfa-test": "workspace:^0.69.0" languageName: unknown linkType: soft From d2667c3f389d442380d426e77920255232d9b2ed Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 12:36:38 +0100 Subject: [PATCH 14/22] Move .add to an instance method --- .changeset/four-parents-jog.md | 9 +++++ packages/alfa-cascade/src/rule-tree.ts | 32 +++++++++-------- packages/alfa-cascade/test/rule-tree.spec.ts | 2 +- packages/alfa-style/src/style.ts | 36 ++++++++++++++++++-- 4 files changed, 62 insertions(+), 17 deletions(-) create mode 100644 .changeset/four-parents-jog.md diff --git a/.changeset/four-parents-jog.md b/.changeset/four-parents-jog.md new file mode 100644 index 0000000000..4e0760b34a --- /dev/null +++ b/.changeset/four-parents-jog.md @@ -0,0 +1,9 @@ +--- +"@siteimprove/alfa-cascade": minor +--- + +**Breaking:** `RuleTree.add` and `RuleTree.Node.add` have been made internal. + +These have heavy assumptions on arguments in order to build a correct rule tree and are not intended for external use. Use `Cascade.of` to build correct cascade and rule trees. + +In addition, `RuleTree.Node.add` has been moved to an instance method, and its arguments have been simplified. diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index 3a1bc14a56..100422058b 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -96,30 +96,33 @@ export class RuleTree implements Serializable { * The rules are assumed to be: * 1. all matching the same element; and * 2. ordered in increasing cascade sort order (lower precedence rule first); and - * 3. be all the rules matching that element (this is not problematic). + * 3. be all the rules matching that element. * * It is up to the caller to ensure this is true, as the tree itself cannot - * check that (notably, it has no access to the DOM tree to ensure the rule + * check that (notably, it has no access to the DOM tree to ensure the rules * match the same element; nor to the origin or order of the rules to check * cascade order). + * + * @internal */ public add(rules: Iterable): Option { let parent = this._root; let children = this._children; - let needNewBranch = true; + let needNewTree = true; for (const item of rules) { // If the lowest precedence item has the same selector as one of the // existing tree in the forest, then we can insert the items in this // tree, so we select that child as parent for the next step. + // Otherwise, we create a new tree in the forest, and add the rule to it. for (const child of children) { if (child.selector.equals(item.selector)) { - parent = Option.of(RuleTree.Node.add(item, child)); - needNewBranch = false; + parent = Option.of(child.add(item)); + needNewTree = false; } } - if (needNewBranch) { + if (needNewTree) { parent = Option.of(RuleTree.Node.of(item, children, parent)); } @@ -130,7 +133,6 @@ export class RuleTree implements Serializable { // Because all rules match the same element (by calling assumption), we // do want to build them as a single path into the tree (baring some sharing). // So each rule essentially creates a child of the preceding one. - // parent = Option.of(RuleTree.Node.add(item, children, parent)); // parent was just build as a non-None Option. children = parent.getUnsafe().children; @@ -242,8 +244,10 @@ export namespace RuleTree { * Initially (for each element), the potential parent is None as * it is possible to create a new tree in the forest. The forest itself * is the children. + * + * @internal */ - public static add(item: Item, parent: Node): Node { + public add(item: Item): Node { // If we have already encountered the exact same selector (physical identity), // we're done. // This occurs when the exact same style rule matches several elements. @@ -252,8 +256,8 @@ export namespace RuleTree { // the rule tree has completely been shared so far). // Notably, because it is the exact same selector, it controls the exact // same rules, so all the information is already in the tree. - if (parent._selector === item.selector) { - return parent; + if (this._selector === item.selector) { + return this; } // Otherwise, if there is a child with a selector that looks the same, @@ -261,18 +265,18 @@ export namespace RuleTree { // This happens, e.g., when encountering two ".foo" selectors. They are // then sorted by order of appearance (by assumption) and the later must // be a descendant of the former as it has higher precedence. - for (const child of parent._children) { + for (const child of this._children) { if (child._selector.equals(item.selector)) { - return Node.add(item, child); + return child.add(item); } } // Otherwise, the selector is brand new (for this branch of the tree). // Add it as a new child and return it (further rules in the same batch, // matching the same element, should be added as its child. - const node = Node.of(item, [], Option.of(parent)); + const node = Node.of(item, [], Option.of(this)); - parent._children.push(node); + this._children.push(node); return node; } diff --git a/packages/alfa-cascade/test/rule-tree.spec.ts b/packages/alfa-cascade/test/rule-tree.spec.ts index be42aefa71..cce45c140a 100644 --- a/packages/alfa-cascade/test/rule-tree.spec.ts +++ b/packages/alfa-cascade/test/rule-tree.spec.ts @@ -16,7 +16,7 @@ test(".of() builds a node", (t) => { { rule: h.rule.style("div", { color: "red" }), selector: Type.of(None, "div"), - declarations: [], + declarations: [h.declaration("color", "red")], }, [], None, diff --git a/packages/alfa-style/src/style.ts b/packages/alfa-style/src/style.ts index 4164a33288..9c4a69f171 100644 --- a/packages/alfa-style/src/style.ts +++ b/packages/alfa-style/src/style.ts @@ -284,6 +284,14 @@ export namespace Style { const cache = Cache.empty>>(); + /** + * Build the style of an element. + * + * @remarks + * This gather all style declarations that apply to the element, in decreasing + * precedence (according to cascade sort order) and delegate the rest of the + * work to `Style.of`. + */ export function from( element: Element, device: Device, @@ -293,6 +301,17 @@ export namespace Style { .get(device, Cache.empty) .get(element.freeze(), Cache.empty) .get(context, () => { + // First, get all declarations on the `style` attribute. The win + // cascade sort at priority 3, trumping everything but origin and + // (shadow tree) context + // * origin is de-facto handled by the fact that these are author + // declarations, trumping non-important UA declaration at 1.6 vs 1.8. + // important UA declarations will win back through shouldOverride. + // important `style` attribute declarations incorrectly trump + // important UA declarations. + // {@link https://github.com/Siteimprove/alfa/issues/1532} + // * (shadow tree) context is not currently handled. + // {@link https://github.com/Siteimprove/alfa/issues/1533} const declarations: Array = element.style .map((block) => [...block.declarations].reverse()) .getOr([]); @@ -304,6 +323,14 @@ export namespace Style { let next = cascade.get(element, context); + // Walk up the cascade, starting from the node associated to the + // element, and gather all declarations met on the way. + // The cascade has been build in decreasing precedence as we move up + // (highest precedence rules are at the bottom), thus the declarations + // are seen in decreasing precedence and pushed to the end of the + // existing list which is thus also ordered in decreasing precedence. + // Cascade doesn't handle importance of declaration, hence this will + // still have to be done here (through `shouldOverride`). while (next.isSome()) { const node = next.get(); @@ -362,8 +389,13 @@ export namespace Style { * The "next" declaration should override the previous if: * - either there is no previous; or * - next is important and previous isn't. - * This suppose that the declarations have been pre--ordered in decreasing - * specificity. + * This supposes that the declarations have been pre--ordered in otherwise + * decreasing precedence. + * + * @privateRemarks + * This is not correct since importance of declaration reverses precedence of UA + * and author origins. + * { @link https://github.com/Siteimprove/alfa/issues/1532} * * @internal */ From 8db440e05d7cb7af1126714e7f63e8c22b6180d4 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 13:17:12 +0100 Subject: [PATCH 15/22] Root rule trees at a fake root. --- .changeset/tiny-eyes-attack.md | 7 +++ packages/alfa-cascade/src/cascade.ts | 4 +- packages/alfa-cascade/src/rule-tree.ts | 74 ++++++++++--------------- packages/alfa-rules/src/sia-r83/rule.ts | 17 +++--- packages/alfa-style/src/style.ts | 9 +-- 5 files changed, 49 insertions(+), 62 deletions(-) create mode 100644 .changeset/tiny-eyes-attack.md diff --git a/.changeset/tiny-eyes-attack.md b/.changeset/tiny-eyes-attack.md new file mode 100644 index 0000000000..d53a3d471e --- /dev/null +++ b/.changeset/tiny-eyes-attack.md @@ -0,0 +1,7 @@ +--- +"@siteimprove/alfa-cascade": minor +--- + +**Breaking:** `Cascade.get()` now returns a `RuleTree.Node` instead of an `Option`. + +`RuleTree` now have a fake root with no declarations, if no rule match the current element, `Cascade.get(element)` will return that fake root. diff --git a/packages/alfa-cascade/src/cascade.ts b/packages/alfa-cascade/src/cascade.ts index 90743ea759..e316123d82 100644 --- a/packages/alfa-cascade/src/cascade.ts +++ b/packages/alfa-cascade/src/cascade.ts @@ -37,7 +37,7 @@ export class Cascade implements Serializable { private readonly _entries = Cache.empty< Element, - Cache> + Cache >(); private constructor(root: Document | Shadow, device: Device) { @@ -76,7 +76,7 @@ export class Cascade implements Serializable { element: Element, context: Context = Context.empty(), filter: Option = None, - ): Option { + ): RuleTree.Node { return this._entries .get(element, Cache.empty) .get(context, () => diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index 100422058b..f547ed3b58 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -1,8 +1,8 @@ -import { Declaration, Rule } from "@siteimprove/alfa-dom"; +import { Declaration, h, Rule } from "@siteimprove/alfa-dom"; import { Iterable } from "@siteimprove/alfa-iterable"; import { Serializable } from "@siteimprove/alfa-json"; import { None, Option } from "@siteimprove/alfa-option"; -import { Selector } from "@siteimprove/alfa-selector"; +import { Selector, Universal } from "@siteimprove/alfa-selector"; import * as json from "@siteimprove/alfa-json"; @@ -51,7 +51,7 @@ import * as json from "@siteimprove/alfa-json"; * * Note that the resulting rule tree depends greatly on the order in which * rules are inserted, which must then be by increasing precedence. The `.foo` - * and `.bar` selectors are not directly comparable, the example above assumes + * and `.bar` selectors are not directly comparable; the example above assumes * that the `.bar` rule came later in the style sheet order and therefore wins * the cascade sort by "Order of Appearance". This information is not available * for the rule tree which relies on rules being fed to it in increasing @@ -68,13 +68,8 @@ import * as json from "@siteimprove/alfa-json"; * * @privateRemarks * The rules tree is actually a forest of nodes since many elements do not share - * any matched selector. We do not artificially root it as it would add little - * value, there is no natural root, and creating a fake root would actually - * make processing the tree harder (as we would need to handle that fake node). - * As a consequence, when inserting new rules in the tree, we may start a - * completely new tree in that forest. This means that rules may be inserted - * without a parent, and adding a single rule must be a static method of the - * Node class, rater than an instance method. + * any matched selector. We artificially root it at a fake node with no + * declarations, hence no impact on style. The fake root is not serialized. * * @public */ @@ -83,14 +78,22 @@ export class RuleTree implements Serializable { return new RuleTree(); } - // Keeping this allow a more streamlined tree vocabulary later on. - private readonly _root: Option = None; - private readonly _children: Array = []; + // Rooting the forest at a fake node with no declaration. + private readonly _root: RuleTree.Node = RuleTree.Node.of( + { + rule: h.rule.style("*", []), + selector: Universal.of(None), + declarations: [], + }, + [], + None, + ); private constructor() {} /** - * Add a bunch of items to the tree. + * Add a bunch of items to the tree. Returns the last node created, which is + * the highest precedence node for that list of items. * * @remarks * The rules are assumed to be: @@ -103,46 +106,23 @@ export class RuleTree implements Serializable { * match the same element; nor to the origin or order of the rules to check * cascade order). * + * @privateRemarks + * This is not stateless. Adding rules to a rule tree does mutate it! + * * @internal */ - public add(rules: Iterable): Option { + public add(rules: Iterable): RuleTree.Node { let parent = this._root; - let children = this._children; - let needNewTree = true; for (const item of rules) { - // If the lowest precedence item has the same selector as one of the - // existing tree in the forest, then we can insert the items in this - // tree, so we select that child as parent for the next step. - // Otherwise, we create a new tree in the forest, and add the rule to it. - for (const child of children) { - if (child.selector.equals(item.selector)) { - parent = Option.of(child.add(item)); - needNewTree = false; - } - } - - if (needNewTree) { - parent = Option.of(RuleTree.Node.of(item, children, parent)); - } - - // Insert the next rule into the current parent, using the returned rule - // entry as the parent of the next rule to insert. This way, we gradually - // build up a path of rule entries and then return the final entry to the - // caller. - // Because all rules match the same element (by calling assumption), we - // do want to build them as a single path into the tree (baring some sharing). - // So each rule essentially creates a child of the preceding one. - - // parent was just build as a non-None Option. - children = parent.getUnsafe().children; + parent = parent.add(item); } return parent; } public toJSON(): RuleTree.JSON { - return this._children.map((node) => node.toJSON()); + return this._root.children.map((node) => node.toJSON()); } } @@ -158,6 +138,10 @@ export namespace RuleTree { * @remarks * Only the selector is used to actually build the structure. The rule and * declarations are just data passed along to be used when resolving style. + * + * If the selector does not match the one in the rule, behavior is not specified. + * + * @internal */ export interface Item { rule: Rule; @@ -240,11 +224,13 @@ export namespace RuleTree { * the rule was added. * * @remarks - * * Initially (for each element), the potential parent is None as * it is possible to create a new tree in the forest. The forest itself * is the children. * + * @privateRemarks + * This is not stateless. Adding a rule to a node mutates the node! + * * @internal */ public add(item: Item): Node { diff --git a/packages/alfa-rules/src/sia-r83/rule.ts b/packages/alfa-rules/src/sia-r83/rule.ts index 593f654db4..f4b2939545 100644 --- a/packages/alfa-rules/src/sia-r83/rule.ts +++ b/packages/alfa-rules/src/sia-r83/rule.ts @@ -503,16 +503,13 @@ function getUsedMediaRules( return Sequence.empty(); } - return Cascade.of(root, device) - .get(element, context) - .map((node) => - // Get all nodes (style rules) in the RuleTree that affect the element; - // for each of these rules, get all ancestor media rules in the CSS tree. - ancestorsInRuleTree(node).flatMap((node) => - ancestorMediaRules(node.rule), - ), - ) - .getOrElse(Sequence.empty); + const node = Cascade.of(root, device).get(element, context); + + // Get all nodes (style rules) in the RuleTree that affect the element; + // for each of these rules, get all ancestor media rules in the CSS tree. + return ancestorsInRuleTree(node).flatMap((node) => + ancestorMediaRules(node.rule), + ); } function usesMediaRule( diff --git a/packages/alfa-style/src/style.ts b/packages/alfa-style/src/style.ts index 9c4a69f171..13d795e757 100644 --- a/packages/alfa-style/src/style.ts +++ b/packages/alfa-style/src/style.ts @@ -321,8 +321,6 @@ export namespace Style { if (Document.isDocument(root) || Shadow.isShadow(root)) { const cascade = Cascade.of(root, device); - let next = cascade.get(element, context); - // Walk up the cascade, starting from the node associated to the // element, and gather all declarations met on the way. // The cascade has been build in decreasing precedence as we move up @@ -331,11 +329,10 @@ export namespace Style { // existing list which is thus also ordered in decreasing precedence. // Cascade doesn't handle importance of declaration, hence this will // still have to be done here (through `shouldOverride`). - while (next.isSome()) { - const node = next.get(); - + for (const node of cascade + .get(element, context) + .inclusiveAncestors()) { declarations.push(...[...node.declarations].reverse()); - next = node.parent; } } From 708afe9e039ffc6020c08c6084a72f6b54e6de7f Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 14:25:06 +0100 Subject: [PATCH 16/22] Add rule tree tests --- packages/alfa-cascade/test/rule-tree.spec.ts | 239 +++++++++++++++++-- 1 file changed, 225 insertions(+), 14 deletions(-) diff --git a/packages/alfa-cascade/test/rule-tree.spec.ts b/packages/alfa-cascade/test/rule-tree.spec.ts index cce45c140a..e8bd9ef9d7 100644 --- a/packages/alfa-cascade/test/rule-tree.spec.ts +++ b/packages/alfa-cascade/test/rule-tree.spec.ts @@ -1,26 +1,237 @@ /// -import { test } from "@siteimprove/alfa-test"; - -import { h, StyleRule } from "@siteimprove/alfa-dom"; +import { h } from "@siteimprove/alfa-dom"; import { None } from "@siteimprove/alfa-option"; -import { Type } from "@siteimprove/alfa-selector"; +import { parse } from "@siteimprove/alfa-selector/test/parser"; +import { test } from "@siteimprove/alfa-test"; import { RuleTree } from "../src"; -function fakeRule(selector: string): StyleRule { - return h.rule.style(selector, []); +function fakeItem(selector: string): RuleTree.Item { + return { + rule: h.rule.style(selector, []), + selector: parse(selector), + declarations: [], + }; } +function fakeJSON(selector: string): RuleTree.Item.JSON { + const item = fakeItem(selector); + + return { + rule: item.rule.toJSON(), + selector: item.selector.toJSON(), + declarations: [], + }; +} + +/** + * Node tests + */ test(".of() builds a node", (t) => { - const node = RuleTree.Node.of( + const node = RuleTree.Node.of(fakeItem("div"), [], None); + + t.deepEqual(node.toJSON(), { + item: fakeJSON("div"), + children: [], + }); +}); + +test(".add() doesn't change a tree that already has the exact same selector", (t) => { + const item1 = fakeItem("div"); + const item2 = fakeItem("div"); + const node = RuleTree.Node.of(item1, [], None); + node.add({ ...item2, selector: item1.selector }); + + t.deepEqual(node.toJSON(), { + item: fakeJSON("div"), + children: [], + }); +}); + +test(".add() adds a child upon inserting identical selector", (t) => { + const node = RuleTree.Node.of(fakeItem("div"), [], None); + node.add(fakeItem("div")); + + t.deepEqual(node.toJSON(), { + item: fakeJSON("div"), + children: [{ item: fakeJSON("div"), children: [] }], + }); +}); + +test("Chaining .add() creates a single branch in the tree", (t) => { + // Selector `div`, `.foo`, `#bar`, matching, e.g., `
` + // and inserted in increasing specificity. + const node = RuleTree.Node.of(fakeItem("div"), [], None); + node.add(fakeItem(".foo")).add(fakeItem("#bar")); + + t.deepEqual(node.toJSON(), { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON("#bar"), children: [] }], + }, + ], + }); +}); + +/** + * Full rule tree tests + * + * Since rule tree are stateful, we need to recreate one for each test. + */ +test(".add() creates a single branch in the rule tree", (t) => { + const tree = RuleTree.empty(); + tree.add([fakeItem("div"), fakeItem(".foo"), fakeItem("#bar")]); + + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON("#bar"), children: [] }], + }, + ], + }, + ]); +}); + +test(".add() does not change the order of items", (t) => { + const tree = RuleTree.empty(); + // Items are not correctly ordered (#bar has higher specificity). Rule tree + // doesn't care. + tree.add([fakeItem("#bar"), fakeItem("div"), fakeItem(".foo")]); + + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("#bar"), + children: [ + { + item: fakeJSON("div"), + children: [{ item: fakeJSON(".foo"), children: [] }], + }, + ], + }, + ]); +}); + +test(".add() duplicate identical but distinct selectors", (t) => { + const tree = RuleTree.empty(); + // Presumably two rules with selector `div` at different place in the sheet. + tree.add([fakeItem("div"), fakeItem("div")]); + + t.deepEqual(tree.toJSON(), [ { - rule: h.rule.style("div", { color: "red" }), - selector: Type.of(None, "div"), - declarations: [h.declaration("color", "red")], + item: fakeJSON("div"), + children: [{ item: fakeJSON("div"), children: [] }], }, - [], - None, - ); + ]); +}); + +test(".add() creates separate trees for entries that don't share initial selectors", (t) => { + const tree = RuleTree.empty(); + // Matching `
+ tree.add([fakeItem("div"), fakeItem(".foo"), fakeItem("#bar")]); + // Matching `` + // Since the first selector differ, we cannot share any part of the tree + tree.add([fakeItem("span"), fakeItem(".foo"), fakeItem("#bar")]); + + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON("#bar"), children: [] }], + }, + ], + }, + { + item: fakeJSON("span"), + children: [ + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON("#bar"), children: [] }], + }, + ], + }, + ]); +}); + +test(".add() share branches as long as selectors are the same", (t) => { + const div = fakeItem("div"); + const foo = fakeItem(".foo"); + const tree = RuleTree.empty(); + tree.add([div, foo, fakeItem("#bar")]); + tree.add([div, foo, fakeItem(".baz")]); - console.dir(node.toJSON(), { depth: null }); + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [ + { item: fakeJSON("#bar"), children: [] }, + { item: fakeJSON(".baz"), children: [] }, + ], + }, + ], + }, + ]); +}); + +test(".add() adds descendants when selectors are identical", (t) => { + const div = fakeItem("div"); + const tree = RuleTree.empty(); + tree.add([div, fakeItem(".foo"), fakeItem("#bar")]); + tree.add([div, fakeItem(".foo"), fakeItem(".baz")]); + + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [ + { item: fakeJSON("#bar"), children: [] }, + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON(".baz"), children: [] }], + }, + ], + }, + ], + }, + ]); +}); + +test(".add() branches as soon as selectors differ", (t) => { + const div = fakeItem("div"); + const foo = fakeItem(".foo"); + const tree = RuleTree.empty(); + tree.add([div, foo, fakeItem("#bar")]); + tree.add([div, fakeItem(".baz"), foo]); + + t.deepEqual(tree.toJSON(), [ + { + item: fakeJSON("div"), + children: [ + { + item: fakeJSON(".foo"), + children: [{ item: fakeJSON("#bar"), children: [] }], + }, + { + item: fakeJSON(".baz"), + children: [ + { + item: fakeJSON(".foo"), + children: [], + }, + ], + }, + ], + }, + ]); }); From 6caeba457d149ce66708493dc8d524e396c9dee0 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 14:26:50 +0100 Subject: [PATCH 17/22] Post-merge cleanup --- packages/alfa-cascade/package.json | 2 +- yarn.lock | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/alfa-cascade/package.json b/packages/alfa-cascade/package.json index 7fd1f08805..e254083a00 100644 --- a/packages/alfa-cascade/package.json +++ b/packages/alfa-cascade/package.json @@ -32,7 +32,7 @@ "@siteimprove/alfa-selector": "workspace:^0.70.0" }, "devDependencies": { - "@siteimprove/alfa-test": "workspace:^0.69.0" + "@siteimprove/alfa-test": "workspace:^0.70.0" }, "publishConfig": { "access": "public", diff --git a/yarn.lock b/yarn.lock index 1440a32f57..42915ddc57 100644 --- a/yarn.lock +++ b/yarn.lock @@ -956,6 +956,7 @@ __metadata: "@siteimprove/alfa-predicate": "workspace:^0.70.0" "@siteimprove/alfa-refinement": "workspace:^0.70.0" "@siteimprove/alfa-selector": "workspace:^0.70.0" + "@siteimprove/alfa-test": "workspace:^0.70.0" languageName: unknown linkType: soft From 5343c231d6dc19fdf3345dca8b02263e13635180 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 13 Dec 2023 13:31:51 +0000 Subject: [PATCH 18/22] Extract API --- docs/review/api/alfa-cascade.api.md | 45 ++++++++++++++++++---------- docs/review/api/alfa-selector.api.md | 6 ++-- docs/review/api/alfa-style.api.md | 1 - 3 files changed, 33 insertions(+), 19 deletions(-) diff --git a/docs/review/api/alfa-cascade.api.md b/docs/review/api/alfa-cascade.api.md index b94813bf71..407bd13c3d 100644 --- a/docs/review/api/alfa-cascade.api.md +++ b/docs/review/api/alfa-cascade.api.md @@ -23,7 +23,7 @@ export class Cascade implements Serializable { // Warning: (ae-forgotten-export) The symbol "AncestorFilter" needs to be exported by the entry point index.d.ts // // (undocumented) - get(element: Element, context?: Context, filter?: Option): Option; + get(element: Element, context?: Context, filter?: Option): RuleTree.Node; // (undocumented) static of(node: Document | Shadow, device: Device): Cascade; // (undocumented) @@ -51,12 +51,8 @@ export namespace Cascade { // @public export class RuleTree implements Serializable { - // (undocumented) - add(rules: Iterable_2<{ - rule: Rule; - selector: Selector; - declarations: Iterable_2; - }>): Option; + // @internal + add(rules: Iterable_2): RuleTree.Node; // (undocumented) static empty(): RuleTree; // (undocumented) @@ -65,12 +61,35 @@ export class RuleTree implements Serializable { // @public (undocumented) export namespace RuleTree { + // @internal + export interface Item { + // (undocumented) + declarations: Iterable_2; + // (undocumented) + rule: Rule; + // (undocumented) + selector: Selector; + } + // (undocumented) + export namespace Item { + // (undocumented) + export interface JSON { + // (undocumented) + [key: string]: json.JSON; + // (undocumented) + declarations: Array; + // (undocumented) + rule: Rule.JSON; + // (undocumented) + selector: Selector.JSON; + } + } // (undocumented) export type JSON = Array; // (undocumented) export class Node implements Serializable { - // (undocumented) - static add(rule: Rule, selector: Selector, declarations: Iterable_2, children: Array, parent: Option): Node; + // @internal + add(item: Item): Node; // (undocumented) ancestors(): Iterable_2; // (undocumented) @@ -80,7 +99,7 @@ export namespace RuleTree { // (undocumented) inclusiveAncestors(): Iterable_2; // (undocumented) - static of(rule: Rule, selector: Selector, declarations: Iterable_2, children: Array, parent: Option): Node; + static of({ rule, selector, declarations }: Item, children: Array, parent: Option): Node; // (undocumented) get parent(): Option; // (undocumented) @@ -99,11 +118,7 @@ export namespace RuleTree { // (undocumented) children: Array; // (undocumented) - declarations: Array; - // (undocumented) - rule: Rule.JSON; - // (undocumented) - selector: Selector.JSON; + item: Item.JSON; } } } diff --git a/docs/review/api/alfa-selector.api.md b/docs/review/api/alfa-selector.api.md index b67cae4b6f..052ae4790e 100644 --- a/docs/review/api/alfa-selector.api.md +++ b/docs/review/api/alfa-selector.api.md @@ -192,7 +192,7 @@ export namespace Complex { right: Simple.JSON | Compound.JSON; } const // @internal (undocumented) - parseComplex: (parseSelector: Thunk>) => Parser, Compound | Simple | Complex, string, []>; + parseComplex: (parseSelector: Thunk>) => Parser, Simple | Compound | Complex, string, []>; } // @public (undocumented) @@ -229,7 +229,7 @@ export namespace Compound { selectors: Array_2; } const // @internal (undocumented) - parseCompound: (parseSelector: () => Parser, Absolute, string>) => Parser, Compound | Simple, string, []>; + parseCompound: (parseSelector: () => Parser, Absolute, string>) => Parser, Simple | Compound, string, []>; } // @public (undocumented) @@ -356,7 +356,7 @@ export namespace List { selectors: Array_2>; } const // @internal (undocumented) - parseList: (parseSelector: Thunk>) => Parser, List, string, []>; + parseList: (parseSelector: Thunk>) => Parser, List, string, []>; } // Warning: (ae-forgotten-export) The symbol "Active" needs to be exported by the entry point index.d.ts diff --git a/docs/review/api/alfa-style.api.md b/docs/review/api/alfa-style.api.md index 006f52590b..a504a47d4f 100644 --- a/docs/review/api/alfa-style.api.md +++ b/docs/review/api/alfa-style.api.md @@ -352,7 +352,6 @@ export namespace Style { export type Computed = Longhands.Computed; // (undocumented) export type Declared = Longhands.Declared; - // (undocumented) export function from(element: Element, device: Device, context?: Context): Style; // (undocumented) export type Inherited = Longhands.Inherited; From 507386f5f4859fd3866931b0e24892e69b8691b1 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 15:18:49 +0100 Subject: [PATCH 19/22] Clean up --- .changeset/tiny-eyes-attack.md | 2 +- docs/review/api/alfa-style.api.md | 58 +++++++++---------- packages/alfa-cascade/README.md | 8 +-- packages/alfa-cascade/src/ancestor-filter.ts | 7 ++- packages/alfa-cascade/src/rule-tree.ts | 33 +++++------ .../test/ancestor-filter.spec.tsx | 9 ++- packages/alfa-cascade/test/rule-tree.spec.ts | 15 +++-- packages/alfa-rules/src/sia-r83/rule.ts | 15 ++--- packages/alfa-style/src/style.ts | 4 +- 9 files changed, 78 insertions(+), 73 deletions(-) diff --git a/.changeset/tiny-eyes-attack.md b/.changeset/tiny-eyes-attack.md index d53a3d471e..5eda9824fe 100644 --- a/.changeset/tiny-eyes-attack.md +++ b/.changeset/tiny-eyes-attack.md @@ -4,4 +4,4 @@ **Breaking:** `Cascade.get()` now returns a `RuleTree.Node` instead of an `Option`. -`RuleTree` now have a fake root with no declarations, if no rule match the current element, `Cascade.get(element)` will return that fake root. +`RuleTree` now have a fake root with no declarations, if no rule matches the current element, `Cascade.get(element)` will return that fake root. diff --git a/docs/review/api/alfa-style.api.md b/docs/review/api/alfa-style.api.md index a504a47d4f..bc8c11634d 100644 --- a/docs/review/api/alfa-style.api.md +++ b/docs/review/api/alfa-style.api.md @@ -114,7 +114,7 @@ export namespace Longhands { readonly "background-attachment": Longhand, List>; readonly "background-clip": Longhand, List>; readonly "background-color": Longhand; - readonly "background-image": Longhand, List>>; + readonly "background-image": Longhand, List | Image.PartiallyResolved>>; readonly "background-origin": Longhand, List>; readonly "background-position-x": Longhand, List>>; readonly "background-position-y": Longhand, List>>; @@ -122,54 +122,54 @@ export namespace Longhands { readonly "background-repeat-y": Longhand, List>; readonly "background-size": Longhand, List, LengthPercentage | Keyword<"auto">]> | Keyword<"cover"> | Keyword<"contain">>>; readonly "border-block-end-color": Longhand; - readonly "border-block-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-block-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-block-end-width": Longhand; readonly "border-block-start-color": Longhand; - readonly "border-block-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-block-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-block-start-width": Longhand; readonly "border-bottom-color": Longhand; readonly "border-bottom-left-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-bottom-right-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; - readonly "border-bottom-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-bottom-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-bottom-width": Longhand; readonly "border-collapse": Longhand, Keyword.ToKeywords<"separate" | "collapse">>; readonly "border-end-end-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-end-start-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-image-outset": Longhand>; readonly "border-image-repeat": Longhand; - readonly "border-image-slice": Longhand | Tuple<[top: Number_2.Fixed | Percentage.Canonical, right: Number_2.Fixed | Percentage.Canonical, bottom: Number_2.Fixed | Percentage.Canonical, left: Number_2.Fixed | Percentage.Canonical, fill: Keyword<"fill">]>>; - readonly "border-image-source": Longhand>; + readonly "border-image-slice": Longhand | Tuple<[top: Percentage.Canonical | Number_2.Fixed, right: Percentage.Canonical | Number_2.Fixed, bottom: Percentage.Canonical | Number_2.Fixed, left: Percentage.Canonical | Number_2.Fixed, fill: Keyword<"fill">]>>; + readonly "border-image-source": Longhand | Image.PartiallyResolved>; readonly "border-image-width": Longhand, right: Number_2.Fixed | LengthPercentage | Keyword<"auto">, bottom: Number_2.Fixed | LengthPercentage | Keyword<"auto">, left: Number_2.Fixed | LengthPercentage | Keyword<"auto">]>>; readonly "border-inline-end-color": Longhand; - readonly "border-inline-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-inline-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-inline-end-width": Longhand; readonly "border-inline-start-color": Longhand; - readonly "border-inline-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-inline-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-inline-start-width": Longhand; readonly "border-left-color": Longhand; - readonly "border-left-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-left-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-left-width": Longhand; readonly "border-right-color": Longhand; - readonly "border-right-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-right-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-right-width": Longhand; readonly "border-start-end-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-start-start-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-top-color": Longhand; readonly "border-top-left-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-top-right-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; - readonly "border-top-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "border-top-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "border-top-width": Longhand; readonly bottom: Longhand, LengthPercentage | Keyword<"auto">>; readonly "box-shadow": Longhand | List>, Keyword<"none"> | List>; - readonly "clip-path": Longhand | Shape, URL | Keyword<"none"> | Shape>; + readonly "clip-path": Longhand | URL | Shape, Keyword<"none"> | URL | Shape>; readonly clip: Longhand | Shape, Keyword<"border-box">>, Keyword<"auto"> | Shape, Keyword<"border-box">>>; readonly color: Longhand; - readonly cursor: Longhand>, Keyword<"auto"> | Keyword<"none"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>, Tuple<[List>, Keyword<"auto"> | Keyword<"none"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>>; + readonly cursor: Longhand>, Keyword<"none"> | Keyword<"auto"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>, Tuple<[List>, Keyword<"none"> | Keyword<"auto"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>>; readonly display: Longhand | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">, listitem: Keyword<"list-item">]> | Tuple<[outside: Keyword<"table-row-group"> | Keyword<"table-header-group"> | Keyword<"table-footer-group"> | Keyword<"table-row"> | Keyword<"table-cell"> | Keyword<"table-column-group"> | Keyword<"table-column"> | Keyword<"table-caption"> | Keyword<"ruby-base"> | Keyword<"ruby-text"> | Keyword<"ruby-base-container"> | Keyword<"ruby-text-container">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[Keyword<"none"> | Keyword<"contents">]>, Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">, listitem: Keyword<"list-item">]> | Tuple<[outside: Keyword<"table-row-group"> | Keyword<"table-header-group"> | Keyword<"table-footer-group"> | Keyword<"table-row"> | Keyword<"table-cell"> | Keyword<"table-column-group"> | Keyword<"table-column"> | Keyword<"table-caption"> | Keyword<"ruby-base"> | Keyword<"ruby-text"> | Keyword<"ruby-base-container"> | Keyword<"ruby-text-container">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[Keyword<"none"> | Keyword<"contents">]>>; readonly "flex-direction": Longhand, Keyword.ToKeywords<"row" | "row-reverse" | "column" | "column-reverse">>; readonly "flex-wrap": Longhand, Keyword.ToKeywords<"nowrap" | "wrap" | "wrap-reverse">>; readonly float: Longhand, Keyword.ToKeywords<"none" | "left" | "right">>; - readonly "font-family": Longhand | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace">>, List | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace">>>; + readonly "font-family": Longhand | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace"> | String_2>, List | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace"> | String_2>>; readonly "font-size": Longhand | Keyword<"xx-small"> | Keyword<"x-small"> | Keyword<"small"> | Keyword<"large"> | Keyword<"x-large"> | Keyword<"xx-large"> | Keyword<"xxx-large"> | Keyword<"larger"> | Keyword<"smaller">, Length>; readonly "font-stretch": Longhand | Percentage.Fixed>; readonly "font-style": Longhand, Keyword.ToKeywords<"normal" | "italic" | "oblique">>; @@ -177,7 +177,7 @@ export namespace Longhands { readonly "font-variant-east-asian": Longhand; readonly "font-variant-ligatures": Longhand; readonly "font-variant-numeric": Longhand; - readonly "font-variant-position": Longhand, Keyword.ToKeywords<"sub" | "normal" | "super">>; + readonly "font-variant-position": Longhand, Keyword.ToKeywords<"normal" | "sub" | "super">>; readonly "font-weight": Longhand | Keyword<"bold"> | Keyword<"bolder"> | Keyword<"lighter">, Number_2.Fixed>; readonly height: Longhand, LengthPercentage | Keyword<"auto">>; readonly "inset-block-end": Longhand, LengthPercentage | Keyword<"auto">>; @@ -186,29 +186,29 @@ export namespace Longhands { readonly "inset-inline-start": Longhand, LengthPercentage | Keyword<"auto">>; readonly left: Longhand, LengthPercentage | Keyword<"auto">>; readonly "letter-spacing": Longhand, Length>; - readonly "line-height": Longhand, Computed>; - readonly "margin-bottom": Longhand, Length | Percentage | Keyword<"auto">>; - readonly "margin-left": Longhand, Length | Percentage | Keyword<"auto">>; - readonly "margin-right": Longhand, Length | Percentage | Keyword<"auto">>; - readonly "margin-top": Longhand, Length | Percentage | Keyword<"auto">>; - readonly "min-height": Longhand | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Percentage | Keyword<"auto"> | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; - readonly "min-width": Longhand | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Percentage | Keyword<"auto"> | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; + readonly "line-height": Longhand, Computed>; + readonly "margin-bottom": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; + readonly "margin-left": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; + readonly "margin-right": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; + readonly "margin-top": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; + readonly "min-height": Longhand | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Keyword<"auto"> | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; + readonly "min-width": Longhand | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Keyword<"auto"> | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; readonly opacity: Longhand, Number_2.Fixed>; readonly "outline-color": Longhand, Color.Canonical | Keyword<"invert">>; readonly "outline-offset": Longhand, Length>; - readonly "outline-style": Longhand, Keyword.ToKeywords<"none" | "inset" | "auto" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; + readonly "outline-style": Longhand, Keyword.ToKeywords<"none" | "auto" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; readonly "outline-width": Longhand | Keyword<"medium"> | Keyword<"thick">, Length>; - readonly "overflow-x": Longhand, Keyword<"auto"> | Keyword<"scroll"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; - readonly "overflow-y": Longhand, Keyword<"auto"> | Keyword<"scroll"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; - readonly position: Longhand, Keyword.ToKeywords<"fixed" | "relative" | "static" | "absolute" | "sticky">>; + readonly "overflow-x": Longhand, Keyword<"scroll"> | Keyword<"auto"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; + readonly "overflow-y": Longhand, Keyword<"scroll"> | Keyword<"auto"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; + readonly position: Longhand, Keyword.ToKeywords<"fixed" | "static" | "relative" | "absolute" | "sticky">>; readonly right: Longhand, LengthPercentage | Keyword<"auto">>; - readonly rotate: Longhand, Rotate | Keyword<"none">>; + readonly rotate: Longhand | Rotate, Keyword<"none"> | Rotate>; readonly "text-align": Longhand, Keyword.ToKeywords<"left" | "right" | "center" | "start" | "end" | "justify">>; readonly "text-decoration-color": Longhand; readonly "text-decoration-line": Longhand | List | Keyword<"overline"> | Keyword<"line-through"> | Keyword<"blink">>, Keyword<"none"> | List | Keyword<"overline"> | Keyword<"line-through"> | Keyword<"blink">>>; readonly "text-decoration-style": Longhand, Keyword.ToKeywords<"dotted" | "dashed" | "solid" | "double" | "wavy">>; readonly "text-decoration-thickness": Longhand | Keyword<"from-font">, Length | Keyword<"auto"> | Keyword<"from-font">>; - readonly "text-indent": Longhand | LengthPercentage | Percentage.Calculated<"length"> | Percentage.Fixed<"length">, LengthPercentage>; + readonly "text-indent": Longhand | Length | Percentage.Calculated<"length"> | Length, LengthPercentage>; readonly "text-overflow": Longhand, Keyword.ToKeywords<"clip" | "ellipsis">>; readonly "text-shadow": Longhand | List>, Keyword<"none"> | List>; readonly "text-transform": Longhand, Keyword.ToKeywords<"none" | "capitalize" | "uppercase" | "lowercase">>; @@ -295,7 +295,7 @@ export namespace Shorthands { readonly "font-variant": Shorthand<"font-variant-caps" | "font-variant-east-asian" | "font-variant-ligatures" | "font-variant-numeric">; readonly "inset-block": Shorthand<"inset-block-end" | "inset-block-start">; readonly "inset-inline": Shorthand<"inset-inline-end" | "inset-inline-start">; - readonly inset: Shorthand<"top" | "bottom" | "left" | "right">; + readonly inset: Shorthand<"left" | "right" | "top" | "bottom">; readonly margin: Shorthand<"margin-bottom" | "margin-left" | "margin-right" | "margin-top">; readonly outline: Shorthand<"outline-color" | "outline-style" | "outline-width">; readonly overflow: Shorthand<"overflow-x" | "overflow-y">; diff --git a/packages/alfa-cascade/README.md b/packages/alfa-cascade/README.md index f729182a1e..d11480bd87 100644 --- a/packages/alfa-cascade/README.md +++ b/packages/alfa-cascade/README.md @@ -2,7 +2,7 @@ This package builds the cascade which is then used by `@siteimprove/alfa-style` in order to find the cascaded value of each property. -While resolving the cascade is in theory somewhat easy (for each element and property, find the highest precedence declaration), it requires a lot of work. A typical page contain hundreds of element and possibly thousands of style rules, and we support nearly 150 properties. So, we cannot just brute force our way through this and need structures to help eliminate quickly most of the selectors. This is especially true for the descendants and sibling selectors whose matching require traversing the DOM tree, potentially at a far away place for something like `main .foo` +While resolving the cascade is in theory somewhat easy (for each element and property, find the highest precedence declaration), it requires a lot of work. A typical page contain hundreds of elements and possibly thousands of style rules, and we support nearly 150 properties. So, we cannot just brute force our way through this and need structures to help eliminate quickly most of the selectors. This is especially true for the descendants and sibling selectors whose matching require traversing the DOM tree, potentially at a far away place for something like `main .foo` ## Ancestor filter @@ -10,13 +10,13 @@ The ancestor filter is a structure to optimize matching of descendants selectors In order to be compact and efficient, we just count the number of each type, class, and id on the path to the element. So, for example, a `div.foo .bar` selector cannot match if there is no `div` type or `.foo` class along the path. We cannot just keep a boolean because we want to be able to update the ancestor filter during the "upward moves" of the traversal, which requires removing elements from it, so we need a precise count to figure out when it reaches 0. -The ancestor filter only allow for guaranteed "won't match" answers, because the type, class and id have been separated for the sake of compactness. For example, a `div.foo .bar` selector won't match if the `div` and `.foo` ancestors are different, but the ancestor filter doesn't hold that information. However, the filter greatly reduce the actual number of elements to test against each descendant selector and thus the amount of work to be done. +The ancestor filter only allows for guaranteed "won't match" answers, because the type, class and id have been separated for the sake of compactness. For example, a `div.foo .bar` selector won't match if the `div` and `.foo` ancestors are different, but the ancestor filter doesn't hold that information. However, the filter greatly reduce the actual number of elements to test against each descendant selector and thus the amount of work to be done. ## Key selector and Selector map The other tool to speed up matching of complex (and compound) selectors is the selector map. -Each selector is associated to a _key selector_ which is the part which is going to be matched against the element itself (not against its siblings or ancestors). For complex selector, the key selector is thus the rightmost bit. For compound selector, it could be any bit; we take the leftmost bit assuming that they are ordered significantly by authors. +Each selector is associated to a _key selector_ which is the part which is going to be matched against the element itself (not against its siblings or ancestors). For complex selectors, the key selector is thus the rightmost selector. For compound selectors, it could be any selector; we take the leftmost one (mostly to limit the risk of key selector being pseudo-classes or -elements; key selectors are not really built for these). That is, in a `div.foo .bar` selector, the key selector is `.bar`. Any element that matches the full `div.foo .bar` selector must necessarily be a `.bar` itself (plus some DOM tree context). For anything else, we don't need to look at DOM structure. Similarly, in the `div.foo` selector, the key selector is `div`. @@ -32,4 +32,4 @@ Using a tree, rather than a separated list for each element allows to share the ## Cascade -The cascade itself is a rule tree associated with a map from elements to nodes in it. Each element is mapped to its highest precedence selector in the rule tree. Thus, in order to find the cascaded value of any property for a given element, we can simply walk up the rule tree until we find a selector (and associated rule) declaring that property. Since we've walk up the lattice from the smallest possible selector (highest precedence), this will be the cascaded value, no matter if more rules up the tree also define this property. +The cascade itself is a rule tree associated with a map from elements to nodes in it. Each element is mapped to its highest precedence selector in the rule tree. Thus, in order to find the cascaded value of any property for a given element, we can simply walk up the rule tree until we find a selector (and associated rule) declaring that property. Since we've walk up the tree from the highest possible precedence to the lowest, this will be the cascaded value, no matter if more rules up the tree also define this property. diff --git a/packages/alfa-cascade/src/ancestor-filter.ts b/packages/alfa-cascade/src/ancestor-filter.ts index 58e981d0f5..74914a0b1c 100644 --- a/packages/alfa-cascade/src/ancestor-filter.ts +++ b/packages/alfa-cascade/src/ancestor-filter.ts @@ -18,7 +18,7 @@ import * as json from "@siteimprove/alfa-json"; * The ancestor filter simply count the number of each ID, class, and type * amongst the path walked so far. When a descendant selector is encountered, we * can quickly see if the ancestor filter contains the ID, class, or type of the - * ancestor bit, without walking up the full tree again. + * ancestor part, without walking up the full tree again. * * We need to remember exact count rather than just existence because the * initial build of the cascade traverses the tree in depth-first order and @@ -38,6 +38,11 @@ import * as json from "@siteimprove/alfa-json"; * Given a selector `main b`, we can therefore reject that the selector would * match the `` as the ancestor filter does not contain the type `main`. * + * However, given a selector `section.highlight`, the ancestor filter can only + * tell that it **may** match the `` element. In this case, it doesn't. So, + * the filter acts as a quick guaranteed rejection mechanism, but actual match + * test is needed to have an accurate final result. + * * NB: None of the operations of the ancestor filter are idempotent to avoid * keeping track of more information than strictly necessary. This is however * not a problem when ancestor filters are used during top-down traversal of the diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index f547ed3b58..118a8302e5 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -29,15 +29,15 @@ import * as json from "@siteimprove/alfa-json"; * * "div" * +-- ".foo" - * +-- ".foo[href]" <- A + * +-- ".foo[href]" (A) * * We then associate rule `".foo[href]"` with element A and insert the matched * rules for element B into the tree: * * "div" * +-- ".foo" - * +-- ".foo[href]" <- A - * +-- ".bar" <- B + * +-- ".foo[href]" (A) + * +-- ".bar" (B) * * We then associate the rule `".bar"` with element B, and we're done. Notice how * the tree branches at rule `".foo"`, allowing the two elements to share the @@ -60,9 +60,9 @@ import * as json from "@siteimprove/alfa-json"; * * "div" * +-- ".foo" - * +-- ".foo[href]" <- A + * +-- ".foo[href]" (A) * +-- ".bar" - * +-- ".foo" <- B + * +-- ".foo" (B) * * {@link http://doc.servo.org/style/rule_tree/struct.RuleTree.html} * @@ -107,7 +107,7 @@ export class RuleTree implements Serializable { * cascade order). * * @privateRemarks - * This is not stateless. Adding rules to a rule tree does mutate it! + * This is stateful. Adding rules to a rule tree does mutate it! * * @internal */ @@ -115,6 +115,10 @@ export class RuleTree implements Serializable { let parent = this._root; for (const item of rules) { + // Insert the next rule into the current parent, using the returned rule + // entry as the parent of the next rule to insert. This way, we gradually + // build up a path of rule entries and then return the final entry to the + // caller. parent = parent.add(item); } @@ -220,16 +224,11 @@ export namespace RuleTree { } /** - * Adds style rule to a potential node in the tree. Returns the node where - * the rule was added. - * - * @remarks - * Initially (for each element), the potential parent is None as - * it is possible to create a new tree in the forest. The forest itself - * is the children. + * Adds style rule to a node in the tree. Returns the node where the rule + * was added. * * @privateRemarks - * This is not stateless. Adding a rule to a node mutates the node! + * This is stateful. Adding a rule to a node mutates the node! * * @internal */ @@ -238,15 +237,15 @@ export namespace RuleTree { // we're done. // This occurs when the exact same style rule matches several elements. // The first element added to the rule tree will add that rule, subsequent - // one will just reuse it. (this also only occurs if the path so far in - // the rule tree has completely been shared so far). + // ones will just reuse it (if the path so far in the rule tree has + // completely been shared). // Notably, because it is the exact same selector, it controls the exact // same rules, so all the information is already in the tree. if (this._selector === item.selector) { return this; } - // Otherwise, if there is a child with a selector that looks the same, + // Otherwise, if there is a child with an identical but separate selector, // recursively add to it. // This happens, e.g., when encountering two ".foo" selectors. They are // then sorted by order of appearance (by assumption) and the later must diff --git a/packages/alfa-cascade/test/ancestor-filter.spec.tsx b/packages/alfa-cascade/test/ancestor-filter.spec.tsx index 1e18455feb..4f5fa2f4f4 100644 --- a/packages/alfa-cascade/test/ancestor-filter.spec.tsx +++ b/packages/alfa-cascade/test/ancestor-filter.spec.tsx @@ -2,6 +2,8 @@ import { Lexer } from "@siteimprove/alfa-css"; import { Selector } from "@siteimprove/alfa-selector"; import { Assertions, test } from "@siteimprove/alfa-test"; +import { parse } from "@siteimprove/alfa-selector/test/parser"; + import { AncestorFilter, Bucket } from "../src/ancestor-filter"; test("Buckets behave as expected", (t) => { @@ -71,10 +73,6 @@ test("Buckets behave as expected", (t) => { t(!bucket.has("b")); }); -function parse(selector: string): Selector { - return Selector.parse(Lexer.lex(selector)).getUnsafe()[1]; -} - const selectors = { divSel: parse("div"), spanSel: parse("span"), @@ -122,7 +120,8 @@ function match( * Since only element's name, id and class matter (not the actual element), we * create them on the fly and remove a structurally identical one rather than the * exact one that has been added. This simplifies the test a bit. Actual cascading - * walk through the actual DOM tree and remove the exact same element when moving up. + * walks through the actual DOM tree and removes the exact same element when + * moving up. */ test("Ancestor filter behaves as expected", (t) => { const filter = AncestorFilter.empty(); diff --git a/packages/alfa-cascade/test/rule-tree.spec.ts b/packages/alfa-cascade/test/rule-tree.spec.ts index e8bd9ef9d7..87dec15065 100644 --- a/packages/alfa-cascade/test/rule-tree.spec.ts +++ b/packages/alfa-cascade/test/rule-tree.spec.ts @@ -1,9 +1,9 @@ -/// import { h } from "@siteimprove/alfa-dom"; import { None } from "@siteimprove/alfa-option"; -import { parse } from "@siteimprove/alfa-selector/test/parser"; import { test } from "@siteimprove/alfa-test"; +import { parse } from "@siteimprove/alfa-selector/test/parser"; + import { RuleTree } from "../src"; function fakeItem(selector: string): RuleTree.Item { @@ -59,7 +59,7 @@ test(".add() adds a child upon inserting identical selector", (t) => { }); test("Chaining .add() creates a single branch in the tree", (t) => { - // Selector `div`, `.foo`, `#bar`, matching, e.g., `
` + // Selectors `div`, `.foo`, `#bar`, matching, e.g., `
` // and inserted in increasing specificity. const node = RuleTree.Node.of(fakeItem("div"), [], None); node.add(fakeItem(".foo")).add(fakeItem("#bar")); @@ -182,10 +182,16 @@ test(".add() share branches as long as selectors are the same", (t) => { ]); }); -test(".add() adds descendants when selectors are identical", (t) => { +test(".add() adds descendants when selectors are merely identical", (t) => { const div = fakeItem("div"); const tree = RuleTree.empty(); tree.add([div, fakeItem(".foo"), fakeItem("#bar")]); + // This is not an actual possible case. This corresponds to two `.foo` + // selectors but each matches different elements, which is impossible. + // Hence, the adding of the .foo / .baz branch under the initial .foo + // looks very wrong but is actually the correct thing to do. In an actual + // case, both .add would contain both .foo selector, since this rules with + // identical selectors match the same elements. tree.add([div, fakeItem(".foo"), fakeItem(".baz")]); t.deepEqual(tree.toJSON(), [ @@ -212,6 +218,7 @@ test(".add() branches as soon as selectors differ", (t) => { const foo = fakeItem(".foo"); const tree = RuleTree.empty(); tree.add([div, foo, fakeItem("#bar")]); + // Even if the selector is the same, the tree doesn't try to merge the branches. tree.add([div, fakeItem(".baz"), foo]); t.deepEqual(tree.toJSON(), [ diff --git a/packages/alfa-rules/src/sia-r83/rule.ts b/packages/alfa-rules/src/sia-r83/rule.ts index f4b2939545..479f88eeb6 100644 --- a/packages/alfa-rules/src/sia-r83/rule.ts +++ b/packages/alfa-rules/src/sia-r83/rule.ts @@ -4,12 +4,12 @@ import { Cascade, RuleTree } from "@siteimprove/alfa-cascade"; import { Length } from "@siteimprove/alfa-css"; import { Device } from "@siteimprove/alfa-device"; import { - Rule as CSSRule, Document, Element, MediaRule, Namespace, Node, + Rule as CSSRule, Text, } from "@siteimprove/alfa-dom"; import { Hash } from "@siteimprove/alfa-hash"; @@ -485,10 +485,7 @@ const ruleTreeCache = Cache.empty>(); function ancestorsInRuleTree(rule: RuleTree.Node): Sequence { return ruleTreeCache.get(rule, () => - rule.parent - .map((parent) => ancestorsInRuleTree(parent)) - .getOrElse>(Sequence.empty) - .prepend(rule), + Sequence.from(rule.inclusiveAncestors()), ); } @@ -503,13 +500,11 @@ function getUsedMediaRules( return Sequence.empty(); } - const node = Cascade.of(root, device).get(element, context); - // Get all nodes (style rules) in the RuleTree that affect the element; // for each of these rules, get all ancestor media rules in the CSS tree. - return ancestorsInRuleTree(node).flatMap((node) => - ancestorMediaRules(node.rule), - ); + return ancestorsInRuleTree( + Cascade.of(root, device).get(element, context), + ).flatMap((node) => ancestorMediaRules(node.rule)); } function usesMediaRule( diff --git a/packages/alfa-style/src/style.ts b/packages/alfa-style/src/style.ts index 13d795e757..e06e72b8c4 100644 --- a/packages/alfa-style/src/style.ts +++ b/packages/alfa-style/src/style.ts @@ -301,7 +301,7 @@ export namespace Style { .get(device, Cache.empty) .get(element.freeze(), Cache.empty) .get(context, () => { - // First, get all declarations on the `style` attribute. The win + // First, get all declarations on the `style` attribute. They win // cascade sort at priority 3, trumping everything but origin and // (shadow tree) context // * origin is de-facto handled by the fact that these are author @@ -392,7 +392,7 @@ export namespace Style { * @privateRemarks * This is not correct since importance of declaration reverses precedence of UA * and author origins. - * { @link https://github.com/Siteimprove/alfa/issues/1532} + * {@link https://github.com/Siteimprove/alfa/issues/1532} * * @internal */ From 433db540c8467de3b575244ec42224d816885d59 Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 13 Dec 2023 15:35:21 +0100 Subject: [PATCH 20/22] Clean up --- packages/alfa-cascade/test/ancestor-filter.spec.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/alfa-cascade/test/ancestor-filter.spec.tsx b/packages/alfa-cascade/test/ancestor-filter.spec.tsx index 4f5fa2f4f4..be1c86e3d1 100644 --- a/packages/alfa-cascade/test/ancestor-filter.spec.tsx +++ b/packages/alfa-cascade/test/ancestor-filter.spec.tsx @@ -1,5 +1,3 @@ -import { Lexer } from "@siteimprove/alfa-css"; -import { Selector } from "@siteimprove/alfa-selector"; import { Assertions, test } from "@siteimprove/alfa-test"; import { parse } from "@siteimprove/alfa-selector/test/parser"; From 0b658e2f27420340ccaca06a4f013e8a046cc4b6 Mon Sep 17 00:00:00 2001 From: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 13 Dec 2023 14:38:27 +0000 Subject: [PATCH 21/22] Extract API --- docs/review/api/alfa-style.api.md | 58 +++++++++++++++---------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/docs/review/api/alfa-style.api.md b/docs/review/api/alfa-style.api.md index bc8c11634d..a504a47d4f 100644 --- a/docs/review/api/alfa-style.api.md +++ b/docs/review/api/alfa-style.api.md @@ -114,7 +114,7 @@ export namespace Longhands { readonly "background-attachment": Longhand, List>; readonly "background-clip": Longhand, List>; readonly "background-color": Longhand; - readonly "background-image": Longhand, List | Image.PartiallyResolved>>; + readonly "background-image": Longhand, List>>; readonly "background-origin": Longhand, List>; readonly "background-position-x": Longhand, List>>; readonly "background-position-y": Longhand, List>>; @@ -122,54 +122,54 @@ export namespace Longhands { readonly "background-repeat-y": Longhand, List>; readonly "background-size": Longhand, List, LengthPercentage | Keyword<"auto">]> | Keyword<"cover"> | Keyword<"contain">>>; readonly "border-block-end-color": Longhand; - readonly "border-block-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-block-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-block-end-width": Longhand; readonly "border-block-start-color": Longhand; - readonly "border-block-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-block-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-block-start-width": Longhand; readonly "border-bottom-color": Longhand; readonly "border-bottom-left-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-bottom-right-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; - readonly "border-bottom-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-bottom-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-bottom-width": Longhand; readonly "border-collapse": Longhand, Keyword.ToKeywords<"separate" | "collapse">>; readonly "border-end-end-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-end-start-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-image-outset": Longhand>; readonly "border-image-repeat": Longhand; - readonly "border-image-slice": Longhand | Tuple<[top: Percentage.Canonical | Number_2.Fixed, right: Percentage.Canonical | Number_2.Fixed, bottom: Percentage.Canonical | Number_2.Fixed, left: Percentage.Canonical | Number_2.Fixed, fill: Keyword<"fill">]>>; - readonly "border-image-source": Longhand | Image.PartiallyResolved>; + readonly "border-image-slice": Longhand | Tuple<[top: Number_2.Fixed | Percentage.Canonical, right: Number_2.Fixed | Percentage.Canonical, bottom: Number_2.Fixed | Percentage.Canonical, left: Number_2.Fixed | Percentage.Canonical, fill: Keyword<"fill">]>>; + readonly "border-image-source": Longhand>; readonly "border-image-width": Longhand, right: Number_2.Fixed | LengthPercentage | Keyword<"auto">, bottom: Number_2.Fixed | LengthPercentage | Keyword<"auto">, left: Number_2.Fixed | LengthPercentage | Keyword<"auto">]>>; readonly "border-inline-end-color": Longhand; - readonly "border-inline-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-inline-end-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-inline-end-width": Longhand; readonly "border-inline-start-color": Longhand; - readonly "border-inline-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-inline-start-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-inline-start-width": Longhand; readonly "border-left-color": Longhand; - readonly "border-left-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-left-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-left-width": Longhand; readonly "border-right-color": Longhand; - readonly "border-right-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-right-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-right-width": Longhand; readonly "border-start-end-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-start-start-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-top-color": Longhand; readonly "border-top-left-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; readonly "border-top-right-radius": Longhand, Tuple<[horizontal: LengthPercentage, vertical: LengthPercentage]>>; - readonly "border-top-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "border-top-style": Longhand, Keyword.ToKeywords<"none" | "hidden" | "inset" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "border-top-width": Longhand; readonly bottom: Longhand, LengthPercentage | Keyword<"auto">>; readonly "box-shadow": Longhand | List>, Keyword<"none"> | List>; - readonly "clip-path": Longhand | URL | Shape, Keyword<"none"> | URL | Shape>; + readonly "clip-path": Longhand | Shape, URL | Keyword<"none"> | Shape>; readonly clip: Longhand | Shape, Keyword<"border-box">>, Keyword<"auto"> | Shape, Keyword<"border-box">>>; readonly color: Longhand; - readonly cursor: Longhand>, Keyword<"none"> | Keyword<"auto"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>, Tuple<[List>, Keyword<"none"> | Keyword<"auto"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>>; + readonly cursor: Longhand>, Keyword<"auto"> | Keyword<"none"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>, Tuple<[List>, Keyword<"auto"> | Keyword<"none"> | Keyword<"default"> | Keyword<"context-menu"> | Keyword<"help"> | Keyword<"pointer"> | Keyword<"progress"> | Keyword<"wait"> | Keyword<"cell"> | Keyword<"crosshair"> | Keyword<"text"> | Keyword<"vertical-text"> | Keyword<"alias"> | Keyword<"copy"> | Keyword<"move"> | Keyword<"no-drop"> | Keyword<"not-allowed"> | Keyword<"grab"> | Keyword<"grabbing"> | Keyword<"e-resize"> | Keyword<"n-resize"> | Keyword<"ne-resize"> | Keyword<"nw-resize"> | Keyword<"s-resize"> | Keyword<"se-resize"> | Keyword<"sw-resize"> | Keyword<"w-resize"> | Keyword<"ew-resize"> | Keyword<"ns-resize"> | Keyword<"nesw-resize"> | Keyword<"nwse-resize"> | Keyword<"col-resize"> | Keyword<"row-resize"> | Keyword<"all-scroll"> | Keyword<"zoom-in"> | Keyword<"zoom-out">]>>; readonly display: Longhand | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">, listitem: Keyword<"list-item">]> | Tuple<[outside: Keyword<"table-row-group"> | Keyword<"table-header-group"> | Keyword<"table-footer-group"> | Keyword<"table-row"> | Keyword<"table-cell"> | Keyword<"table-column-group"> | Keyword<"table-column"> | Keyword<"table-caption"> | Keyword<"ruby-base"> | Keyword<"ruby-text"> | Keyword<"ruby-base-container"> | Keyword<"ruby-text-container">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[Keyword<"none"> | Keyword<"contents">]>, Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[outside: Keyword<"block"> | Keyword<"inline"> | Keyword<"run-in">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">, listitem: Keyword<"list-item">]> | Tuple<[outside: Keyword<"table-row-group"> | Keyword<"table-header-group"> | Keyword<"table-footer-group"> | Keyword<"table-row"> | Keyword<"table-cell"> | Keyword<"table-column-group"> | Keyword<"table-column"> | Keyword<"table-caption"> | Keyword<"ruby-base"> | Keyword<"ruby-text"> | Keyword<"ruby-base-container"> | Keyword<"ruby-text-container">, inside: Keyword<"flow"> | Keyword<"flow-root"> | Keyword<"table"> | Keyword<"flex"> | Keyword<"grid"> | Keyword<"ruby">]> | Tuple<[Keyword<"none"> | Keyword<"contents">]>>; readonly "flex-direction": Longhand, Keyword.ToKeywords<"row" | "row-reverse" | "column" | "column-reverse">>; readonly "flex-wrap": Longhand, Keyword.ToKeywords<"nowrap" | "wrap" | "wrap-reverse">>; readonly float: Longhand, Keyword.ToKeywords<"none" | "left" | "right">>; - readonly "font-family": Longhand | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace"> | String_2>, List | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace"> | String_2>>; + readonly "font-family": Longhand | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace">>, List | Keyword<"sans-serif"> | Keyword<"cursive"> | Keyword<"fantasy"> | Keyword<"monospace">>>; readonly "font-size": Longhand | Keyword<"xx-small"> | Keyword<"x-small"> | Keyword<"small"> | Keyword<"large"> | Keyword<"x-large"> | Keyword<"xx-large"> | Keyword<"xxx-large"> | Keyword<"larger"> | Keyword<"smaller">, Length>; readonly "font-stretch": Longhand | Percentage.Fixed>; readonly "font-style": Longhand, Keyword.ToKeywords<"normal" | "italic" | "oblique">>; @@ -177,7 +177,7 @@ export namespace Longhands { readonly "font-variant-east-asian": Longhand; readonly "font-variant-ligatures": Longhand; readonly "font-variant-numeric": Longhand; - readonly "font-variant-position": Longhand, Keyword.ToKeywords<"normal" | "sub" | "super">>; + readonly "font-variant-position": Longhand, Keyword.ToKeywords<"sub" | "normal" | "super">>; readonly "font-weight": Longhand | Keyword<"bold"> | Keyword<"bolder"> | Keyword<"lighter">, Number_2.Fixed>; readonly height: Longhand, LengthPercentage | Keyword<"auto">>; readonly "inset-block-end": Longhand, LengthPercentage | Keyword<"auto">>; @@ -186,29 +186,29 @@ export namespace Longhands { readonly "inset-inline-start": Longhand, LengthPercentage | Keyword<"auto">>; readonly left: Longhand, LengthPercentage | Keyword<"auto">>; readonly "letter-spacing": Longhand, Length>; - readonly "line-height": Longhand, Computed>; - readonly "margin-bottom": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; - readonly "margin-left": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; - readonly "margin-right": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; - readonly "margin-top": Longhand | Percentage, Length | Keyword<"auto"> | Percentage>; - readonly "min-height": Longhand | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Keyword<"auto"> | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; - readonly "min-width": Longhand | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Keyword<"auto"> | Percentage | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; + readonly "line-height": Longhand, Computed>; + readonly "margin-bottom": Longhand, Length | Percentage | Keyword<"auto">>; + readonly "margin-left": Longhand, Length | Percentage | Keyword<"auto">>; + readonly "margin-right": Longhand, Length | Percentage | Keyword<"auto">>; + readonly "margin-top": Longhand, Length | Percentage | Keyword<"auto">>; + readonly "min-height": Longhand | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Percentage | Keyword<"auto"> | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; + readonly "min-width": Longhand | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">, Length | Percentage | Keyword<"auto"> | Keyword<"fit-content"> | Keyword<"max-content"> | Keyword<"min-content">>; readonly opacity: Longhand, Number_2.Fixed>; readonly "outline-color": Longhand, Color.Canonical | Keyword<"invert">>; readonly "outline-offset": Longhand, Length>; - readonly "outline-style": Longhand, Keyword.ToKeywords<"none" | "auto" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "inset" | "outset">>; + readonly "outline-style": Longhand, Keyword.ToKeywords<"none" | "inset" | "auto" | "dotted" | "dashed" | "solid" | "double" | "groove" | "ridge" | "outset">>; readonly "outline-width": Longhand | Keyword<"medium"> | Keyword<"thick">, Length>; - readonly "overflow-x": Longhand, Keyword<"scroll"> | Keyword<"auto"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; - readonly "overflow-y": Longhand, Keyword<"scroll"> | Keyword<"auto"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; - readonly position: Longhand, Keyword.ToKeywords<"fixed" | "static" | "relative" | "absolute" | "sticky">>; + readonly "overflow-x": Longhand, Keyword<"auto"> | Keyword<"scroll"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; + readonly "overflow-y": Longhand, Keyword<"auto"> | Keyword<"scroll"> | Keyword<"hidden"> | Keyword<"visible"> | Keyword<"clip">>; + readonly position: Longhand, Keyword.ToKeywords<"fixed" | "relative" | "static" | "absolute" | "sticky">>; readonly right: Longhand, LengthPercentage | Keyword<"auto">>; - readonly rotate: Longhand | Rotate, Keyword<"none"> | Rotate>; + readonly rotate: Longhand, Rotate | Keyword<"none">>; readonly "text-align": Longhand, Keyword.ToKeywords<"left" | "right" | "center" | "start" | "end" | "justify">>; readonly "text-decoration-color": Longhand; readonly "text-decoration-line": Longhand | List | Keyword<"overline"> | Keyword<"line-through"> | Keyword<"blink">>, Keyword<"none"> | List | Keyword<"overline"> | Keyword<"line-through"> | Keyword<"blink">>>; readonly "text-decoration-style": Longhand, Keyword.ToKeywords<"dotted" | "dashed" | "solid" | "double" | "wavy">>; readonly "text-decoration-thickness": Longhand | Keyword<"from-font">, Length | Keyword<"auto"> | Keyword<"from-font">>; - readonly "text-indent": Longhand | Length | Percentage.Calculated<"length"> | Length, LengthPercentage>; + readonly "text-indent": Longhand | LengthPercentage | Percentage.Calculated<"length"> | Percentage.Fixed<"length">, LengthPercentage>; readonly "text-overflow": Longhand, Keyword.ToKeywords<"clip" | "ellipsis">>; readonly "text-shadow": Longhand | List>, Keyword<"none"> | List>; readonly "text-transform": Longhand, Keyword.ToKeywords<"none" | "capitalize" | "uppercase" | "lowercase">>; @@ -295,7 +295,7 @@ export namespace Shorthands { readonly "font-variant": Shorthand<"font-variant-caps" | "font-variant-east-asian" | "font-variant-ligatures" | "font-variant-numeric">; readonly "inset-block": Shorthand<"inset-block-end" | "inset-block-start">; readonly "inset-inline": Shorthand<"inset-inline-end" | "inset-inline-start">; - readonly inset: Shorthand<"left" | "right" | "top" | "bottom">; + readonly inset: Shorthand<"top" | "bottom" | "left" | "right">; readonly margin: Shorthand<"margin-bottom" | "margin-left" | "margin-right" | "margin-top">; readonly outline: Shorthand<"outline-color" | "outline-style" | "outline-width">; readonly overflow: Shorthand<"overflow-x" | "overflow-y">; From 9ec0d85fbfa33cf4c62ba0050598bc059fa0a15b Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Thu, 14 Dec 2023 11:43:01 +0100 Subject: [PATCH 22/22] Improve comment --- packages/alfa-cascade/src/rule-tree.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/alfa-cascade/src/rule-tree.ts b/packages/alfa-cascade/src/rule-tree.ts index 118a8302e5..ec2592e7a9 100644 --- a/packages/alfa-cascade/src/rule-tree.ts +++ b/packages/alfa-cascade/src/rule-tree.ts @@ -56,7 +56,7 @@ import * as json from "@siteimprove/alfa-json"; * the cascade sort by "Order of Appearance". This information is not available * for the rule tree which relies on rules being fed to it in increasing * precedence for each element. If `.bar` came before `.foo`, the resulting tree - * would be: + * would be (notice that `.foo` is not sharable anymore): * * "div" * +-- ".foo"