From 3b683eeec6dff41044de1ddc324fb64be4becafd Mon Sep 17 00:00:00 2001 From: Jean-Yves Moyen Date: Wed, 10 Jan 2024 09:43:51 +0100 Subject: [PATCH 1/2] Fix tree traversal of flat tree (#1547) * Fix tree traversal of flat tree * Remove unused import * Extract API --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- .changeset/dirty-lizards-explode.md | 5 ++ docs/review/api/alfa-dom.api.md | 4 -- packages/alfa-dom/src/node.ts | 28 ++++++++ packages/alfa-dom/src/node/element.ts | 20 ------ packages/alfa-dom/src/node/shadow.ts | 2 + packages/alfa-dom/src/node/text.ts | 22 ------- .../src/node/traversal/get-nodes-between.ts | 5 +- packages/alfa-dom/test/traversal.spec.tsx | 66 +++++++++++++++++++ packages/alfa-dom/tsconfig.json | 5 +- 9 files changed, 106 insertions(+), 51 deletions(-) create mode 100644 .changeset/dirty-lizards-explode.md create mode 100644 packages/alfa-dom/test/traversal.spec.tsx diff --git a/.changeset/dirty-lizards-explode.md b/.changeset/dirty-lizards-explode.md new file mode 100644 index 0000000000..eb799df63f --- /dev/null +++ b/.changeset/dirty-lizards-explode.md @@ -0,0 +1,5 @@ +--- +"@siteimprove/alfa-dom": patch +--- + +**Fixed:** Parents of `Comment` inside a shadow tree now correctly skip over the shadow root when traversing the flat tree. diff --git a/docs/review/api/alfa-dom.api.md b/docs/review/api/alfa-dom.api.md index b758b4d793..385c50523b 100644 --- a/docs/review/api/alfa-dom.api.md +++ b/docs/review/api/alfa-dom.api.md @@ -289,8 +289,6 @@ export class Element extends Node<"element"> implemen // (undocumented) static of(namespace: Option, prefix: Option, name: N, attributes?: Iterable_2, children?: Iterable_2, style?: Option, box?: Option, device?: Option, externalId?: string, extraData?: any): Element; // (undocumented) - parent(options?: Node.Traversal): Option; - // (undocumented) get prefix(): Option; // (undocumented) get qualifiedName(): string; @@ -1149,8 +1147,6 @@ export class Text extends Node<"text"> implements Slotable { // (undocumented) static of(data: string, externalId?: string, extraData?: any): Text; // (undocumented) - parent(options?: Node.Traversal): Option; - // (undocumented) toJSON(): Text.JSON; // (undocumented) toString(): string; diff --git a/packages/alfa-dom/src/node.ts b/packages/alfa-dom/src/node.ts index 9aecde7f7c..9c85730e97 100644 --- a/packages/alfa-dom/src/node.ts +++ b/packages/alfa-dom/src/node.ts @@ -22,6 +22,7 @@ import { Fragment, Shadow, Slot, + Slotable, Text, Type, } from "."; @@ -135,6 +136,33 @@ export abstract class Node }); } + public parent(options: Node.Traversal = Node.Traversal.empty): Option { + const parent = this._parent as Option; + + // If we traverse the flat tree, we want to jump over shadow roots. + if (options.isSet(Node.Traversal.flattened)) { + return parent.flatMap((parent) => { + if (Shadow.isShadow(parent)) { + return parent.host; + } + + // Additionally, if this is a slottable light child of a shadow host, we want + // to search for where it is slotted, and return that parent instead. + if ( + Element.isElement(parent) && + parent.shadow.isSome() && + Slotable.isSlotable(this) + ) { + return this.assignedSlot().flatMap((slot) => slot.parent(options)); + } + + return Option.of(parent); + }); + } + + return parent; + } + private _path: Array = []; /** diff --git a/packages/alfa-dom/src/node/element.ts b/packages/alfa-dom/src/node/element.ts index c02c9c7162..a981f2d359 100644 --- a/packages/alfa-dom/src/node/element.ts +++ b/packages/alfa-dom/src/node/element.ts @@ -161,26 +161,6 @@ export class Element return this._boxes.get(device); } - public parent(options: Node.Traversal = Node.Traversal.empty): Option { - const parent = this._parent as Option; - - if (options.isSet(Node.Traversal.flattened)) { - return parent.flatMap((parent) => { - if (Shadow.isShadow(parent)) { - return parent.host; - } - - if (Element.isElement(parent) && parent.shadow.isSome()) { - return this.assignedSlot().flatMap((slot) => slot.parent(options)); - } - - return Option.of(parent); - }); - } - - return parent; - } - public children( options: Node.Traversal = Node.Traversal.empty, ): Sequence { diff --git a/packages/alfa-dom/src/node/shadow.ts b/packages/alfa-dom/src/node/shadow.ts index ee6ed5fed0..ea28639981 100644 --- a/packages/alfa-dom/src/node/shadow.ts +++ b/packages/alfa-dom/src/node/shadow.ts @@ -61,6 +61,8 @@ export class Shadow extends Node<"shadow"> { } public parent(options: Node.Traversal = Node.Traversal.empty): Option { + // We only "land" on Shadow roots when traversing the composed tree. + // Notably, flattening the tree "jumps" over them. if (options.isSet(Node.Traversal.composed)) { return this._host; } diff --git a/packages/alfa-dom/src/node/text.ts b/packages/alfa-dom/src/node/text.ts index db4a2e86b0..1267ce6bb9 100644 --- a/packages/alfa-dom/src/node/text.ts +++ b/packages/alfa-dom/src/node/text.ts @@ -2,8 +2,6 @@ import { Option } from "@siteimprove/alfa-option"; import { Trampoline } from "@siteimprove/alfa-trampoline"; import { Node } from "../node"; -import { Element } from "./element"; -import { Shadow } from "./shadow"; import { Slot } from "./slot"; import { Slotable } from "./slotable"; @@ -31,26 +29,6 @@ export class Text extends Node<"text"> implements Slotable { return this._data; } - public parent(options: Node.Traversal = Node.Traversal.empty): Option { - const parent = this._parent as Option; - - if (options.isSet(Node.Traversal.flattened)) { - return parent.flatMap((parent) => { - if (Shadow.isShadow(parent)) { - return parent.host; - } - - if (Element.isElement(parent) && parent.shadow.isSome()) { - return this.assignedSlot().flatMap((slot) => slot.parent(options)); - } - - return Option.of(parent); - }); - } - - return parent; - } - public assignedSlot(): Option { return Slotable.findSlot(this); } diff --git a/packages/alfa-dom/src/node/traversal/get-nodes-between.ts b/packages/alfa-dom/src/node/traversal/get-nodes-between.ts index 105279fc0e..0897bf9448 100755 --- a/packages/alfa-dom/src/node/traversal/get-nodes-between.ts +++ b/packages/alfa-dom/src/node/traversal/get-nodes-between.ts @@ -1,6 +1,6 @@ -import { Node } from "../.."; import { Predicate } from "@siteimprove/alfa-predicate"; import { Sequence } from "@siteimprove/alfa-sequence"; +import { Node } from "../.."; import { lowestCommonAncestor } from "./lowest-common-ancestor"; @@ -43,7 +43,7 @@ export function getNodesBetween( // of the closest ancestor having one. between = first // Closest ancestor with a next sibling. - .closest((ancestor) => ancestor.next(treeOptions).isSome()) + .closest((ancestor) => ancestor.next(treeOptions).isSome(), treeOptions) // Get that sibling. .flatMap((node) => node.next(treeOptions)) // Skip everything until next. @@ -71,7 +71,6 @@ function getNodesInclusivelyBetween( ): Sequence { const isFrontier = or(equals(node1), equals(node2)); - // Get descendants of the LCA, and skip everything before and after both nodes. return lowestCommonAncestor(node1, node2, treeOptions) .map((context) => context diff --git a/packages/alfa-dom/test/traversal.spec.tsx b/packages/alfa-dom/test/traversal.spec.tsx new file mode 100644 index 0000000000..33c1b1922a --- /dev/null +++ b/packages/alfa-dom/test/traversal.spec.tsx @@ -0,0 +1,66 @@ +import { None } from "@siteimprove/alfa-option"; +import { test } from "@siteimprove/alfa-test"; + +import { h, Comment, Node } from "../src"; + +const targets = [, h.text("hello"), Comment.of("world")]; +const shadow = h.shadow(targets); +const div =
{shadow}
; + +// Start with no flag. +// For each of the three flags, duplicate existing list and add the flag to one side. +// This ends up giving the eight possible options. +const flags = [ + Node.Traversal.composed, + Node.Traversal.flattened, + Node.Traversal.nested, +].reduce( + (old, cur) => old.flatMap((flag) => [flag, flag.add(cur)]), + [Node.Traversal.of(Node.Traversal.none)], +); + +test(".parent() of a shadow host returns None in composed traversal, the shadow root otherwise", (t) => { + for (const traversal of flags) { + if (traversal.has(Node.Traversal.composed)) { + t.deepEqual(shadow.parent(traversal).getUnsafe(), div); + } else { + t.deepEqual(shadow.parent(traversal), None); + } + } +}); + +test(".parent() of the children of a shadow root returns the shadow host in flat traversal, the shadow root otherwise", (t) => { + for (const target of targets) { + for (const traversal of flags) { + if (traversal.has(Node.Traversal.flattened)) { + t.deepEqual(target.parent(traversal).getUnsafe(), div); + } else { + t.deepEqual(target.parent(traversal).getUnsafe(), shadow); + } + } + } +}); + +test(".parent() of a slottable child of a shadow host returns the slot's parent in flat traversal, the light parent otherwise", (t) => { + const target = ; + const shadowDiv = ( +
+ +
+ ); + const shadow = h.shadow([shadowDiv]); + const lightDiv = ( +
+ {shadow} + {target} +
+ ); + + for (const traversal of flags) { + if (traversal.has(Node.Traversal.flattened)) { + t.deepEqual(target.parent(traversal).getUnsafe(), shadowDiv); + } else { + t.deepEqual(target.parent(traversal).getUnsafe(), lightDiv); + } + } +}); diff --git a/packages/alfa-dom/tsconfig.json b/packages/alfa-dom/tsconfig.json index 6eba504794..a096aaf300 100644 --- a/packages/alfa-dom/tsconfig.json +++ b/packages/alfa-dom/tsconfig.json @@ -86,7 +86,8 @@ "test/node/traversal/get-nodes-between.spec.tsx", "test/node/traversal/lowest-common-ancestor.spec.tsx", "test/node/query/element-descendants.spec.tsx", - "test/node/query/element-id-map.spec.tsx" + "test/node/query/element-id-map.spec.tsx", + "test/traversal.spec.tsx" ], "references": [ { "path": "../alfa-array" }, @@ -103,7 +104,7 @@ { "path": "../alfa-map" }, { "path": "../alfa-option" }, { "path": "../alfa-predicate" }, - { "path": "../alfa-rectangle"}, + { "path": "../alfa-rectangle" }, { "path": "../alfa-refinement" }, { "path": "../alfa-sarif" }, { "path": "../alfa-selective" }, From 698761c337e7ec563d45c5ce7f82652996ad1e90 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Mon, 15 Jan 2024 09:20:21 +0100 Subject: [PATCH 2/2] Update Other deps (minor/patch) (#1549) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- yarn.lock | 35 ++++++++++++++--------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/yarn.lock b/yarn.lock index 6f549c5b8a..60cb50f81c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -406,9 +406,9 @@ __metadata: linkType: hard "@mdn/browser-compat-data@npm:^5.0.0": - version: 5.5.4 - resolution: "@mdn/browser-compat-data@npm:5.5.4" - checksum: bdef5c85cc6e90bbf6e873033bfcf92dba2df699fb9fdd7390f5d1dc0e6d14fba9b8b3585370248a82913585eb1792c1afd20900efb6604c9e59122ba98060d7 + version: 5.5.6 + resolution: "@mdn/browser-compat-data@npm:5.5.6" + checksum: b22f1d9e69451c12295b0ead856dd5d12b08f43a068047bc8e40f365fd8b41207a96e2a5383048a0b6e87daa0edc4e8cbdaf70e1e593b653e347178a85b30530 languageName: node linkType: hard @@ -2092,11 +2092,11 @@ __metadata: linkType: hard "@types/node@npm:*, @types/node@npm:^20.5.9": - version: 20.10.7 - resolution: "@types/node@npm:20.10.7" + version: 20.11.1 + resolution: "@types/node@npm:20.11.1" dependencies: undici-types: "npm:~5.26.4" - checksum: d626cea1b7da4784ee7b335dcc54e64adba9725dab7ca51a690167de502ef89fec07b05ad8e25845d188d7ad7f72c192ec92964d456321ed5b9452113bf9351f + checksum: f665cdce28b0b6e57338d1f74e0599ee9b10eac74cff729921c8f473807398e9aba2f8cf74c74a4d3dfbc2d616c73267da7de3003ed3c8152ea366bf4c96a91a languageName: node linkType: hard @@ -2663,13 +2663,6 @@ __metadata: languageName: node linkType: hard -"chalk@npm:^5.3.0": - version: 5.3.0 - resolution: "chalk@npm:5.3.0" - checksum: 8297d436b2c0f95801103ff2ef67268d362021b8210daf8ddbe349695333eb3610a71122172ff3b0272f1ef2cf7cc2c41fdaa4715f52e49ffe04c56340feed09 - languageName: node - linkType: hard - "chardet@npm:^0.7.0": version: 0.7.0 resolution: "chardet@npm:0.7.0" @@ -4593,8 +4586,8 @@ __metadata: linkType: hard "knip@npm:^3.0.0": - version: 3.12.0 - resolution: "knip@npm:3.12.0" + version: 3.13.2 + resolution: "knip@npm:3.13.2" dependencies: "@ericcornelissen/bash-parser": "npm:0.5.2" "@npmcli/map-workspaces": "npm:3.0.4" @@ -4603,7 +4596,6 @@ __metadata: "@pnpm/logger": "npm:5.0.0" "@pnpm/workspace.pkgs-graph": "npm:^2.0.13" "@snyk/github-codeowners": "npm:1.1.0" - chalk: "npm:^5.3.0" easy-table: "npm:1.2.0" fast-glob: "npm:3.3.2" globby: "npm:^14.0.0" @@ -4611,6 +4603,7 @@ __metadata: js-yaml: "npm:4.1.0" micromatch: "npm:4.0.5" minimist: "npm:1.2.8" + picocolors: "npm:1.0.0" pretty-ms: "npm:8.0.0" strip-json-comments: "npm:5.0.1" summary: "npm:2.1.0" @@ -4621,7 +4614,7 @@ __metadata: typescript: ">=5.0.4" bin: knip: bin/knip.js - checksum: 29599ebb99d369154320aa6ab9bb0d24a1eb3ff8386593e8bef6fb5481eb1ed59a00bf490d43495a07a5164d21c8bbd3c5a79c24a74cdb1e12ae2855918c8901 + checksum: 9221fa8d863d298ad642fd379e0e8df7f160663e773591e226b46d73219e9a3679528fdc405071b8a3741f7b87491017b496331179db272df2db1945ea91c40d languageName: node linkType: hard @@ -5645,7 +5638,7 @@ __metadata: languageName: node linkType: hard -"picocolors@npm:^1.0.0": +"picocolors@npm:1.0.0, picocolors@npm:^1.0.0": version: 1.0.0 resolution: "picocolors@npm:1.0.0" checksum: 20a5b249e331c14479d94ec6817a182fd7a5680debae82705747b2db7ec50009a5f6648d0621c561b0572703f84dbef0858abcbd5856d3c5511426afcb1961f7 @@ -5697,11 +5690,11 @@ __metadata: linkType: hard "prettier@npm:^3.0.0": - version: 3.1.1 - resolution: "prettier@npm:3.1.1" + version: 3.2.2 + resolution: "prettier@npm:3.2.2" bin: prettier: bin/prettier.cjs - checksum: facc944ba20e194ff4db765e830ffbcb642803381f0d2033ed397e79904fa4ccc877dc25ad68f42d36985c01d051c990ca1b905fb83d2d7d65fe69e4386fa1a3 + checksum: e84d0d2a4ce2b88ee1636904effbdf68b59da63d9f887128f2ed5382206454185432e7c0a9578bc4308bc25d099cfef47fd0b9c211066777854e23e65e34044d languageName: node linkType: hard