Skip to content

Commit 531a41e

Browse files
authored
Give the render probes the one open-root walk their comments already claim (#120)
## Problem Three comments across the render probes name `OPEN_ROOTS` as the one answer to which trees are the page, and the identifier does not exist. The shadow-root walk it names is written out inline six times — byte-identical — in `words.js` (twice), `widgets.js` (twice), `framing.js`, and `runtime.js`. Each of the three comments states the opposite of what the code does: - `words.js` carried a section note ending "Written once, because it is one claim about the page and two copies of it are two things to keep level", sitting above two copies in that same file. - `framing.js`'s `trappedMargins` opens "Which document each box is in, imported rather than restated, for the same reason UNMARKABLE_ITEMS imports its two" — and the next four lines restate it. - `widgets.js`'s `retiredSlots` says the holders "are found through OPEN_ROOTS too". `UNMARKABLE_ITEMS`, `COVERED_WORDS` and `TINY_BOXES` all resolve — the prose convention is SCREAMING_CASE for a camelCase export, and `unmarkableItems` really does import `shownBox`/`shownParts` rather than restate them. `OPEN_ROOTS` is the one reference in the directory with nothing behind it, so the file that documents the invariant is the file that breaks it. ## Solution Extract `openRoots` into `render-checks/open-roots.js` and import it in the four modules, deleting all six copies. The section note moves to the new module, where it is now true, and gains the two boundaries a reader has to know: why this is not the runtime's `shadowRootsIn` (that one reads the registry's `x-shadow` list and is right to, this one walks so an undeclared root is still read), and why `standalone.js` keeps a walk of its own (it is served import-free so a BAKE'd copy can be probed from a `file://` URL). The three prose references are left spelled `OPEN_ROOTS`, which is now the same kind of reference as its neighbours. `PROBE_SOURCES` is glob-built over the directory, so the new module is served with no change to `render_checks.py` or `http.py`. It is deliberately not added to `index.js`: it is a helper the probes import, not a probe the gate calls. ## Testing No regression test. The change is behaviour-preserving by construction — six byte-identical expressions replaced by one import of the same expression — so there is no state in which the old code and the new disagree, and a test could only assert that the file count is one, which is what `git` already shows. The probes themselves are covered by the existing gate suite, and the walk is load-bearing across all of it: `pageSettled` (`runtime.js`) is the wait almost every render test hangs on, so a broken walk fails the suite rather than one case. Run on this branch: - `tests/test_render_commands.py`, `tests/test_render_export.py` — 32 passed. - `tests/test_render_gate.py`, `tests/test_render_projection.py`, `tests/test_render_startup.py` — 182 passed, 4 failed. - `pre-commit` over the five changed files — prettier, eslint, typos all pass. <details><summary>The four failures are the layout regression already red on main</summary> They are not this branch's. [`ci` run 33296070275](https://github.com/max-sixty/leaf/actions/runs/33296070275) on `3d494e1` and [run 33295489155](https://github.com/max-sixty/leaf/actions/runs/33295489155) on `26feec8` both report the same four from this file, with the same assertions: - `test_a_reader_arrives_at_what_they_left_rather_than_watching_it_arrive` - `test_the_render_gate_tells_a_float_in_the_margin_from_one_spilling_out_of_it` - `test_the_reader_draws_an_edge_to_the_width_they_want[comments]` - `test_a_window_with_no_room_for_a_chosen_width_does_not_un_choose_it[comments]` They belong to a 13-test cluster spanning `test_render_controls.py`, `test_render_gate.py`, `test_render_options.py`, `test_render_pages.py` and `test_render_widgets.py`, all reporting sideways document scroll or a 12–15px offset (`assert -15 == 0`, "the page scrolls sideways with the tray up"). `tend-ci-fix` runs are in flight for both commits, so this branch leaves them alone. </details> Co-authored-by: leaf-agent <318509791+leaf-agent@users.noreply.github.com>
1 parent 84aef1d commit 531a41e

5 files changed

Lines changed: 38 additions & 55 deletions

File tree

skills/leaf/scripts/leaf/render-checks/framing.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { inChrome } from "/runtime/widget-api.js";
2+
import { openRoots } from "./open-roots.js";
23

34
// A box that draws an inset and shows a different one. A child's outer margin normally
45
// collapses through its parent and is spent between blocks; where the parent draws
@@ -27,16 +28,10 @@ import { inChrome } from "/runtime/widget-api.js";
2728
//
2829
// Deduped per tag and edge, because one mistake is on every instance of that widget.
2930
export function trappedMargins() {
30-
// Which document each box is in, imported rather than restated, for the same
31-
// reason UNMARKABLE_ITEMS imports its two: the runtime's layer holds shadow roots
32-
// of its own, and a `closest` written out here stops at the first of them and
31+
// Which document each box is in is OPEN_ROOTS', imported rather than restated, for
32+
// the same reason UNMARKABLE_ITEMS imports its two: the runtime's layer holds shadow
33+
// roots of its own, and a `closest` written out here stops at the first of them and
3334
// calls what it finds the page's.
34-
const roots = (root) => [
35-
root,
36-
...[...root.querySelectorAll("*")]
37-
.filter((el) => el.shadowRoot)
38-
.flatMap((el) => roots(el.shadowRoot)),
39-
];
4035
const px = (v) => parseFloat(v) || 0;
4136
// The platform's own answer to "does a child's margin reach my edge, and can it get
4237
// past it": a box that establishes a formatting context keeps every margin inside.
@@ -78,7 +73,7 @@ export function trappedMargins() {
7873
return out;
7974
};
8075
const found = [];
81-
for (const root of roots(document))
76+
for (const root of openRoots(document))
8277
for (const el of root.querySelectorAll("*")) {
8378
const s = getComputedStyle(el);
8479
if (s.display === "none" || s.display === "contents") continue;
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
/* Which trees are the page, for the readings that answer for what a widget renders
2+
* rather than for what it declares.
3+
*
4+
* Every open root, found by walking rather than read off the registry's x-shadow list:
5+
* a root a module attached without declaring one still holds words and code the reader
6+
* has to read, and a reading that asked the registry would look away from exactly the
7+
* tree nobody vouched for. That is the whole of the difference from the runtime's own
8+
* `shadowRootsIn`, which answers the document's question — whose words these are — and
9+
* is right to stop at what a version promised.
10+
*
11+
* Written once, because it is one claim about the page and two copies of it are two
12+
* things to keep level. Every probe that crosses a shadow boundary imports it rather
13+
* than restating it, for the same reason UNMARKABLE_ITEMS imports its two readings
14+
* from the runtime: what one probe walks and what another walks cannot come apart.
15+
*
16+
* `standalone.js` keeps a walk of its own, and has to: it is served import-free so a
17+
* BAKE'd copy can be probed from a file:// URL with no module graph behind it. */
18+
export const openRoots = (root) => [
19+
root,
20+
...[...root.querySelectorAll("*")]
21+
.filter((el) => el.shadowRoot)
22+
.flatMap((el) => openRoots(el.shadowRoot)),
23+
];

skills/leaf/scripts/leaf/render-checks/runtime.js

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { ARRANGEMENTS } from "/runtime/widget-api.js";
2+
import { openRoots } from "./open-roots.js";
23

34
export const runtimeStarted = () => document.querySelector(".lf-banner") !== null;
45
export const upgraded = () => document.body.dataset.lfUpgraded === "1";
@@ -9,20 +10,14 @@ export const logApplied = (applied) =>
910
Number(document.body.dataset.lfApplied ?? -1) >= applied;
1011

1112
export function moving() {
12-
const roots = (root) => [
13-
root,
14-
...[...root.querySelectorAll("*")]
15-
.filter((el) => el.shadowRoot)
16-
.flatMap((el) => roots(el.shadowRoot)),
17-
];
1813
const at = (el) => {
1914
for (let node = el; node; node = node.getRootNode?.()?.host) {
2015
const named = node.closest?.("[id]");
2116
if (named) return `<${named.tagName.toLowerCase()} id=${named.id}>`;
2217
}
2318
return `<${el?.tagName?.toLowerCase() ?? "?"}>`;
2419
};
25-
return roots(document)
20+
return openRoots(document)
2621
.flatMap((root) => root.getAnimations())
2722
.filter(
2823
(animation) =>

skills/leaf/scripts/leaf/render-checks/widgets.js

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
quoted,
66
textNodesUnder,
77
} from "/runtime/widget-api.js";
8+
import { openRoots } from "./open-roots.js";
89

910
export const failSoftErrors = () =>
1011
[...document.querySelectorAll(".lf-error")].map((el) => el.textContent.trim());
@@ -75,19 +76,13 @@ export const missingConversations = (widgets) =>
7576
//
7677
// Deduped and reported per tag and attribute, because one mistake is on every instance.
7778
export function undeclaredAttrs(widgets) {
78-
const roots = (root) => [
79-
root,
80-
...[...root.querySelectorAll("*")]
81-
.filter((el) => el.shadowRoot)
82-
.flatMap((el) => roots(el.shadowRoot)),
83-
];
8479
// What a module may write without declaring: the platform's own vocabulary for
8580
// what a control is and how it behaves, and the data-* namespace the runtime and
8681
// the widgets both paint in. `class` and `style` are the same kind of fact — a
8782
// look, not a state a version could carry.
8883
const painted = /^(?:data-|aria-)/;
8984
const platform = new Set(["role", "class", "style", "hidden", "tabindex"]);
90-
const all = roots(document);
85+
const all = openRoots(document);
9186
const found = [];
9287
for (const [tag, entry] of Object.entries(widgets)) {
9388
if (!entry.properties) continue;
@@ -134,13 +129,7 @@ export function undeclaredAttrs(widgets) {
134129
// `Animation.finished` here would give page.evaluate a promise the driver cannot
135130
// interrupt if the compositor stops.
136131
export function retiredSlots(holders) {
137-
const roots = (root) => [
138-
root,
139-
...[...root.querySelectorAll("*")]
140-
.filter((el) => el.shadowRoot)
141-
.flatMap((el) => roots(el.shadowRoot)),
142-
];
143-
const all = roots(document);
132+
const all = openRoots(document);
144133
const find = (id) => {
145134
for (const r of all) {
146135
const el = r.getElementById(id);

skills/leaf/scripts/leaf/render-checks/words.js

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { says } from "/runtime/widget-api.js";
2+
import { openRoots } from "./open-roots.js";
23

34
export const shownVerbatim = ({ widgets, touched }) =>
45
Object.entries(widgets)
@@ -106,14 +107,6 @@ export function paperWords() {
106107
// group that has been opened and closed, which is where the test pins it.
107108
export { coveredWords } from "./standalone.js";
108109

109-
// Which trees are the page, for the two readings below that answer for what a widget
110-
// renders rather than for what it declares. Every open root, found by walking rather than
111-
// read off the registry's x-shadow list: a root a module attached without declaring one
112-
// still holds words and code the reader has to read, and a reading that asked the
113-
// registry would look away from exactly the tree nobody vouched for. Written once,
114-
// because it is one claim about the page and two copies of it are two things to keep
115-
// level.
116-
117110
// Code that came out the colour of the code around it. Colouring takes two halves that
118111
// meet nowhere a static lint can reach: the runtime writes data-lf-syn in the browser,
119112
// and the theme answers it with a var() the browser resolves. Either half can stop
@@ -150,17 +143,11 @@ export { coveredWords } from "./standalone.js";
150143
// asked for colour-contrast alone on the example carrying the beige comment, it returned
151144
// 44 passing elements, no violation, and not one of the spans among them.
152145
//
153-
// Which shadow roots it crosses into is OPEN_ROOTS' answer (the section note above says
154-
// why it crosses at all), which is the choice everything else here makes — colour is
146+
// Which shadow roots it crosses into is OPEN_ROOTS' answer (that module says why it
147+
// crosses at all), which is the choice everything else here makes — colour is
155148
// asked of what the browser painted, so where it painted is too, and a root a widget
156149
// attached without declaring one still holds code the reader has to read.
157150
export function unreadSyntax() {
158-
const roots = (root) => [
159-
root,
160-
...[...root.querySelectorAll("*")]
161-
.filter((el) => el.shadowRoot)
162-
.flatMap((el) => roots(el.shadowRoot)),
163-
];
164151
const cx = document.createElement("canvas").getContext("2d");
165152
const paint = (...layers) => {
166153
cx.clearRect(0, 0, 1, 1);
@@ -185,7 +172,7 @@ export function unreadSyntax() {
185172
};
186173
const seen = new Set(),
187174
found = new Map();
188-
for (const span of roots(document).flatMap((r) => [
175+
for (const span of openRoots(document).flatMap((r) => [
189176
...r.querySelectorAll("[data-lf-syn]"),
190177
])) {
191178
const role = span.dataset.lfSyn;
@@ -254,14 +241,8 @@ export function unreadSyntax() {
254241
// `hidden` and `hidden="until-found"` are two of the ways an element stops rendering, and
255242
// asking whether it renders covers both and the panel besides.
256243
export function silentWords(widgets) {
257-
const roots = (root) => [
258-
root,
259-
...[...root.querySelectorAll("*")]
260-
.filter((el) => el.shadowRoot)
261-
.flatMap((el) => roots(el.shadowRoot)),
262-
];
263244
const found = [];
264-
const all = roots(document);
245+
const all = openRoots(document);
265246
const at = (el) => `<${el.localName}${el.id ? " id=" + el.id : ""}>`;
266247
const every = (tag) => all.flatMap((r) => [...r.querySelectorAll(tag)]);
267248
for (const [tag, entry] of Object.entries(widgets)) {

0 commit comments

Comments
 (0)