From b9b0a4c0956cd4db82d5e610fb79040ad63531f5 Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:21:04 +0100 Subject: [PATCH 1/4] --browser webkit: shoot both sides in WebKit WebKit has no CDP, so a shot falls back to page.screenshot({ fullPage, clip }), tiles kept, and --computed walks without forced states and says so. WebKit holds fonts.ready, which its screenshot waits on, until the load event, so a page a stalled frame keeps loading is stopped first. browser is optional in ResolvedConfig, so a Chromium run's config hash and capture path are unchanged. A webkit run writes renderdiff-out-webkit and renderdiff.accepted.webkit.json by default, and merge refuses to mix engines. The Action takes a browser input; CI installs WebKit. Closes #150 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/action.yml | 15 ++ .github/workflows/ci.yml | 2 +- README.md | 6 +- action.yml | 14 +- docs/accepted.md | 2 + docs/github-action.md | 1 + docs/how-it-works.md | 5 +- docs/output.md | 2 +- skills/renderdiff/reference.md | 6 +- src/capture.ts | 7 +- src/cli.ts | 23 ++- src/config.ts | 18 ++- src/locate.ts | 4 +- src/probe.ts | 2 +- src/run.ts | 26 +++- tests/fixtures/pages/README.md | 2 +- .../pages/box-decoration-break/left.html | 25 ++++ .../pages/box-decoration-break/right.html | 25 ++++ tests/helpers/corpus.ts | 1 + tests/unit/cli-args.test.ts | 9 +- tests/unit/config-load.test.ts | 1 + tests/unit/report.test.ts | 5 + tests/visual/webkit.test.ts | 136 ++++++++++++++++++ 23 files changed, 314 insertions(+), 23 deletions(-) create mode 100644 tests/fixtures/pages/box-decoration-break/left.html create mode 100644 tests/fixtures/pages/box-decoration-break/right.html create mode 100644 tests/visual/webkit.test.ts diff --git a/.github/workflows/action.yml b/.github/workflows/action.yml index 7880fd8..c088cca 100644 --- a/.github/workflows/action.yml +++ b/.github/workflows/action.yml @@ -57,6 +57,18 @@ jobs: out: accept-out artifact-name: accept + - id: webkit + continue-on-error: true + uses: ./ + with: + left: ${{ steps.site.outputs.left }} + right: ${{ steps.site.outputs.right }} + routes: /flex-vs-grid /box-decoration-break + widths: "1440" + browser: webkit + out: webkit-out + artifact-name: webkit + - id: down continue-on-error: true uses: ./ @@ -71,12 +83,15 @@ jobs: DOWN: ${{ steps.down.outputs.exit-code }} CLEAN: ${{ steps.clean.outputs.exit-code }} DIFFER: ${{ steps.differ.outputs.exit-code }} + WEBKIT: ${{ steps.webkit.outputs.exit-code }} OUTCOME: ${{ steps.differ.outcome }} run: | test "$CLEAN" = 0 test "$DIFFER" = 1 test "$OUTCOME" = failure test "$DOWN" = 2 + test "$WEBKIT" = 1 + node -e 'const { meta, results: r } = require("./webkit-out/results.json"); if (meta.browser.name !== "webkit" || r.find((x) => x.url === "/flex-vs-grid").diffPixels !== 0 || !(r.find((x) => x.url === "/box-decoration-break").diffPixels > 0)) throw meta.browser' node -e 'const r = require("./differ-out/results.json").results; for (const p of ["/color", "/tint"]) if (!(r.find((x) => x.url === p)?.score < 0.999)) throw p' grep -q '"/color"' accept-out/renderdiff.accepted.json grep -q '' "$RUNNER_TEMP/renderdiff-comment-differ.md" diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index c351942..aca06ff 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -20,6 +20,6 @@ jobs: - run: npm ci - run: npm run lint - run: npm run format:check - - run: npx playwright install --with-deps chromium + - run: npx playwright install --with-deps chromium webkit - run: npm run typecheck - run: npm run test:all diff --git a/README.md b/README.md index efc6b4f..1f7d719 100644 --- a/README.md +++ b/README.md @@ -30,7 +30,7 @@ the agent needs a cheap, trustworthy loop: change → `renderdiff` → explain e **Don't use it for:** - intentional redesigns: when everything is meant to change, the score says nothing -- cross-browser testing (Chromium only) or stored-baseline snapshot tests (both origins must be live) +- cross-browser testing (each run shoots both sides in one engine, Chromium or WebKit, never one against the other) or stored-baseline snapshot tests (both origins must be live) - discovering routes: you list them, or `--sitemap` reads the reference's sitemap; there is no crawler - component libraries in isolation; Storybook-based tools fit that better @@ -88,7 +88,8 @@ production on every pull request, with the result in the job summary: Node >= 22.18 and Chromium (`npx playwright install chromium`). `npm install -g renderdiff` installs it globally. If the bundled Chromium won't launch, renderdiff falls back to an installed Google Chrome on both sides and says so; `meta.browser` in `results.json` records -which ran. +which ran. `--browser webkit` needs WebKit too (`npx playwright install webkit`, with +`--with-deps` on Linux). ## Usage @@ -107,6 +108,7 @@ renderdiff [options] | `--out ` | Output directory, wiped at the start of a run. Default `./renderdiff-out`. A directory holding files renderdiff didn't write is refused. | | `--concurrency ` | Parallel workers. Default half the CPU cores, between 2 and 8. Lower it when one side is someone's live site. | | `--config ` | Config file. Default: `renderdiff.config.{ts,mts,js,mjs}` in cwd. | +| `--browser ` | `chromium` (default) or `webkit`: both sides are shot in it, for CSS that only Safari reads differently, such as a dropped `-webkit-` prefix Chromium no longer needs. WebKit writes to `./renderdiff-out-webkit` unless `--out` or `outDir` say otherwise and accepts into `renderdiff.accepted.webkit.json`, so a Chromium and a WebKit run keep separate results. On WebKit, animated GIFs keep playing (their pixels show as unstable and aren't measured) and `--computed` doesn't force `:hover`/`:focus-visible`. | | `--selector ` | Compare the one element this Playwright selector matches, on both sides, instead of the whole page. Applies to every route without its own `@selector`, over the config's per-route `selector`. | | `--left-selector `, `--right-selector ` | That side's selector, over `--selector`. One without the other side (or `--selector`) is an error. | | `--mask ` | Paint this Playwright selector over on both sides, on every route; repeatable. Added to the config's masks and listed in the report as coverage given up. | diff --git a/action.yml b/action.yml index 85d472e..22e8473 100644 --- a/action.yml +++ b/action.yml @@ -15,6 +15,9 @@ inputs: routes: description: Routes separated by spaces or newlines, e.g. "/ /about /pricing::/plans". Split on spaces, so a per-route selector ('/route @sel') doesn't work here; put it in the config. Optional when the config lists them. default: "" + browser: + description: chromium or webkit; both sides are shot in it. For both, use two steps with their own out and artifact-name. + default: chromium config: description: Path to a renderdiff config file. Default renderdiff.config.{ts,mts,js,mjs} in the workspace. default: "" @@ -70,15 +73,17 @@ runs: - uses: actions/cache@v4 with: path: ${{ runner.tool_cache }}/renderdiff-ms-playwright - key: renderdiff-chromium-${{ runner.os }}-${{ runner.arch }}-${{ steps.pw.outputs.version }} + key: renderdiff-${{ inputs.browser }}-${{ runner.os }}-${{ runner.arch }}-${{ steps.pw.outputs.version }} - shell: bash working-directory: ${{ github.action_path }} env: PLAYWRIGHT_BROWSERS_PATH: ${{ runner.tool_cache }}/renderdiff-ms-playwright + BROWSER: ${{ inputs.browser }} run: | npm ci --omit=dev --ignore-scripts --no-audit --no-fund - npx playwright install --with-deps chromium + case "$BROWSER" in chromium|webkit) ;; *) echo "browser must be chromium or webkit, got '$BROWSER'"; exit 2 ;; esac + npx playwright install --with-deps "$BROWSER" - id: run shell: bash @@ -89,6 +94,7 @@ runs: RIGHT: ${{ inputs.right }} ROUTES: ${{ inputs.routes }} CONFIG: ${{ inputs.config }} + BROWSER: ${{ inputs.browser }} WIDTHS: ${{ inputs.widths }} THRESHOLD: ${{ inputs.threshold }} CONCURRENCY: ${{ inputs.concurrency }} @@ -119,7 +125,7 @@ runs: fi if [ "$code" = 0 ]; then - args=("--left=$LEFT" "--right=$RIGHT" "--out=$OUT" --no-progress) + args=("--left=$LEFT" "--right=$RIGHT" "--out=$OUT" "--browser=$BROWSER" --no-progress) read -ra routes <<< "$(echo "$ROUTES" | tr '\n' ' ')" args+=("${routes[@]}") for w in ${WIDTHS//,/ }; do args+=("--width=$w"); done @@ -139,7 +145,7 @@ runs: set -e rm -f "${state:-}" - accepted="$(dirname "${CONFIG:-./_}")/renderdiff.accepted.json" + accepted="$(dirname "${CONFIG:-./_}")/renderdiff.accepted$([ "$BROWSER" = chromium ] || echo ".$BROWSER").json" if [ -n "$ACCEPT" ] && [ -f "$accepted" ] && [ -d "$OUT" ]; then cp "$accepted" "$OUT/"; fi fi echo "exit-code=$code" >> "$GITHUB_OUTPUT" diff --git a/docs/accepted.md b/docs/accepted.md index e1a1ba1..acb9819 100644 --- a/docs/accepted.md +++ b/docs/accepted.md @@ -19,6 +19,8 @@ It records every unaccepted change in that scope, not only the ones you meant, s unintended ones before accepting, or check `git diff renderdiff.accepted.json` afterwards and delete the lines you didn't mean. Commit the file; it is one entry per line, so its diff reads as what was accepted. +A `--browser webkit` run reads and writes `renderdiff.accepted.webkit.json` instead: an +entry's pixel hash only ever matches the engine it was taken in, so each engine keeps its own. A shot whose every difference matches an accepted one is `accepted`: it is listed on an `accepted:` line like `clean:`, doesn't count as differing or toward the exit code, and has diff --git a/docs/github-action.md b/docs/github-action.md index 77d5bb8..1fd7a40 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -23,6 +23,7 @@ exits non-zero. | `left` | Your change: a preview URL or `http://localhost:3000`. Required. | | `right` | The reference, usually production. Required. | | `routes` | Separated by spaces or newlines; pairs (`/pricing::/plans`) work; per-route selectors go in the config, since the input splits on spaces. Optional when the config lists routes. | +| `browser` | `chromium` (default) or `webkit`, over the config's `browser`; both sides are shot in it, and `accept` records into `renderdiff.accepted.webkit.json` for WebKit. For both engines, run the action twice. | | `config` | Config file path. Default: `renderdiff.config.{ts,mts,js,mjs}` in the workspace. | | `widths` | Spaces or commas. Default renderdiff's five. | | `threshold` | Fail only below this score. Without it, any difference fails the job (exit 1); `"0"` never fails on one. | diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 9af7e68..96a7478 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -23,7 +23,10 @@ honestly. This is what renderdiff does to make sure of that. - **Kills animation, transitions, caret blink and smooth scrolling** (CSS `scroll-behavior` and `behavior: "smooth"` in script scrolls, from before any hook), and hides common consent banners and dev toolbars. Animated images (GIF, WebP, APNG) and SVG animations show their first frame; - it is a Blink setting, so cross-origin and CSS background images freeze too. + it is a Blink setting, so cross-origin and CSS background images freeze too. WebKit + (`--browser webkit`) has no such setting: there an animated image keeps playing, so its + pixels are caught as unstable, painted out of the score and can't be accepted. Mask one + that flips between the two sides. - **Settles the page** — makes native lazy images eager, scrolls every viewport-height slice a frame at a time, to the bottom even as the page grows, so script lazy loaders fire, waits for every image to decode, `document.fonts.ready` (at most 5s) and up to 3s for the network to go 250ms with nothing in flight diff --git a/docs/output.md b/docs/output.md index 71b0ccd..9da0d69 100644 --- a/docs/output.md +++ b/docs/output.md @@ -524,7 +524,7 @@ file, not each line), scores rounded down to four decimals (so a shot that diffe "clock": "…", // the instant both sides' clocks start at "blockUrlPatterns": ["…"], "hideSelectors": ["…"], "masks": [{ "urlPath": "/pricing", "reason": "live star count" }], // no urlPath: every route - "browser": { "name": "chromium", "version": "…" }, // "chrome" when it fell back + "browser": { "name": "chromium", "version": "…" }, // "chrome" when it fell back; "webkit" with --browser webkit "storageState": { "left": "auth.json", "right": null }, // file names only "config": "…", // hash of what decides a shot; --rerun-failed needs it unchanged "rightReplayedFrom": null, // --cache-right: when the replayed recording was made diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index 069dace..905b7dd 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -463,7 +463,7 @@ commas: `JSON.parse` the whole file, not each line. Paths are relative to the ou (each rule group once with `matched` and `unmatched` `[{ url, widths }]`) and `prefixes` (each declaration once: `side`, `declaration`, `at`). - **`meta`:** origins, widths, threshold, `clock`, block patterns, hidden selectors, mask - reasons, `browser` (`chromium`, or `chrome` after a fallback), storage state file names, + reasons, `browser` (`chromium`, `chrome` after a fallback, or `webkit`), storage state file names, `config` hash, `rightReplayedFrom`, `freezeVideo`, `lottieFrame` (only when set), `platform`, `rerun` (`{ rerun, reused }` or null), with `--computed` `unreadableSheets` `{ left, right }`. @@ -471,6 +471,10 @@ commas: `JSON.parse` the whole file, not each line. Paths are relative to the ou - **Browser:** if the bundled Chromium won't launch, the system Chrome is used on both sides and the run says so; otherwise it prints the `npx playwright install chromium` hint. + `--browser webkit` shoots both sides in WebKit (`npx playwright install webkit`) into + `renderdiff-out-webkit`, accepting into `renderdiff.accepted.webkit.json`: for CSS only + Safari reads differently. There animated GIFs aren't frozen (unstable, unmeasured), and + `--computed` forces no `:hover`/`:focus-visible`. `merge` refuses to mix engines. - **Pinned on both sides:** clock (one instant per run, floored to the minute), seeded `Math.random`, UTC, en-US, light scheme, reduced motion; animations and transitions stilled, animated images and SVG animations held on their first frame. diff --git a/src/capture.ts b/src/capture.ts index 0164591..d41190c 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -538,6 +538,8 @@ export async function capture( `at ${url} — likely a broken layout, not a real page`, ); } + // WebKit holds fonts.ready, which its screenshot waits on, until the load event, which a stalled frame never fires. + if (webkit(page)) await page.evaluate(() => document.readyState === "complete" || window.stop()); const tiled = area.height > MAX_CAPTURE_HEIGHT; // Read on both sides of the shot, so a box that moves while it is taken stays covered. const shoot = async () => { @@ -722,8 +724,11 @@ async function captureTiled(page: Page, box: Box): Promise { const sessions = new WeakMap>(); -/** A PNG of `clip`, in page px, on the page's one CDP session. */ +export const webkit = (page: Page) => page.context().browser()?.browserType().name() === "webkit"; + +/** A PNG of `clip`, in page px, on the page's one CDP session; WebKit has none, and renders it all offscreen. */ async function grab(page: Page, clip: Box, beyond: boolean): Promise { + if (webkit(page)) return page.screenshot({ fullPage: true, clip }); let s = sessions.get(page); if (!s) sessions.set(page, (s = page.context().newCDPSession(page))); const { data } = await ( diff --git a/src/cli.ts b/src/cli.ts index a9618b2..25aea2a 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -16,6 +16,7 @@ import { ACCEPTED_FILE } from "./accept.ts"; import type { Results } from "./report.ts"; import { alive, + ENGINES, isHttpUrl, label, loadConfig, @@ -26,6 +27,7 @@ import { selectorText, version, type CliOverrides, + type Engine, type ResolvedConfig, type Route, type Sides, @@ -120,7 +122,12 @@ function parseArgs(argv: string[]): Args { else if (v === "--right") a.right = value(); else if (v === "--out") a.outDir = value(); else if (v === "--config") a.config = value(); - else if (v === "--mask") (a.masks ??= []).push(value()); + else if (v === "--browser") { + const b = value(); + if (!ENGINES.includes(b as Engine)) + throw new Error(`--browser expects ${ENGINES.join(" or ")}, got ${b}`); + a.browser = b as Engine; + } else if (v === "--mask") (a.masks ??= []).push(value()); else if (v === "--only") (a.only ??= []).push(value()); else if (v === "--hide") (a.hideSelectors ??= []).push(value()); else if (v === "--selector" || v === "--left-selector" || v === "--right-selector") { @@ -258,6 +265,9 @@ const HELP = ` --out Output directory (default: ./renderdiff-out) --concurrency Parallel workers (default: half the cores, 2–8) --config Config file. Default: renderdiff.config.{ts,mts,js,mjs} in cwd. + --browser chromium or webkit; both sides are shot in it (default: chromium). + webkit writes to ./renderdiff-out-webkit by default and accepts + into renderdiff.accepted.webkit.json. --selector Compare one element (a Playwright selector) on both sides. --left-selector , --right-selector That side's selector, over --selector. @@ -283,8 +293,8 @@ const HELP = ` --no-recheck Don't reload a differing shot's right side to find content that varies per load. Off anyway under --cache-right. --computed Also diff every element's computed style and ::before/::after, - and links' and controls' :hover/:focus-visible, and say whether - the markup is identical. Pixels still decide. + and links' and controls' :hover/:focus-visible (Chromium only), + and say whether the markup is identical. Pixels still decide. --aria Also diff each page's accessibility tree (Playwright's ARIA snapshot) line by line. Pixels still decide. --sitemap[=] Add the paths in the right side's sitemap (one index level, files @@ -520,6 +530,7 @@ const PROBE_FLAGS = [ "--mask-media", "--no-freeze-video", "--storage-state", + "--browser", ]; /** `renderdiff probe `: load one URL several times and list what varies between loads. */ @@ -672,7 +683,11 @@ try { rerun: args.rerunFailed, cacheRight: args.cacheRight, recheck: args.recheck, - acceptedFile: path.join(configPath ? path.dirname(configPath) : process.cwd(), ACCEPTED_FILE), + acceptedFile: path.join( + configPath ? path.dirname(configPath) : process.cwd(), + // An engine's pixels never match the other's, so neither may prune the other's entries. + cfg.browser ? ACCEPTED_FILE.replace(/\.json$/, `.${cfg.browser}.json`) : ACCEPTED_FILE, + ), }), ); } catch (e) { diff --git a/src/config.ts b/src/config.ts index 7aaff94..96c2907 100644 --- a/src/config.ts +++ b/src/config.ts @@ -152,6 +152,9 @@ export const DEFAULT_WIDTHS = [1440, 1152, 896, 704, 375]; /** Directory a run writes its screenshots and `report.html` into. Relative to cwd. */ export const DEFAULT_OUT_DIR = "renderdiff-out"; +export const ENGINES = ["chromium", "webkit"] as const; +export type Engine = (typeof ENGINES)[number]; + /** * Captures run in parallel: half the cores, between 2 and 8. A run is bound by waiting on * pages, not CPU, and each worker holds two. No extra rate limiting needed: each worker @@ -326,6 +329,8 @@ export interface RenderdiffConfig { freezeVideo?: boolean; /** Hold every lottie-player on this frame once it loads. Unset: left playing. */ lottieFrame?: number; + /** The engine both sides are shot in. Default `chromium`. */ + browser?: Engine; } /** A fully merged config. Everything the run needs, nothing optional. */ @@ -350,6 +355,8 @@ export interface ResolvedConfig { lottieFrame?: number; /** Optional like `lottieFrame`. */ aria?: true; + /** Optional like `lottieFrame`: unset is Chromium. */ + browser?: "webkit"; } /** package.json's version; the same relative path resolves from `src/` and `dist/`. */ @@ -412,6 +419,7 @@ const CONFIG_KEYS = Object.keys({ maskMedia: 0, freezeVideo: 0, lottieFrame: 0, + browser: 0, } satisfies Record); const MASK_KEYS = Object.keys({ selector: 0, @@ -612,6 +620,8 @@ function validate(c: Record, file: string) { !(Number.isSafeInteger(c.lottieFrame) && (c.lottieFrame as number) >= 0) ) bad("lottieFrame", "a whole number ≥ 0"); + if (c.browser !== undefined && !ENGINES.includes(c.browser as Engine)) + bad("browser", `one of ${ENGINES.map((e) => `"${e}"`).join(", ")}`); } /** CLI-supplied values. `undefined` means "not passed", so the config file wins. */ @@ -632,6 +642,7 @@ export interface CliOverrides { hideSelectors?: string[]; maskMedia?: boolean; freezeVideo?: boolean; + browser?: Engine; } /** Merge built-in defaults, the config file, and CLI flags into one resolved config. */ @@ -664,13 +675,17 @@ export function resolveConfig(file: RenderdiffConfig, cli: CliOverrides): Resolv ...(cli.hideSelectors ?? []), ]; const freezeVideo = cli.freezeVideo ?? file.freezeVideo ?? true; + const webkit = (cli.browser ?? file.browser) === "webkit"; return { left: left.replace(/\/+$/, ""), right: right.replace(/\/+$/, ""), widths: cli.widths ?? file.widths ?? [...DEFAULT_WIDTHS], threshold: cli.threshold ?? file.threshold ?? null, - outDir: path.resolve(process.cwd(), cli.outDir ?? file.outDir ?? DEFAULT_OUT_DIR), + outDir: path.resolve( + process.cwd(), + cli.outDir ?? file.outDir ?? (webkit ? `${DEFAULT_OUT_DIR}-webkit` : DEFAULT_OUT_DIR), + ), concurrency: cli.concurrency ?? file.concurrency ?? DEFAULT_CONCURRENCY, blockUrlPatterns: [...DEFAULT_BLOCK_URL_PATTERNS, ...(file.blockUrlPatterns ?? [])], hideSelectors, @@ -698,6 +713,7 @@ export function resolveConfig(file: RenderdiffConfig, cli: CliOverrides): Resolv freezeVideo, lottieFrame: file.lottieFrame, aria: (cli.aria ?? file.aria) || undefined, + ...(webkit ? { browser: "webkit" as const } : {}), }; } diff --git a/src/locate.ts b/src/locate.ts index b70b9ab..811d695 100644 --- a/src/locate.ts +++ b/src/locate.ts @@ -1,5 +1,5 @@ import type { Locator, Page } from "playwright"; -import type { Shot } from "./capture.ts"; +import { webkit, type Shot } from "./capture.ts"; import type { Box, Spot } from "./compare.ts"; import type { Snapshot } from "./computed.ts"; @@ -595,6 +595,8 @@ export async function snapshotStates( ): Promise<{ snap: Snapshot; unchecked: number }> { const t0 = Date.now(); const run = await walker(page, masks, budget, root, hide); + // Forcing a state needs CDP, which only Chromium has. + if (webkit(page)) return { snap: (await run()).snap!, unchecked: 0 }; const client = await page.context().newCDPSession(page); try { const { snap, states, unchecked } = await run({ collect: MAX_STATES }); diff --git a/src/probe.ts b/src/probe.ts index b05c27f..e5f9e49 100644 --- a/src/probe.ts +++ b/src/probe.ts @@ -69,7 +69,7 @@ export async function probe( cfg: ResolvedConfig, loads: number, ): Promise { - const { browser } = await launch(); + const { browser } = await launch(cfg.browser); const now = Math.floor(Date.now() / 60_000) * 60_000; const done: { snap: Snapshot; row: Record }[] = []; let next = 0; diff --git a/src/run.ts b/src/run.ts index b775556..f025d9e 100644 --- a/src/run.ts +++ b/src/run.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; -import { chromium, errors, type Browser, type BrowserContext, type Page } from "playwright"; +import { chromium, errors, webkit, type Browser, type BrowserContext, type Page } from "playwright"; import sharp from "sharp"; import { applyAccepted, readAccepted, unacceptable, updateAccepted } from "./accept.ts"; import { CACHE_DIR, cacheFile, replayable, REPLAY_TTL, saveRecording, shotId } from "./cache.ts"; @@ -305,7 +305,7 @@ export async function run({ const reused = new Set(results); const stems = slugs(routes); - const launched = jobs.length ? await launch() : null; + const launched = jobs.length ? await launch(cfg.browser) : null; const queue = [...jobs]; let done = 0; let down = ""; @@ -715,6 +715,10 @@ export async function run({ if (progress) { console.log(`Capturing ${jobs.length} pair(s) with ${cfg.concurrency} worker(s)...\n`); } + if (cfg.computed && cfg.browser && jobs.length) + console.log( + `--computed on ${cfg.browser}: links and controls aren't forced into :hover/:focus-visible (that needs Chromium)`, + ); if (launched) { await Promise.all( Array.from({ length: cfg.concurrency }, (_, i) => worker(launched.browser, i)), @@ -909,6 +913,12 @@ export async function merge(fullDir: string, partialDir: string, outDir: string) `${side} differs: ${full.meta[side]} in ${fullDir}, ${partial.meta[side]} in ${partialDir}`, ); } + const engine = (r: Results) => (r.meta.browser.name === "webkit" ? "webkit" : "chromium"); + if (engine(full) !== engine(partial)) + throw new Error( + `browser differs: ${engine(full)} in ${fullDir}, ${engine(partial)} in ${partialDir}; ` + + `one engine's pixels never match the other's`, + ); checkOut([fullDir, partialDir], outDir); const key = (r: RouteResult) => `${name(r)}@${r.width}`; @@ -1065,7 +1075,17 @@ const ARGS = [ * A missing browser binary is the likeliest first-run failure, so fall back to the system * Chrome (both sides share it, so the comparison stays symmetric), then name the fix. */ -export async function launch() { +export async function launch(browser?: "webkit") { + if (browser === "webkit") { + try { + return { browser: await webkit.launch(), channel: "webkit" }; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + throw new Error( + `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit`, + ); + } + } try { return { browser: await chromium.launch({ args: ARGS }), channel: null }; } catch (e) { diff --git a/tests/fixtures/pages/README.md b/tests/fixtures/pages/README.md index e404ccc..52119dc 100644 --- a/tests/fixtures/pages/README.md +++ b/tests/fixtures/pages/README.md @@ -20,7 +20,7 @@ Three groups: elements each change names (`margin`, `color`, `nav`), and a lost `::before` line and a re-parented pairing (`divider`). - **Guard rails** — `blank`, `tall`, `animated`, `lazy`, `lazy-deep`, `lazy-js`, `lazy-grow`, `consent`, - `drifting`, `tracker`, `clock`, `ticker`, `unsettled`, `unsettled-once`, `sprite`, `short`, `tall-region`, `reorder`, `stripe`, `frameset`, `menu`, `scroll-spy`, `signed-in`, `fixed`, `inserted`, `labelled`, `broken-asset`, `fonts`, `media`, `long`, `video`, `video-seek`, `lottie`, `hang`, `origin`, `cooperate`, `mask-geometry`, `bistable`, `offcanvas`, `appshell`, `smooth-scroll`, `rules`, `prefix`, `prefix-same`, `reflowing`, `waapi`, `geometry`, `stalled-frame`. + `drifting`, `tracker`, `clock`, `ticker`, `unsettled`, `unsettled-once`, `sprite`, `short`, `tall-region`, `reorder`, `stripe`, `frameset`, `menu`, `scroll-spy`, `signed-in`, `fixed`, `inserted`, `labelled`, `broken-asset`, `fonts`, `media`, `long`, `video`, `video-seek`, `lottie`, `hang`, `origin`, `cooperate`, `mask-geometry`, `bistable`, `offcanvas`, `appshell`, `smooth-scroll`, `rules`, `prefix`, `prefix-same`, `box-decoration-break`, `reflowing`, `waapi`, `geometry`, `stalled-frame`. One per defence in `src/capture.ts` / `src/config.ts`. ## Authoring rules diff --git a/tests/fixtures/pages/box-decoration-break/left.html b/tests/fixtures/pages/box-decoration-break/left.html new file mode 100644 index 0000000..bb7840b --- /dev/null +++ b/tests/fixtures/pages/box-decoration-break/left.html @@ -0,0 +1,25 @@ + + + + +Box decoration break + + + + +
+

Box decoration break

+

A highlighted phrase that wraps onto several lines of text inside its narrow column

+
+

Notes

+

Each line of a wrapped inline box gets its own border, padding and corners only when the browser reads the declaration; otherwise the box is sliced at each line break.

+
+
+ + diff --git a/tests/fixtures/pages/box-decoration-break/right.html b/tests/fixtures/pages/box-decoration-break/right.html new file mode 100644 index 0000000..6da0caf --- /dev/null +++ b/tests/fixtures/pages/box-decoration-break/right.html @@ -0,0 +1,25 @@ + + + + +Box decoration break + + + + +
+

Box decoration break

+

A highlighted phrase that wraps onto several lines of text inside its narrow column

+
+

Notes

+

Each line of a wrapped inline box gets its own border, padding and corners only when the browser reads the declaration; otherwise the box is sliced at each line break.

+
+
+ + diff --git a/tests/helpers/corpus.ts b/tests/helpers/corpus.ts index 364c032..b984934 100644 --- a/tests/helpers/corpus.ts +++ b/tests/helpers/corpus.ts @@ -71,6 +71,7 @@ export const GUARDRAILS = [ "rules", "prefix", "prefix-same", + "box-decoration-break", "reflowing", "waapi", "geometry", diff --git a/tests/unit/cli-args.test.ts b/tests/unit/cli-args.test.ts index e8054b0..ed42017 100644 --- a/tests/unit/cli-args.test.ts +++ b/tests/unit/cli-args.test.ts @@ -75,7 +75,7 @@ describe("help and version", () => { if (!options.includes(f)) problems.push(`the README table lists ${f}, --help's Options don't`); - // yarn install in docs/case-study.md; wrangler and git in docs/output.md; gh in skills/renderdiff/SKILL.md; git worktree in skills/renderdiff/reference.md + // yarn install in docs/case-study.md; playwright install in README.md; wrangler and git in docs/output.md; gh in skills/renderdiff/SKILL.md; git worktree in skills/renderdiff/reference.md const foreign = new Set([ "--immutable", "--production-branch", @@ -86,6 +86,7 @@ describe("help and version", () => { "--detach", "--format", "--force", + "--with-deps", ]); for (const f of ["README.md", ...md("docs"), ...md("skills")]) for (const flag of flags(read(f))) @@ -189,6 +190,12 @@ describe("flag validation", () => { await rejects(["/a", "--left", L, "--right", R, "--width="], "--width needs a value"); }); + test("--browser takes chromium or webkit only", () => + rejects( + ["/a", "--left", L, "--right", R, "--browser", "firefox"], + "--browser expects chromium or webkit, got firefox", + )); + test("--width rejects fractions", () => rejects( ["/a", "--left", L, "--right", R, "--width", "1.5"], diff --git a/tests/unit/config-load.test.ts b/tests/unit/config-load.test.ts index 560b3a3..92a0d6f 100644 --- a/tests/unit/config-load.test.ts +++ b/tests/unit/config-load.test.ts @@ -198,6 +198,7 @@ describe("validation", () => { ['{ freezeVideo: "no" }', /`freezeVideo` must be a boolean/], ["{ lottieFrame: 1.5 }", /`lottieFrame` must be a whole number/], ["{ lottieFrame: -1 }", /`lottieFrame` must be a whole number/], + ['{ browser: "firefox" }', /`browser` must be one of "chromium", "webkit"/], ["{ storageState: 1 }", /`storageState` must be a non-empty path/], [ '{ storageState: { left: "a.json", center: "b.json" } }', diff --git a/tests/unit/report.test.ts b/tests/unit/report.test.ts index ee94072..6bb9a4b 100644 --- a/tests/unit/report.test.ts +++ b/tests/unit/report.test.ts @@ -465,6 +465,11 @@ describe("renderdiff merge", () => { const [a, b] = [full(), partial()]; const other = partial([], { right: "https://other.example" }); assert.match((await merge(a, other, path.join(tempDir(), "o"))).stderr, /✗ right differs/); + const engine = partial([], { browser: { name: "webkit", version: "26.5" } }); + assert.match( + (await merge(a, engine, path.join(tempDir(), "o"))).stderr, + /✗ browser differs: chromium in .*, webkit in /, + ); const self = await merge(a, b, a); assert.equal(self.code, 2); assert.match(self.stderr, /is or is inside/); diff --git a/tests/visual/webkit.test.ts b/tests/visual/webkit.test.ts new file mode 100644 index 0000000..813d313 --- /dev/null +++ b/tests/visual/webkit.test.ts @@ -0,0 +1,136 @@ +/** + * `--browser webkit`: both sides shot in Playwright's WebKit, which has no CDP. The capture, + * the stalled-frame and tall-page guard rails and `--computed` still work there; the case + * the option exists for is CSS Chromium reads one way and WebKit another. + */ +import assert from "node:assert/strict"; +import fs from "node:fs"; +import path from "node:path"; +import { after, before, test } from "node:test"; +import sharp from "sharp"; +import { + cleanupTempDirs, + context, + leftCss, + renderdiffArgs, + runCli, + tempDir, +} from "../helpers/cli.ts"; +import { result, results } from "../helpers/parse.ts"; +import { startFixtureSite, type FixtureSite } from "../helpers/server.ts"; +import { EXPECT_HEIGHT } from "../fixtures/pages/tall/page.ts"; + +let site: FixtureSite; + +before(async () => { + site = await startFixtureSite(); +}); + +after(async () => { + await site?.close(); + cleanupTempDirs(); +}); + +const run = (routes: string[], extra: string[] = [], widths = [1440], engine = "webkit") => + runCli([ + ...renderdiffArgs({ left: site.left, right: site.right, routes, widths }), + "--browser", + engine, + ...extra, + ]); + +test("a dropped -webkit- prefix is parity in Chromium and a diff in WebKit", async () => { + const chromium = await run(["/box-decoration-break"], [], [1440], "chromium"); + assert.equal(chromium.code, 0, context(chromium)); + assert.equal(result(results(chromium.outDir), "/box-decoration-break", 1440).diffPixels, 0); + const webkit = await run(["/box-decoration-break"]); + assert.equal(webkit.code, 1, context(webkit)); + const out = results(webkit.outDir); + assert.equal(out.meta.browser.name, "webkit"); + assert.ok(result(out, "/box-decoration-break", 1440).diffPixels > 0, context(webkit)); +}); + +test("an equivalence pair scores 100% and a control does not", async () => { + const r = await run(["/flex-vs-grid", "/color"], [], [1440, 375]); + assert.equal(r.code, 1, context(r)); + for (const width of [1440, 375]) { + assert.equal(result(results(r.outDir), "/flex-vs-grid", width).score, 1, context(r)); + assert.ok(result(results(r.outDir), "/color", width).score < 1, context(r)); + } +}); + +test("a page past the one-shot height is tiled and stitched to its full height", async () => { + const r = await run(["/tall"]); + assert.equal(r.code, 0, context(r)); + assert.equal(result(results(r.outDir), "/tall", 1440).diffPixels, 0, context(r)); + for (const side of ["left", "right"]) { + const { height } = await sharp(r.file(`tall.1440.${side}.png`)).metadata(); + assert.equal(height, EXPECT_HEIGHT, context(r)); + } +}); + +test("a frame that never loads is skipped, not waited on", async () => { + const t0 = Date.now(); + const r = await run(["/stalled-frame"]); + assert.equal(r.code, 0, context(r)); + assert.ok(Date.now() - t0 < 30_000, `took ${Date.now() - t0}ms${context(r)}`); +}); + +test("an animated GIF isn't frozen: its pixels are unstable, not a difference", async () => { + const r = await run(["/animated"]); + assert.equal(r.code, 0, context(r)); + const shot = result(results(r.outDir), "/animated", 1440); + assert.equal(shot.diffPixels, 0, context(r)); + assert.ok(shot.unstablePixels > 0, context(r)); +}); + +test("--computed works without forced states, and says so", async () => { + const r = await run(["/margin"], ["--computed"]); + assert.equal(r.code, 1, context(r)); + assert.match(r.stdout, /--computed on webkit: links and controls aren't forced/); + const shot = result(results(r.outDir), "/margin", 1440); + assert.equal(shot.computed?.dom, 0, context(r)); + assert.ok( + shot.computed!.changes.some((c) => c.prop === "margin-left"), + context(r), + ); +}); + +test("--accept writes renderdiff.accepted.webkit.json and leaves Chromium's file alone", async () => { + const cwd = tempDir("renderdiff-cwd-"); + const chromiumFile = path.join(cwd, "renderdiff.accepted.json"); + fs.writeFileSync(chromiumFile, "[]\n"); + const hook = leftCss(cwd, ".notes h2 { color: #2e7d32 }"); + const r = await runCli( + [ + ...renderdiffArgs({ left: site.left, right: site.right, routes: ["/"], widths: [1440] }), + ...hook, + "--browser", + "webkit", + "--accept", + "known", + ], + { cwd }, + ); + assert.equal(r.code, 0, context(r)); + const list = JSON.parse( + fs.readFileSync(path.join(cwd, "renderdiff.accepted.webkit.json"), "utf8"), + ); + assert.ok(list.length > 0, context(r)); + assert.equal(fs.readFileSync(chromiumFile, "utf8"), "[]\n"); +}); + +test("without --out a webkit run writes renderdiff-out-webkit", async () => { + const cwd = tempDir("renderdiff-cwd-"); + const r = await runCli( + [ + ...renderdiffArgs({ left: site.left, right: site.right, routes: ["/"], widths: [375] }), + "--browser", + "webkit", + ], + { cwd, noOut: true }, + ); + assert.equal(r.code, 0, `${r.stdout}\n${r.stderr}`); + assert.ok(fs.existsSync(path.join(cwd, "renderdiff-out-webkit", "results.json"))); + assert.ok(!fs.existsSync(path.join(cwd, "renderdiff-out"))); +}); From 5911e5fb1478f4806d7a64b57c00c0d361074bee Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:08:08 +0100 Subject: [PATCH 2/4] webkit: stop a load only after the shot times out, and record it window.stop() fails the page's open requests (a pending fetch rejects, images break), so it is a last resort: the WebKit shot gets 5s, and only past that is the load stopped and the shot retaken, with the side noted as stopped in results.json and the row. GIF docs say "usually" caught as unstable; the Action's browser input is documented as deciding over the config; the rerun refusal names --browser; the missing-WebKit hint names --with-deps. Tests trimmed per review. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/action.yml | 4 ++-- README.md | 2 +- action.yml | 2 +- docs/github-action.md | 2 +- docs/how-it-works.md | 13 +++++++++--- docs/output.md | 1 + renderdiff.config.example.ts | 4 ++++ skills/renderdiff/reference.md | 3 ++- src/capture.ts | 27 +++++++++++++++++++----- src/locate.ts | 4 ++-- src/report.ts | 2 ++ src/run.ts | 9 ++++++-- tests/unit/cache.test.ts | 18 ++++++++++++++++ tests/visual/webkit.test.ts | 38 ++++++++++++---------------------- 14 files changed, 86 insertions(+), 43 deletions(-) diff --git a/.github/workflows/action.yml b/.github/workflows/action.yml index c088cca..8d0392d 100644 --- a/.github/workflows/action.yml +++ b/.github/workflows/action.yml @@ -63,7 +63,7 @@ jobs: with: left: ${{ steps.site.outputs.left }} right: ${{ steps.site.outputs.right }} - routes: /flex-vs-grid /box-decoration-break + routes: /box-decoration-break widths: "1440" browser: webkit out: webkit-out @@ -91,7 +91,7 @@ jobs: test "$OUTCOME" = failure test "$DOWN" = 2 test "$WEBKIT" = 1 - node -e 'const { meta, results: r } = require("./webkit-out/results.json"); if (meta.browser.name !== "webkit" || r.find((x) => x.url === "/flex-vs-grid").diffPixels !== 0 || !(r.find((x) => x.url === "/box-decoration-break").diffPixels > 0)) throw meta.browser' + node -e 'const { meta } = require("./webkit-out/results.json"); if (meta.browser.name !== "webkit") throw meta.browser' node -e 'const r = require("./differ-out/results.json").results; for (const p of ["/color", "/tint"]) if (!(r.find((x) => x.url === p)?.score < 0.999)) throw p' grep -q '"/color"' accept-out/renderdiff.accepted.json grep -q '' "$RUNNER_TEMP/renderdiff-comment-differ.md" diff --git a/README.md b/README.md index 1f7d719..7a9ac91 100644 --- a/README.md +++ b/README.md @@ -108,7 +108,7 @@ renderdiff [options] | `--out ` | Output directory, wiped at the start of a run. Default `./renderdiff-out`. A directory holding files renderdiff didn't write is refused. | | `--concurrency ` | Parallel workers. Default half the CPU cores, between 2 and 8. Lower it when one side is someone's live site. | | `--config ` | Config file. Default: `renderdiff.config.{ts,mts,js,mjs}` in cwd. | -| `--browser ` | `chromium` (default) or `webkit`: both sides are shot in it, for CSS that only Safari reads differently, such as a dropped `-webkit-` prefix Chromium no longer needs. WebKit writes to `./renderdiff-out-webkit` unless `--out` or `outDir` say otherwise and accepts into `renderdiff.accepted.webkit.json`, so a Chromium and a WebKit run keep separate results. On WebKit, animated GIFs keep playing (their pixels show as unstable and aren't measured) and `--computed` doesn't force `:hover`/`:focus-visible`. | +| `--browser ` | `chromium` (default) or `webkit`: both sides are shot in it, for CSS that only Safari reads differently, such as a dropped `-webkit-` prefix Chromium no longer needs. WebKit writes to `./renderdiff-out-webkit` unless `--out` or `outDir` say otherwise and accepts into `renderdiff.accepted.webkit.json`, so a Chromium and a WebKit run keep separate results. On WebKit, animated GIFs keep playing (usually caught as unstable and left unmeasured; mask one if it flips), a load still open after 5s is stopped and recorded as `stopped`, and `--computed` doesn't force `:hover`/`:focus-visible`. In the GitHub Action its `browser` input decides, not the config's. | | `--selector ` | Compare the one element this Playwright selector matches, on both sides, instead of the whole page. Applies to every route without its own `@selector`, over the config's per-route `selector`. | | `--left-selector `, `--right-selector ` | That side's selector, over `--selector`. One without the other side (or `--selector`) is an error. | | `--mask ` | Paint this Playwright selector over on both sides, on every route; repeatable. Added to the config's masks and listed in the report as coverage given up. | diff --git a/action.yml b/action.yml index 22e8473..a35cd65 100644 --- a/action.yml +++ b/action.yml @@ -34,7 +34,7 @@ inputs: description: Contents of a Playwright storage-state JSON for both sides, from a secret. Written to a 0600 temp file, deleted after the run. default: "" accept: - description: Record this run's differences as accepted, with this reason. Commit the renderdiff.accepted.json from the artifact. + description: Record this run's differences as accepted, with this reason. Commit the renderdiff.accepted.json (renderdiff.accepted.webkit.json with browser webkit) from the artifact. default: "" wait-timeout: description: Whole seconds to wait for left to answer (any status under 500) before running. Past it, exit-code is 2. diff --git a/docs/github-action.md b/docs/github-action.md index 1fd7a40..97b7488 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -23,7 +23,7 @@ exits non-zero. | `left` | Your change: a preview URL or `http://localhost:3000`. Required. | | `right` | The reference, usually production. Required. | | `routes` | Separated by spaces or newlines; pairs (`/pricing::/plans`) work; per-route selectors go in the config, since the input splits on spaces. Optional when the config lists routes. | -| `browser` | `chromium` (default) or `webkit`, over the config's `browser`; both sides are shot in it, and `accept` records into `renderdiff.accepted.webkit.json` for WebKit. For both engines, run the action twice. | +| `browser` | `chromium` (default) or `webkit`. It decides: the config's `browser` key is ignored in the Action. Both sides are shot in it, and `accept` records into `renderdiff.accepted.webkit.json` for WebKit. For both engines, run the action twice. | | `config` | Config file path. Default: `renderdiff.config.{ts,mts,js,mjs}` in the workspace. | | `widths` | Spaces or commas. Default renderdiff's five. | | `threshold` | Fail only below this score. Without it, any difference fails the job (exit 1); `"0"` never fails on one. | diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 96a7478..7d8bd9f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -24,9 +24,16 @@ honestly. This is what renderdiff does to make sure of that. `behavior: "smooth"` in script scrolls, from before any hook), and hides common consent banners and dev toolbars. Animated images (GIF, WebP, APNG) and SVG animations show their first frame; it is a Blink setting, so cross-origin and CSS background images freeze too. WebKit - (`--browser webkit`) has no such setting: there an animated image keeps playing, so its - pixels are caught as unstable, painted out of the score and can't be accepted. Mask one - that flips between the two sides. + (`--browser webkit`) has no such setting: there an animated image keeps playing, which is + usually caught as unstable (painted out of the score, never accepted). One with slow + frames can hold still for both shots and land on a different frame per side: mask it if + it flips. +- **On WebKit, stops a load that never ends**, as a last resort. WebKit's `fonts.ready`, + which its screenshot waits on, holds until the load event, so a frame that never loads + would hang the shot. When a shot takes over 5s, renderdiff calls `window.stop()` and + shoots again. That fails the page's open requests (a pending `fetch` rejects, images + break), which can show as a difference, so the shot records it: `stopped loading on right` + in the row, `stopped` in results.json. - **Settles the page** — makes native lazy images eager, scrolls every viewport-height slice a frame at a time, to the bottom even as the page grows, so script lazy loaders fire, waits for every image to decode, `document.fonts.ready` (at most 5s) and up to 3s for the network to go 250ms with nothing in flight diff --git a/docs/output.md b/docs/output.md index 9da0d69..ea120fc 100644 --- a/docs/output.md +++ b/docs/output.md @@ -559,6 +559,7 @@ file, not each line), scores rounded down to four decimals (so a shot that diffe "failed": { "left": [], "right": [] }, // stylesheets and font faces that didn't load: "font Inter 700", "stylesheet https://…" "fonts": { "added": [], "removed": ["Inter 600"] }, // web font faces one side loaded; absent when equal "masked": ["rotating quote (data-renderdiff-mask)"], // absent unless a side marked elements data-renderdiff-mask + "stopped": ["right"], // WebKit only, absent unless a load still open after 5s was stopped for the shot "computed": { "dom": 1, "changes": [], "truncated": 0, "unmatched": { "left": [], "right": ["body > nav > a[href=\"/pricing\"]"] }, "rules": [{ "side": "left", "sheet": "docs.css", "conditions": ["@media (width >= 60em)"], "rules": 5, "elements": 67 }], "prefixes": { "left": ["-webkit-user-select: none"], "right": [] } }, // --computed only; null: snapshot failed, "computedError" says why; rules: one-sided rule groups, elements they could style here; prefixes: { left, right }, prefixed declarations only that side's CSS source has diff --git a/renderdiff.config.example.ts b/renderdiff.config.example.ts index 03633fe..e7b0d9f 100644 --- a/renderdiff.config.example.ts +++ b/renderdiff.config.example.ts @@ -99,6 +99,10 @@ export default { // they keep playing. // lottieFrame: 60, + // Shoot both sides in WebKit instead of Chromium, for CSS only Safari reads differently. + // --browser overrides it; in the GitHub Action the browser input decides and this is ignored. + // browser: "webkit", + // Mask every iframe on every route (embeds that differ per load). Same as --mask-media. maskMedia: false, diff --git a/skills/renderdiff/reference.md b/skills/renderdiff/reference.md index 905b7dd..66a18a3 100644 --- a/skills/renderdiff/reference.md +++ b/skills/renderdiff/reference.md @@ -473,7 +473,8 @@ commas: `JSON.parse` the whole file, not each line. Paths are relative to the ou and the run says so; otherwise it prints the `npx playwright install chromium` hint. `--browser webkit` shoots both sides in WebKit (`npx playwright install webkit`) into `renderdiff-out-webkit`, accepting into `renderdiff.accepted.webkit.json`: for CSS only - Safari reads differently. There animated GIFs aren't frozen (unstable, unmeasured), and + Safari reads differently. There animated GIFs aren't frozen (usually caught as unstable; + mask one if it flips), a load still open after 5s is stopped (`stopped loading on `), and `--computed` forces no `:hover`/`:focus-visible`. `merge` refuses to mix engines. - **Pinned on both sides:** clock (one instant per run, floored to the minute), seeded `Math.random`, UTC, en-US, light scheme, reduced motion; animations and transitions stilled, diff --git a/src/capture.ts b/src/capture.ts index d41190c..9c73d4c 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -1,5 +1,12 @@ import { setTimeout as sleep } from "node:timers/promises"; -import type { CDPSession, Locator, Page, Request, Response } from "playwright"; +import { + errors, + type CDPSession, + type Locator, + type Page, + type Request, + type Response, +} from "playwright"; import sharp from "sharp"; import { label, MASK_COLOR, masksFor, type ResolvedConfig, type Route } from "./config.ts"; @@ -26,6 +33,8 @@ export interface Shot { fonts: string[]; /** Reasons of the page's `data-renderdiff-mask` elements, e.g. `rotating quote (data-renderdiff-mask)`. */ marked: string[]; + /** WebKit only: the shot timed out on a load still open (fonts.ready waits on it), so it was stopped. */ + stopped?: true; } /** @@ -366,6 +375,7 @@ export async function capture( await page.clock.setSystemTime(now); // Here, not on framenavigated, which a pushState during hydration fires too. network.get(page)!.failed.clear(); + stopped.delete(page); doc.delete(page); const res = await page .goto(url, { waitUntil: "domcontentloaded", timeout: 45_000 }) @@ -538,8 +548,6 @@ export async function capture( `at ${url} — likely a broken layout, not a real page`, ); } - // WebKit holds fonts.ready, which its screenshot waits on, until the load event, which a stalled frame never fires. - if (webkit(page)) await page.evaluate(() => document.readyState === "complete" || window.stop()); const tiled = area.height > MAX_CAPTURE_HEIGHT; // Read on both sides of the shot, so a box that moves while it is taken stays covered. const shoot = async () => { @@ -572,6 +580,7 @@ export async function capture( failed, fonts: loadedFonts, marked, + ...(stopped.has(page) ? { stopped: true as const } : {}), }; prev = png; } @@ -724,11 +733,19 @@ async function captureTiled(page: Page, box: Box): Promise { const sessions = new WeakMap>(); -export const webkit = (page: Page) => page.context().browser()?.browserType().name() === "webkit"; +export const isWebKit = (page: Page) => page.context().browser()?.browserType().name() === "webkit"; +const stopped = new WeakSet(); /** A PNG of `clip`, in page px, on the page's one CDP session; WebKit has none, and renders it all offscreen. */ async function grab(page: Page, clip: Box, beyond: boolean): Promise { - if (webkit(page)) return page.screenshot({ fullPage: true, clip }); + if (isWebKit(page)) + return page.screenshot({ fullPage: true, clip, timeout: 5_000 }).catch(async (e) => { + if (!(e instanceof errors.TimeoutError) || stopped.has(page)) throw e; + // WebKit's fonts.ready, which its screenshot waits on, holds until the load event; a stalled frame never fires it. + await page.evaluate(() => window.stop()); + stopped.add(page); + return page.screenshot({ fullPage: true, clip }); + }); let s = sessions.get(page); if (!s) sessions.set(page, (s = page.context().newCDPSession(page))); const { data } = await ( diff --git a/src/locate.ts b/src/locate.ts index 811d695..c8a82ec 100644 --- a/src/locate.ts +++ b/src/locate.ts @@ -1,5 +1,5 @@ import type { Locator, Page } from "playwright"; -import { webkit, type Shot } from "./capture.ts"; +import { isWebKit, type Shot } from "./capture.ts"; import type { Box, Spot } from "./compare.ts"; import type { Snapshot } from "./computed.ts"; @@ -596,7 +596,7 @@ export async function snapshotStates( const t0 = Date.now(); const run = await walker(page, masks, budget, root, hide); // Forcing a state needs CDP, which only Chromium has. - if (webkit(page)) return { snap: (await run()).snap!, unchecked: 0 }; + if (isWebKit(page)) return { snap: (await run()).snap!, unchecked: 0 }; const client = await page.context().newCDPSession(page); try { const { snap, states, unchecked } = await run({ collect: MAX_STATES }); diff --git a/src/report.ts b/src/report.ts index f464b3c..88a4e1b 100644 --- a/src/report.ts +++ b/src/report.ts @@ -74,6 +74,8 @@ export interface ComparedResult extends BaseResult { text: TextDiff; /** Reasons of `data-renderdiff-mask` elements either side carried. Absent when none did. */ masked?: string[]; + /** WebKit: sides whose load was stopped so the shot could be taken; their pending requests failed. */ + stopped?: ("left" | "right")[]; /** Paths relative to the out dir. `diff` is absent only for a zero-diff shot in `renderdiff pack` output. */ images: { left: string; right: string; diff?: string }; /** At most three, worst first: one-sided or text changes, then property changes, then shifts. */ diff --git a/src/run.ts b/src/run.ts index f025d9e..5dbb0a5 100644 --- a/src/run.ts +++ b/src/run.ts @@ -209,7 +209,7 @@ function previous(file: string, now: Record): Results | string for (const [key, value] of Object.entries(now)) { if (key === "config" && value !== was[key]) { return ( - `the config (masks, hooks, hidden selectors, blocked URLs, CSS, storage state, --computed, --aria) ` + + `the config (masks, hooks, hidden selectors, blocked URLs, CSS, storage state, --computed, --aria, --browser) ` + `differs from the one that wrote ${file}; run without --rerun-failed to start over, ` + `or run the routes you changed into a new --out and renderdiff merge it with this one` ); @@ -634,6 +634,9 @@ export async function run({ if (!rightShot.failed.length) recorded.push(shotId(route, width)); const marked = [...new Set([...leftShot.marked, ...rightShot.marked])]; + const stopped = (["left", "right"] as const).filter( + (s) => ({ left: leftShot, right: rightShot })[s].stopped, + ); const fonts = { added: leftShot.fonts.filter((f) => !rightShot.fonts.includes(f)), removed: rightShot.fonts.filter((f) => !leftShot.fonts.includes(f)), @@ -650,6 +653,7 @@ export async function run({ accepted: false, failed: { left: leftShot.failed, right: rightShot.failed }, ...(marked.length ? { masked: marked } : {}), + ...(stopped.length ? { stopped } : {}), ...(fonts.added.length || fonts.removed.length ? { fonts } : {}), ...computed, ...aria, @@ -661,6 +665,7 @@ export async function run({ const details = [ sel, assets ? `failed ${assets}` : "", + stopped.length ? `stopped loading on ${stopped.join(" and ")}` : "", unstableBox ? `unstable within ${fmtRegion(unstableBox)}` : "", regions.length ? `${regions.length} region${regions.length > 1 ? "s" : ""}, largest ${fmtRegion(regions[0]!)}` @@ -1082,7 +1087,7 @@ export async function launch(browser?: "webkit") { } catch (e) { const msg = e instanceof Error ? e.message : String(e); throw new Error( - `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit`, + `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit (--with-deps on Linux)`, ); } } diff --git a/tests/unit/cache.test.ts b/tests/unit/cache.test.ts index 0bb913f..b947899 100644 --- a/tests/unit/cache.test.ts +++ b/tests/unit/cache.test.ts @@ -100,6 +100,24 @@ describe("replayable", () => { }); describe("keys", () => { + test("a webkit run is its own config, out dir and recording; Chromium's config is unchanged", () => { + const sides = { left: "http://l.test", right: "http://r.test" }; + const wk = resolveConfig({}, { ...sides, browser: "webkit" }); + assert.equal(wk.browser, "webkit"); + assert.equal(path.basename(wk.outDir), "renderdiff-out-webkit"); + assert.equal( + path.basename(resolveConfig({ browser: "webkit" }, sides).outDir), + "renderdiff-out-webkit", + ); + const ch = resolveConfig({ browser: "chromium" }, sides); + assert.equal("browser" in ch, false); + assert.equal(path.basename(ch.outDir), "renderdiff-out"); + assert.notEqual( + cacheFile(cfg({ browser: "webkit" }), { name: "webkit", version: "1" }), + cacheFile(cfg(), browser), + ); + }); + test("a hook, the right path or the width is a different shot", () => { const hooked = parseRoutes(["/a"], undefined, async () => {})[0]!; const paired = parseRoutes(["/b::/a"])[0]!; diff --git a/tests/visual/webkit.test.ts b/tests/visual/webkit.test.ts index 813d313..bdc54f4 100644 --- a/tests/visual/webkit.test.ts +++ b/tests/visual/webkit.test.ts @@ -50,13 +50,9 @@ test("a dropped -webkit- prefix is parity in Chromium and a diff in WebKit", asy assert.ok(result(out, "/box-decoration-break", 1440).diffPixels > 0, context(webkit)); }); -test("an equivalence pair scores 100% and a control does not", async () => { - const r = await run(["/flex-vs-grid", "/color"], [], [1440, 375]); - assert.equal(r.code, 1, context(r)); - for (const width of [1440, 375]) { - assert.equal(result(results(r.outDir), "/flex-vs-grid", width).score, 1, context(r)); - assert.ok(result(results(r.outDir), "/color", width).score < 1, context(r)); - } +test("an equivalence pair scores 100%", async () => { + const r = await run(["/flex-vs-grid"]); + assert.equal(result(results(r.outDir), "/flex-vs-grid", 1440).score, 1, context(r)); }); test("a page past the one-shot height is tiled and stitched to its full height", async () => { @@ -69,11 +65,18 @@ test("a page past the one-shot height is tiled and stitched to its full height", } }); -test("a frame that never loads is skipped, not waited on", async () => { +test("a frame that never loads is stopped after the shot times out, and the stop recorded", async () => { const t0 = Date.now(); - const r = await run(["/stalled-frame"]); + const r = await run(["/stalled-frame"], ["--progress"]); assert.equal(r.code, 0, context(r)); - assert.ok(Date.now() - t0 < 30_000, `took ${Date.now() - t0}ms${context(r)}`); + assert.ok(Date.now() - t0 < 45_000, `took ${Date.now() - t0}ms${context(r)}`); + assert.deepEqual(result(results(r.outDir), "/stalled-frame", 1440).stopped, ["left", "right"]); + assert.match(r.stdout, /stopped loading on left and right/, context(r)); +}); + +test("a page that finishes loading is never stopped", async () => { + const r = await run(["/"]); + assert.equal(result(results(r.outDir), "/", 1440).stopped, undefined, context(r)); }); test("an animated GIF isn't frozen: its pixels are unstable, not a difference", async () => { @@ -119,18 +122,3 @@ test("--accept writes renderdiff.accepted.webkit.json and leaves Chromium's file assert.ok(list.length > 0, context(r)); assert.equal(fs.readFileSync(chromiumFile, "utf8"), "[]\n"); }); - -test("without --out a webkit run writes renderdiff-out-webkit", async () => { - const cwd = tempDir("renderdiff-cwd-"); - const r = await runCli( - [ - ...renderdiffArgs({ left: site.left, right: site.right, routes: ["/"], widths: [375] }), - "--browser", - "webkit", - ], - { cwd, noOut: true }, - ); - assert.equal(r.code, 0, `${r.stdout}\n${r.stderr}`); - assert.ok(fs.existsSync(path.join(cwd, "renderdiff-out-webkit", "results.json"))); - assert.ok(!fs.existsSync(path.join(cwd, "renderdiff-out"))); -}); From e19de7b3f2486f5e54b3ba90f835151f12321a01 Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:56:18 +0100 Subject: [PATCH 3/4] engine: one object per browser instead of isWebKit checks src/engine.ts holds an Engine interface with a chromium and a webkit implementation: output defaults, launch, grab, shot and forced states. capture, run, probe and the CLI resolve it once with engineOf(cfg) and pass it to pageShot, captureTiled and snapshotStates, so isWebKit, the launch branch, browserOf and the out-dir and accepted-file ternaries go. A stalled WebKit shot is no longer retaken: window.stop() lets the shot already in flight finish. Its 5s now covers a whole tiled shot, not each tile. The stopped flag is a local in capture, not a WeakSet. Co-Authored-By: Claude Opus 5.5 --- docs/how-it-works.md | 4 +- src/accept.ts | 2 - src/capture.ts | 66 ++++----------- src/cli.ts | 10 +-- src/config.ts | 21 +++-- src/engine.ts | 148 ++++++++++++++++++++++++++++++++++ src/locate.ts | 37 ++------- src/probe.ts | 9 ++- src/run.ts | 61 +++----------- tests/visual/computed.test.ts | 7 +- 10 files changed, 206 insertions(+), 159 deletions(-) create mode 100644 src/engine.ts diff --git a/docs/how-it-works.md b/docs/how-it-works.md index 7d8bd9f..a49393f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -30,8 +30,8 @@ honestly. This is what renderdiff does to make sure of that. it flips. - **On WebKit, stops a load that never ends**, as a last resort. WebKit's `fonts.ready`, which its screenshot waits on, holds until the load event, so a frame that never loads - would hang the shot. When a shot takes over 5s, renderdiff calls `window.stop()` and - shoots again. That fails the page's open requests (a pending `fetch` rejects, images + would hang the shot. When a shot takes over 5s, renderdiff calls `window.stop()`, which + lets it finish. That fails the page's open requests (a pending `fetch` rejects, images break), which can show as a difference, so the shot records it: `stopped loading on right` in the row, `stopped` in results.json. - **Settles the page** — makes native lazy images eager, scrolls every viewport-height diff --git a/src/accept.ts b/src/accept.ts index ed13915..705ae8e 100644 --- a/src/accept.ts +++ b/src/accept.ts @@ -4,8 +4,6 @@ import { name } from "./config.ts"; import { fmtElements } from "./format.ts"; import type { ComparedResult, RouteResult } from "./report.ts"; -export const ACCEPTED_FILE = "renderdiff.accepted.json"; - /** * One band or region, without its `y`: a change above it moves it down the page without * changing it. `hash` and `elements` tell a new change in the same spot from the accepted one. diff --git a/src/capture.ts b/src/capture.ts index 9c73d4c..5b8fae1 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -1,14 +1,8 @@ import { setTimeout as sleep } from "node:timers/promises"; -import { - errors, - type CDPSession, - type Locator, - type Page, - type Request, - type Response, -} from "playwright"; +import type { Locator, Page, Request, Response } from "playwright"; import sharp from "sharp"; import { label, MASK_COLOR, masksFor, type ResolvedConfig, type Route } from "./config.ts"; +import { engineOf, type Engine } from "./engine.ts"; /** Loads a page, settles it, and screenshots it. */ @@ -369,13 +363,13 @@ export async function capture( cfg: ResolvedConfig, now: number, ): Promise { + const engine = engineOf(cfg); step.set(page, "load"); await page.setViewportSize({ width, height: 900 }); const maxHeight = Math.max(40_000, Math.floor(MAX_PAGE_AREA / width)); await page.clock.setSystemTime(now); // Here, not on framenavigated, which a pushState during hydration fires too. network.get(page)!.failed.clear(); - stopped.delete(page); doc.delete(page); const res = await page .goto(url, { waitUntil: "domcontentloaded", timeout: 45_000 }) @@ -549,15 +543,18 @@ export async function capture( ); } const tiled = area.height > MAX_CAPTURE_HEIGHT; + const target = clip ?? (hidesX ? { x: 0, y: 0, width, height: 1e6 } : undefined); + let stopped = false; // Read on both sides of the shot, so a box that moves while it is taken stays covered. const shoot = async () => { const before = await read(); - const png = await stilled(page, () => - tiled - ? captureTiled(page, area) - : pageShot(page, clip ?? (hidesX ? { x: 0, y: 0, width, height: 1e6 } : undefined)), + const shot = await stilled(page, () => + engine.shot(page, () => + tiled ? captureTiled(engine, page, area) : pageShot(engine, page, target), + ), ); - return paintBoxes(png, [...before, ...(await read())]); + stopped ||= shot.stopped; + return paintBoxes(shot.png, [...before, ...(await read())]); }; // A tiled shot is expensive, so it gets one comparison. @@ -580,7 +577,7 @@ export async function capture( failed, fonts: loadedFonts, marked, - ...(stopped.has(page) ? { stopped: true as const } : {}), + ...(stopped ? { stopped: true as const } : {}), }; prev = png; } @@ -678,7 +675,7 @@ export function bbox(mask: Uint8Array, width: number) { return { x: x0, y: y0, w: x1 - x0 + 1, h: y1 - y0 + 1 }; } -type Box = { x: number; y: number; width: number; height: number }; +export type Box = { x: number; y: number; width: number; height: number }; async function paintBoxes(png: Buffer, boxes: Box[]): Promise { if (!boxes.length) return png; @@ -711,11 +708,11 @@ async function paintBoxes(png: Buffer, boxes: Box[]): Promise { * * `deviceScaleFactor` is 1 (`CONTEXT_OPTIONS` in config.ts), so clip coords are CSS px. */ -async function captureTiled(page: Page, box: Box): Promise { +async function captureTiled(engine: Engine, page: Page, box: Box): Promise { const tiles: Buffer[] = []; for (let y = 0; y < box.height; y += TILE_HEIGHT) { const height = Math.min(TILE_HEIGHT, box.height - y); - tiles.push(await grab(page, { ...box, y: box.y + y, height }, true)); + tiles.push(await engine.grab(page, { ...box, y: box.y + y, height }, true)); } return sharp({ @@ -731,35 +728,6 @@ async function captureTiled(page: Page, box: Box): Promise { .toBuffer(); } -const sessions = new WeakMap>(); - -export const isWebKit = (page: Page) => page.context().browser()?.browserType().name() === "webkit"; -const stopped = new WeakSet(); - -/** A PNG of `clip`, in page px, on the page's one CDP session; WebKit has none, and renders it all offscreen. */ -async function grab(page: Page, clip: Box, beyond: boolean): Promise { - if (isWebKit(page)) - return page.screenshot({ fullPage: true, clip, timeout: 5_000 }).catch(async (e) => { - if (!(e instanceof errors.TimeoutError) || stopped.has(page)) throw e; - // WebKit's fonts.ready, which its screenshot waits on, holds until the load event; a stalled frame never fires it. - await page.evaluate(() => window.stop()); - stopped.add(page); - return page.screenshot({ fullPage: true, clip }); - }); - let s = sessions.get(page); - if (!s) sessions.set(page, (s = page.context().newCDPSession(page))); - const { data } = await ( - await s - ).send("Page.captureScreenshot", { - format: "png", - // Lossless either way, and sharp re-encodes every shot kept. - optimizeForSpeed: true, - captureBeyondViewport: beyond, - clip: { ...clip, scale: 1 }, - }); - return Buffer.from(data, "base64"); -} - type Resumable = Window & { __renderdiffResume?: () => void }; /** Runs in each frame: Playwright 1.62's screenshot animation and caret step, until `__renderdiffResume`. */ @@ -854,7 +822,7 @@ function docSize() { } /** `page.screenshot({ fullPage: true, clip })` as Playwright 1.62 takes it on Chromium. */ -async function pageShot(page: Page, clip?: Box): Promise { +async function pageShot(engine: Engine, page: Page, clip?: Box): Promise { const size = await page.evaluate(docSize); const c = clip ?? { x: 0, y: 0, ...size }; const [x, y] = [Math.max(0, Math.min(c.x, size.width)), Math.max(0, Math.min(c.y, size.height))]; @@ -864,5 +832,5 @@ async function pageShot(page: Page, clip?: Box): Promise { throw new Error("Clipped area is either empty or outside the resulting image"); const view = page.viewportSize()!; const fits = size.width <= view.width && size.height <= view.height; - return grab(page, { x, y, width: w, height: h }, !fits); + return engine.grab(page, { x, y, width: w, height: h }, !fits); } diff --git a/src/cli.ts b/src/cli.ts index 25aea2a..4afc25f 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,7 +12,7 @@ */ import fs from "node:fs"; import path from "node:path"; -import { ACCEPTED_FILE } from "./accept.ts"; +import { engineOf, type Engine } from "./engine.ts"; import type { Results } from "./report.ts"; import { alive, @@ -27,7 +27,6 @@ import { selectorText, version, type CliOverrides, - type Engine, type ResolvedConfig, type Route, type Sides, @@ -124,9 +123,9 @@ function parseArgs(argv: string[]): Args { else if (v === "--config") a.config = value(); else if (v === "--browser") { const b = value(); - if (!ENGINES.includes(b as Engine)) + if (!ENGINES.includes(b as Engine["name"])) throw new Error(`--browser expects ${ENGINES.join(" or ")}, got ${b}`); - a.browser = b as Engine; + a.browser = b as Engine["name"]; } else if (v === "--mask") (a.masks ??= []).push(value()); else if (v === "--only") (a.only ??= []).push(value()); else if (v === "--hide") (a.hideSelectors ??= []).push(value()); @@ -685,8 +684,7 @@ try { recheck: args.recheck, acceptedFile: path.join( configPath ? path.dirname(configPath) : process.cwd(), - // An engine's pixels never match the other's, so neither may prune the other's entries. - cfg.browser ? ACCEPTED_FILE.replace(/\.json$/, `.${cfg.browser}.json`) : ACCEPTED_FILE, + engineOf(cfg).acceptedFile, ), }), ); diff --git a/src/config.ts b/src/config.ts index 96c2907..2e9ce34 100644 --- a/src/config.ts +++ b/src/config.ts @@ -3,6 +3,7 @@ import os from "node:os"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; import type { Locator, Page } from "playwright"; +import { engines, type Engine } from "./engine.ts"; /** * Configuration: built-in defaults, the user-facing `RenderdiffConfig` shape, and the @@ -150,10 +151,9 @@ export function parseRoutes( export const DEFAULT_WIDTHS = [1440, 1152, 896, 704, 375]; /** Directory a run writes its screenshots and `report.html` into. Relative to cwd. */ -export const DEFAULT_OUT_DIR = "renderdiff-out"; +export const DEFAULT_OUT_DIR = engines.chromium.outDir; -export const ENGINES = ["chromium", "webkit"] as const; -export type Engine = (typeof ENGINES)[number]; +export const ENGINES = Object.keys(engines) as Engine["name"][]; /** * Captures run in parallel: half the cores, between 2 and 8. A run is bound by waiting on @@ -330,7 +330,7 @@ export interface RenderdiffConfig { /** Hold every lottie-player on this frame once it loads. Unset: left playing. */ lottieFrame?: number; /** The engine both sides are shot in. Default `chromium`. */ - browser?: Engine; + browser?: Engine["name"]; } /** A fully merged config. Everything the run needs, nothing optional. */ @@ -620,7 +620,7 @@ function validate(c: Record, file: string) { !(Number.isSafeInteger(c.lottieFrame) && (c.lottieFrame as number) >= 0) ) bad("lottieFrame", "a whole number ≥ 0"); - if (c.browser !== undefined && !ENGINES.includes(c.browser as Engine)) + if (c.browser !== undefined && !ENGINES.includes(c.browser as Engine["name"])) bad("browser", `one of ${ENGINES.map((e) => `"${e}"`).join(", ")}`); } @@ -642,7 +642,7 @@ export interface CliOverrides { hideSelectors?: string[]; maskMedia?: boolean; freezeVideo?: boolean; - browser?: Engine; + browser?: Engine["name"]; } /** Merge built-in defaults, the config file, and CLI flags into one resolved config. */ @@ -675,17 +675,14 @@ export function resolveConfig(file: RenderdiffConfig, cli: CliOverrides): Resolv ...(cli.hideSelectors ?? []), ]; const freezeVideo = cli.freezeVideo ?? file.freezeVideo ?? true; - const webkit = (cli.browser ?? file.browser) === "webkit"; + const engine = engines[cli.browser ?? file.browser ?? "chromium"]; return { left: left.replace(/\/+$/, ""), right: right.replace(/\/+$/, ""), widths: cli.widths ?? file.widths ?? [...DEFAULT_WIDTHS], threshold: cli.threshold ?? file.threshold ?? null, - outDir: path.resolve( - process.cwd(), - cli.outDir ?? file.outDir ?? (webkit ? `${DEFAULT_OUT_DIR}-webkit` : DEFAULT_OUT_DIR), - ), + outDir: path.resolve(process.cwd(), cli.outDir ?? file.outDir ?? engine.outDir), concurrency: cli.concurrency ?? file.concurrency ?? DEFAULT_CONCURRENCY, blockUrlPatterns: [...DEFAULT_BLOCK_URL_PATTERNS, ...(file.blockUrlPatterns ?? [])], hideSelectors, @@ -713,7 +710,7 @@ export function resolveConfig(file: RenderdiffConfig, cli: CliOverrides): Resolv freezeVideo, lottieFrame: file.lottieFrame, aria: (cli.aria ?? file.aria) || undefined, - ...(webkit ? { browser: "webkit" as const } : {}), + ...(engine.name === "chromium" ? {} : { browser: engine.name }), }; } diff --git a/src/engine.ts b/src/engine.ts new file mode 100644 index 0000000..cdd5030 --- /dev/null +++ b/src/engine.ts @@ -0,0 +1,148 @@ +import { setTimeout as sleep } from "node:timers/promises"; +import type { Browser, CDPSession, Page } from "playwright"; +import type { Box } from "./capture.ts"; +import type { ResolvedConfig } from "./config.ts"; + +/** What differs between the browsers a run can shoot in, one implementation each. */ +export interface Engine { + name: "chromium" | "webkit"; + outDir: string; + acceptedFile: string; + /** Printed once with `--computed`, for an engine that can't force states. */ + statesNote?: string; + /** `name` is what results.json records: `chrome` when Chromium fell back to it. */ + launch(): Promise<{ browser: Browser; name: string }>; + /** A PNG of `clip`, in page px. */ + grab(page: Page, clip: Box, beyond: boolean): Promise; + /** `take`'s PNG, and whether the page's load had to be stopped to get it. */ + shot(page: Page, take: () => Promise): Promise<{ png: Buffer; stopped: boolean }>; + /** Forces `globalThis.__renderdiffStates` into pseudo-classes; null where that can't be done. */ + states( + page: Page, + ): Promise<{ force(pseudo: readonly string[]): Promise; close(): Promise } | null>; +} + +// Replaces Playwright's own headless --blink-settings, so its hover/pointer values are repeated. +const ARGS = [ + "--blink-settings=imageAnimationPolicy=2,primaryHoverType=2,availableHoverTypes=2,primaryPointerType=4,availablePointerTypes=4", +]; + +const sessions = new WeakMap>(); + +async function stateNodes(client: CDPSession): Promise { + await Promise.all([client.send("DOM.enable"), client.send("CSS.enable")]); + await client.send("DOM.getDocument", { depth: 0 }); + const { result } = await client.send("Runtime.evaluate", { + expression: "globalThis.__renderdiffStates", + }); + const { result: own } = await client.send("Runtime.getProperties", { + objectId: result.objectId!, + ownProperties: true, + }); + return Promise.all( + own + .filter((p) => /^\d+$/.test(p.name)) + .map((p) => + client.send("DOM.requestNode", { objectId: p.value!.objectId! }).then((r) => r.nodeId), + ), + ); +} + +const chromium: Engine = { + name: "chromium", + outDir: "renderdiff-out", + acceptedFile: "renderdiff.accepted.json", + /** + * A missing browser binary is the likeliest first-run failure, so fall back to the system + * Chrome (both sides share it, so the comparison stays symmetric), then name the fix. + */ + async launch() { + const { chromium } = await import("playwright"); + try { + return { browser: await chromium.launch({ args: ARGS }), name: "chromium" }; + } catch (e) { + try { + const browser = await chromium.launch({ channel: "chrome", args: ARGS }); + console.log( + `Bundled Chromium failed to launch; using the system Chrome ${browser.version()}.`, + ); + return { browser, name: "chrome" }; + } catch { + const msg = e instanceof Error ? e.message : String(e); + throw new Error( + `could not launch Chromium, or the system Chrome:\n ${msg}\n\n` + + ` Install the browser with: npx playwright install chromium`, + ); + } + } + }, + async grab(page, clip, beyond) { + let s = sessions.get(page); + if (!s) sessions.set(page, (s = page.context().newCDPSession(page))); + const { data } = await ( + await s + ).send("Page.captureScreenshot", { + format: "png", + // Lossless either way, and sharp re-encodes every shot kept. + optimizeForSpeed: true, + captureBeyondViewport: beyond, + clip: { ...clip, scale: 1 }, + }); + return Buffer.from(data, "base64"); + }, + shot: async (_, take) => ({ png: await take(), stopped: false }), + async states(page) { + const client = await page.context().newCDPSession(page); + let nodeIds: number[] | undefined; + return { + async force(pseudo) { + nodeIds ??= await stateNodes(client); + // A new force replaces the last; detaching clears them all. + await Promise.all( + nodeIds.map((nodeId) => + client.send("CSS.forcePseudoState", { nodeId, forcedPseudoClasses: [...pseudo] }), + ), + ); + }, + async close() { + await client + .send("Runtime.evaluate", { expression: "delete globalThis.__renderdiffStates" }) + .catch(() => {}); + await client.detach().catch(() => {}); + }, + }; + }, +}; + +const webkit: Engine = { + name: "webkit", + outDir: "renderdiff-out-webkit", + // An engine's pixels never match the other's, so neither may prune the other's entries. + acceptedFile: "renderdiff.accepted.webkit.json", + statesNote: + "--computed on webkit: links and controls aren't forced into :hover/:focus-visible (that needs Chromium)", + async launch() { + const { webkit } = await import("playwright"); + try { + return { browser: await webkit.launch(), name: "webkit" }; + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + throw new Error( + `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit (--with-deps on Linux)`, + ); + } + }, + grab: (page, clip) => page.screenshot({ fullPage: true, clip }), + async shot(page, take) { + const shot = take(); + const png = await Promise.race([shot, sleep(5_000, null, { ref: false })]); + if (png) return { png, stopped: false }; + // WebKit's fonts.ready, which its screenshot waits on, holds until the load event; a stalled frame never fires it. + await page.evaluate(() => window.stop()); + return { png: await shot, stopped: true }; + }, + states: async () => null, +}; + +export const engines: Record = { chromium, webkit }; +export const engineOf = (cfg: ResolvedConfig) => engines[cfg.browser ?? "chromium"]; diff --git a/src/locate.ts b/src/locate.ts index c8a82ec..f715bf3 100644 --- a/src/locate.ts +++ b/src/locate.ts @@ -1,7 +1,8 @@ import type { Locator, Page } from "playwright"; -import { isWebKit, type Shot } from "./capture.ts"; +import type { Shot } from "./capture.ts"; import type { Box, Spot } from "./compare.ts"; import type { Snapshot } from "./computed.ts"; +import type { Engine } from "./engine.ts"; /** Names the DOM elements behind each change, from the pages still holding the shot's document. */ @@ -587,6 +588,7 @@ export async function snapshot( * rules relating two of them, and never puts an ancestor in `:hover`. Leaves the page resting. */ export async function snapshotStates( + engine: Engine, page: Page, masks: Locator[], budget: number, @@ -595,42 +597,17 @@ export async function snapshotStates( ): Promise<{ snap: Snapshot; unchecked: number }> { const t0 = Date.now(); const run = await walker(page, masks, budget, root, hide); - // Forcing a state needs CDP, which only Chromium has. - if (isWebKit(page)) return { snap: (await run()).snap!, unchecked: 0 }; - const client = await page.context().newCDPSession(page); + const forcer = await engine.states(page); + if (!forcer) return { snap: (await run()).snap!, unchecked: 0 }; try { const { snap, states, unchecked } = await run({ collect: MAX_STATES }); - await Promise.all([client.send("DOM.enable"), client.send("CSS.enable")]); - await client.send("DOM.getDocument", { depth: 0 }); - const { result } = await client.send("Runtime.evaluate", { - expression: "globalThis.__renderdiffStates", - }); - const { result: own } = await client.send("Runtime.getProperties", { - objectId: result.objectId!, - ownProperties: true, - }); - const nodeIds = await Promise.all( - own - .filter((p) => /^\d+$/.test(p.name)) - .map((p) => - client.send("DOM.requestNode", { objectId: p.value!.objectId! }).then((r) => r.nodeId), - ), - ); for (const [state, forced] of STATES) { - // A new force replaces the last; detaching clears them all. - await Promise.all( - nodeIds.map((nodeId) => - client.send("CSS.forcePseudoState", { nodeId, forcedPseudoClasses: [...forced] }), - ), - ); + await forcer.force(forced); Object.assign(snap!, (await run({ spent: Date.now() - t0, state, states })).snap); } return { snap: snap!, unchecked: unchecked! }; } finally { - await client - .send("Runtime.evaluate", { expression: "delete globalThis.__renderdiffStates" }) - .catch(() => {}); - await client.detach().catch(() => {}); + await forcer.close(); } } diff --git a/src/probe.ts b/src/probe.ts index e5f9e49..d821607 100644 --- a/src/probe.ts +++ b/src/probe.ts @@ -1,9 +1,10 @@ -import type { Page } from "playwright"; +import type { Browser, Page } from "playwright"; import { prepare } from "./capture.ts"; import { collapseInherited, pageError, SHOWN, type Snapshot } from "./computed.ts"; import { CONTEXT_OPTIONS, masksFor, type ResolvedConfig, type Route } from "./config.ts"; +import { engineOf } from "./engine.ts"; import { SNAP_PROPS, snapshot } from "./locate.ts"; -import { BLANK_HINT, COMPUTED_TIMEOUT, launch, shoot } from "./run.ts"; +import { BLANK_HINT, COMPUTED_TIMEOUT, shoot } from "./run.ts"; // ponytail: x/y also list the siblings a grown box pushes down; drop a y a previous sibling's height explains if it gets noisy const FIELDS = [...SNAP_PROPS, "x", "y"]; @@ -16,7 +17,7 @@ interface Change { /** One load in a fresh context, as a run loads a side: every element's styles, then its position in its parent. */ async function load( - browser: Awaited>["browser"], + browser: Browser, url: string, route: Route, width: number, @@ -69,7 +70,7 @@ export async function probe( cfg: ResolvedConfig, loads: number, ): Promise { - const { browser } = await launch(cfg.browser); + const { browser } = await engineOf(cfg).launch(); const now = Math.floor(Date.now() / 60_000) * 60_000; const done: { snap: Snapshot; row: Record }[] = []; let next = 0; diff --git a/src/run.ts b/src/run.ts index 5dbb0a5..8380c46 100644 --- a/src/run.ts +++ b/src/run.ts @@ -3,7 +3,7 @@ import { createHash } from "node:crypto"; import fs from "node:fs"; import path from "node:path"; import { setTimeout as sleep } from "node:timers/promises"; -import { chromium, errors, webkit, type Browser, type BrowserContext, type Page } from "playwright"; +import { errors, type Browser, type BrowserContext, type Page } from "playwright"; import sharp from "sharp"; import { applyAccepted, readAccepted, unacceptable, updateAccepted } from "./accept.ts"; import { CACHE_DIR, cacheFile, replayable, REPLAY_TTL, saveRecording, shotId } from "./cache.ts"; @@ -28,6 +28,7 @@ import { prefixDiff, ruleCoverage, } from "./computed.ts"; +import { engineOf } from "./engine.ts"; import { locate, MAX_SPOTS, severity, snapshotStates, type ElementDiff } from "./locate.ts"; import { alive, @@ -305,7 +306,9 @@ export async function run({ const reused = new Set(results); const stems = slugs(routes); - const launched = jobs.length ? await launch(cfg.browser) : null; + const engine = engineOf(cfg); + const launched = jobs.length ? await engine.launch() : null; + const launchedMeta = launched && { name: launched.name, version: launched.browser.version() }; const queue = [...jobs]; let done = 0; let down = ""; @@ -321,7 +324,7 @@ export async function run({ }; // Whole minute, so an HH:MM display reads the same on both sides. const now = Math.floor(Date.now() / 60_000) * 60_000; - const har = launched && cacheRight ? cacheFile(cfg, browserOf(launched)) : null; + const har = launchedMeta && cacheRight ? cacheFile(cfg, launchedMeta) : null; const replayedAt = har && replayable( @@ -563,6 +566,7 @@ export async function run({ const made = await crops(leftShot.png, rightShot.png, worstFirst(spots)); const snap = (page: Page, side: "left" | "right") => snapshotStates( + engine, page, masksFor(cfg, page, route), COMPUTED_TIMEOUT, @@ -720,10 +724,7 @@ export async function run({ if (progress) { console.log(`Capturing ${jobs.length} pair(s) with ${cfg.concurrency} worker(s)...\n`); } - if (cfg.computed && cfg.browser && jobs.length) - console.log( - `--computed on ${cfg.browser}: links and controls aren't forced into :hover/:focus-visible (that needs Chromium)`, - ); + if (cfg.computed && engine.statesNote && jobs.length) console.log(engine.statesNote); if (launched) { await Promise.all( Array.from({ length: cfg.concurrency }, (_, i) => worker(launched.browser, i)), @@ -734,7 +735,7 @@ export async function run({ console.error(`✗ ${down} stopped responding mid-run. Start it again and rerun.`); return 2; } - const browserMeta = launched ? browserOf(launched) : prev!.meta.browser; + const browserMeta = launchedMeta ?? prev!.meta.browser; let unsaved = ""; if (har && !replayedAt) { try { @@ -1066,50 +1067,6 @@ export async function pack(srcDir: string, outDir: string): Promise { ); } -const browserOf = (l: { browser: Browser; channel: string | null }) => ({ - name: l.channel ?? "chromium", - version: l.browser.version(), -}); - -// Replaces Playwright's own headless --blink-settings, so its hover/pointer values are repeated. -const ARGS = [ - "--blink-settings=imageAnimationPolicy=2,primaryHoverType=2,availableHoverTypes=2,primaryPointerType=4,availablePointerTypes=4", -]; - -/** - * A missing browser binary is the likeliest first-run failure, so fall back to the system - * Chrome (both sides share it, so the comparison stays symmetric), then name the fix. - */ -export async function launch(browser?: "webkit") { - if (browser === "webkit") { - try { - return { browser: await webkit.launch(), channel: "webkit" }; - } catch (e) { - const msg = e instanceof Error ? e.message : String(e); - throw new Error( - `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit (--with-deps on Linux)`, - ); - } - } - try { - return { browser: await chromium.launch({ args: ARGS }), channel: null }; - } catch (e) { - try { - const browser = await chromium.launch({ channel: "chrome", args: ARGS }); - console.log( - `Bundled Chromium failed to launch; using the system Chrome ${browser.version()}.`, - ); - return { browser, channel: "chrome" }; - } catch { - const msg = e instanceof Error ? e.message : String(e); - throw new Error( - `could not launch Chromium, or the system Chrome:\n ${msg}\n\n` + - ` Install the browser with: npx playwright install chromium`, - ); - } - } -} - const transient = (e: unknown) => !(e instanceof HookError) && (e instanceof errors.TimeoutError || diff --git a/tests/visual/computed.test.ts b/tests/visual/computed.test.ts index 4c5a23c..ec3e069 100644 --- a/tests/visual/computed.test.ts +++ b/tests/visual/computed.test.ts @@ -7,6 +7,7 @@ import assert from "node:assert/strict"; import { after, before, test } from "node:test"; import { chromium, type Browser, type Page } from "playwright"; import { diffSnapshots } from "../../src/computed.ts"; +import { engines } from "../../src/engine.ts"; import { locate, PROPS, snapshot, snapshotStates } from "../../src/locate.ts"; import { readText, type Shot } from "../../src/capture.ts"; import type { Region } from "../../src/compare.ts"; @@ -137,7 +138,9 @@ test("pixel mode's selectors keep the classes in source order", async () => { async function states(body: string, css: [string, string]) { const [l, r] = await Promise.all([page(body, css[0]), page(body, css[1])]); - const s = await Promise.all([l, r].map((p) => snapshotStates(p, [], BUDGET, null))); + const s = await Promise.all( + [l, r].map((p) => snapshotStates(engines.chromium, p, [], BUDGET, null)), + ); await Promise.all([l.close(), r.close()]); return diffSnapshots(s[0]!.snap, s[1]!.snap); } @@ -183,7 +186,7 @@ test("past 500 links the rest are counted, and the page is left resting", async "a:hover { color: red }", ); const before = await snapshot(p, [], BUDGET, null); - const { snap, unchecked } = await snapshotStates(p, [], BUDGET, null); + const { snap, unchecked } = await snapshotStates(engines.chromium, p, [], BUDGET, null); assert.equal(unchecked, 1); const color = PROPS.indexOf("color"); assert.equal(snap[`body > a[href="#x"]:hover`]?.styles[color], "rgb(255, 0, 0)"); From 7fb74b372f4c62c6b9f90746f063c0d2440d1c6e Mon Sep 17 00:00:00 2001 From: Timur Mashara <62305878+tmashara@users.noreply.github.com> Date: Thu, 5 Nov 2026 17:30:24 +0100 Subject: [PATCH 4/4] webkit: stop only an open load, and report a stopped shot like a failed asset The 5s now spans a whole tiled shot, so a slow finished page was flagged stopped. window.stop() now runs only while the load is open, and that is the flag. A hung or rejected evaluate no longer costs a finished shot. A stopped shot read as clean: broken() ignored it and the note printed only with --progress. It now counts as broken, so it is never clean or reused, shows under the row and in report.html, and evidence.html says Not verified. Like a failed asset it leaves the exit code alone, and a stopped right side isn't recorded for --cache-right. ENGINES goes, for Object.keys(engines), and engine.ts loses comments. Co-Authored-By: Claude Opus 5.5 --- docs/how-it-works.md | 9 +++++---- src/capture.ts | 4 ++-- src/cli.ts | 12 ++++-------- src/config.ts | 13 ++++++++----- src/engine.ts | 27 +++++++++++++-------------- src/report.ts | 3 +++ src/run.ts | 19 ++++++++++--------- tests/unit/report.test.ts | 3 +++ tests/visual/webkit.test.ts | 22 ++++++++++++++++++++-- 9 files changed, 68 insertions(+), 44 deletions(-) diff --git a/docs/how-it-works.md b/docs/how-it-works.md index a49393f..447057f 100644 --- a/docs/how-it-works.md +++ b/docs/how-it-works.md @@ -30,10 +30,11 @@ honestly. This is what renderdiff does to make sure of that. it flips. - **On WebKit, stops a load that never ends**, as a last resort. WebKit's `fonts.ready`, which its screenshot waits on, holds until the load event, so a frame that never loads - would hang the shot. When a shot takes over 5s, renderdiff calls `window.stop()`, which - lets it finish. That fails the page's open requests (a pending `fetch` rejects, images - break), which can show as a difference, so the shot records it: `stopped loading on right` - in the row, `stopped` in results.json. + would hang the shot. When a shot takes over 5s and the load event hasn't fired, renderdiff + calls `window.stop()`, which lets it finish. That fails the page's open requests (a pending + `fetch` rejects, images break), so like a failed asset the shot is never clean or reused + and doesn't change the exit code: `stopped loading on right` under the row and in + report.html, `stopped` in results.json, and evidence.html says Not verified. - **Settles the page** — makes native lazy images eager, scrolls every viewport-height slice a frame at a time, to the bottom even as the page grows, so script lazy loaders fire, waits for every image to decode, `document.fonts.ready` (at most 5s) and up to 3s for the network to go 250ms with nothing in flight diff --git a/src/capture.ts b/src/capture.ts index 5b8fae1..f72d105 100644 --- a/src/capture.ts +++ b/src/capture.ts @@ -28,7 +28,7 @@ export interface Shot { /** Reasons of the page's `data-renderdiff-mask` elements, e.g. `rotating quote (data-renderdiff-mask)`. */ marked: string[]; /** WebKit only: the shot timed out on a load still open (fonts.ready waits on it), so it was stopped. */ - stopped?: true; + stopped?: boolean; } /** @@ -577,7 +577,7 @@ export async function capture( failed, fonts: loadedFonts, marked, - ...(stopped ? { stopped: true as const } : {}), + stopped, }; prev = png; } diff --git a/src/cli.ts b/src/cli.ts index 4afc25f..c4580c0 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -12,11 +12,10 @@ */ import fs from "node:fs"; import path from "node:path"; -import { engineOf, type Engine } from "./engine.ts"; +import { engineOf, engines, type Engine } from "./engine.ts"; import type { Results } from "./report.ts"; import { alive, - ENGINES, isHttpUrl, label, loadConfig, @@ -123,8 +122,8 @@ function parseArgs(argv: string[]): Args { else if (v === "--config") a.config = value(); else if (v === "--browser") { const b = value(); - if (!ENGINES.includes(b as Engine["name"])) - throw new Error(`--browser expects ${ENGINES.join(" or ")}, got ${b}`); + if (!Object.keys(engines).includes(b)) + throw new Error(`--browser expects ${Object.keys(engines).join(" or ")}, got ${b}`); a.browser = b as Engine["name"]; } else if (v === "--mask") (a.masks ??= []).push(value()); else if (v === "--only") (a.only ??= []).push(value()); @@ -682,10 +681,7 @@ try { rerun: args.rerunFailed, cacheRight: args.cacheRight, recheck: args.recheck, - acceptedFile: path.join( - configPath ? path.dirname(configPath) : process.cwd(), - engineOf(cfg).acceptedFile, - ), + acceptedFile: path.resolve(path.dirname(configPath ?? "."), engineOf(cfg).acceptedFile), }), ); } catch (e) { diff --git a/src/config.ts b/src/config.ts index 2e9ce34..a52b3ab 100644 --- a/src/config.ts +++ b/src/config.ts @@ -153,8 +153,6 @@ export const DEFAULT_WIDTHS = [1440, 1152, 896, 704, 375]; /** Directory a run writes its screenshots and `report.html` into. Relative to cwd. */ export const DEFAULT_OUT_DIR = engines.chromium.outDir; -export const ENGINES = Object.keys(engines) as Engine["name"][]; - /** * Captures run in parallel: half the cores, between 2 and 8. A run is bound by waiting on * pages, not CPU, and each worker holds two. No extra rate limiting needed: each worker @@ -355,7 +353,7 @@ export interface ResolvedConfig { lottieFrame?: number; /** Optional like `lottieFrame`. */ aria?: true; - /** Optional like `lottieFrame`: unset is Chromium. */ + /** Unset is Chromium; absent, not `chromium`, so a Chromium run's config hash stays the same. */ browser?: "webkit"; } @@ -620,8 +618,13 @@ function validate(c: Record, file: string) { !(Number.isSafeInteger(c.lottieFrame) && (c.lottieFrame as number) >= 0) ) bad("lottieFrame", "a whole number ≥ 0"); - if (c.browser !== undefined && !ENGINES.includes(c.browser as Engine["name"])) - bad("browser", `one of ${ENGINES.map((e) => `"${e}"`).join(", ")}`); + if (c.browser !== undefined && !Object.keys(engines).includes(c.browser as string)) + bad( + "browser", + `one of ${Object.keys(engines) + .map((e) => `"${e}"`) + .join(", ")}`, + ); } /** CLI-supplied values. `undefined` means "not passed", so the config file wins. */ diff --git a/src/engine.ts b/src/engine.ts index cdd5030..b8e0172 100644 --- a/src/engine.ts +++ b/src/engine.ts @@ -3,18 +3,14 @@ import type { Browser, CDPSession, Page } from "playwright"; import type { Box } from "./capture.ts"; import type { ResolvedConfig } from "./config.ts"; -/** What differs between the browsers a run can shoot in, one implementation each. */ export interface Engine { name: "chromium" | "webkit"; outDir: string; acceptedFile: string; - /** Printed once with `--computed`, for an engine that can't force states. */ statesNote?: string; /** `name` is what results.json records: `chrome` when Chromium fell back to it. */ launch(): Promise<{ browser: Browser; name: string }>; - /** A PNG of `clip`, in page px. */ grab(page: Page, clip: Box, beyond: boolean): Promise; - /** `take`'s PNG, and whether the page's load had to be stopped to get it. */ shot(page: Page, take: () => Promise): Promise<{ png: Buffer; stopped: boolean }>; /** Forces `globalThis.__renderdiffStates` into pseudo-classes; null where that can't be done. */ states( @@ -52,10 +48,7 @@ const chromium: Engine = { name: "chromium", outDir: "renderdiff-out", acceptedFile: "renderdiff.accepted.json", - /** - * A missing browser binary is the likeliest first-run failure, so fall back to the system - * Chrome (both sides share it, so the comparison stays symmetric), then name the fix. - */ + // Both sides share the fallback, so the comparison stays symmetric. async launch() { const { chromium } = await import("playwright"); try { @@ -68,9 +61,8 @@ const chromium: Engine = { ); return { browser, name: "chrome" }; } catch { - const msg = e instanceof Error ? e.message : String(e); throw new Error( - `could not launch Chromium, or the system Chrome:\n ${msg}\n\n` + + `could not launch Chromium, or the system Chrome:\n ${e instanceof Error ? e.message : e}\n\n` + ` Install the browser with: npx playwright install chromium`, ); } @@ -126,9 +118,8 @@ const webkit: Engine = { try { return { browser: await webkit.launch(), name: "webkit" }; } catch (e) { - const msg = e instanceof Error ? e.message : String(e); throw new Error( - `could not launch WebKit:\n ${msg}\n\n Install the browser with: npx playwright install webkit (--with-deps on Linux)`, + `could not launch WebKit:\n ${e instanceof Error ? e.message : e}\n\n Install the browser with: npx playwright install webkit (--with-deps on Linux)`, ); } }, @@ -138,8 +129,16 @@ const webkit: Engine = { const png = await Promise.race([shot, sleep(5_000, null, { ref: false })]); if (png) return { png, stopped: false }; // WebKit's fonts.ready, which its screenshot waits on, holds until the load event; a stalled frame never fires it. - await page.evaluate(() => window.stop()); - return { png: await shot, stopped: true }; + const stop = page + .evaluate(() => { + const open = document.readyState !== "complete"; + if (open) window.stop(); + return open; + }) + .catch(() => false); + const done = await shot; + // A hung evaluate must not cost a finished shot, and it stopped nothing we can vouch for. + return { png: done, stopped: await Promise.race([stop, false]) }; }, states: async () => null, }; diff --git a/src/report.ts b/src/report.ts index 88a4e1b..82ba363 100644 --- a/src/report.ts +++ b/src/report.ts @@ -438,6 +438,8 @@ function findings(r: ComparedResult, crop = cropLink): string { const items: string[] = []; const failed = fmtFailed(r.failed); if (failed) items.push(`

failed to load: ${esc(failed)}

`); + if (r.stopped) + items.push(`

stopped loading on ${esc(r.stopped.join(" and "))}

`); if (r.fonts) items.push(`

fonts ${esc(fmtFonts(r.fonts))}

`); if (r.regions.length) { items.push( @@ -742,6 +744,7 @@ export async function writeEvidence( ...count(open.filter((r) => r.unstablePixels).length, "shot(s) with unstable pixels"), ...count(open.filter((r) => r.varies).length, "shot(s) vary per load (painted out)"), ...count(open.filter((r) => fmtFailed(r.failed)).length, "shot(s) with failed assets"), + ...count(open.filter((r) => r.stopped).length, "shot(s) with a stopped load"), ...count( open.filter((r) => r.diffPixels > 0 && !fails(r, meta.threshold)).length, "shot(s) differ under threshold", diff --git a/src/run.ts b/src/run.ts index 8380c46..8f16795 100644 --- a/src/run.ts +++ b/src/run.ts @@ -156,8 +156,9 @@ function wipeable(dir: string): boolean { return fs.statSync(dir).isDirectory() && fs.readdirSync(dir).every(ours); } -/** A stylesheet or font face failed on a side; older results.json files lack the field. */ -const broken = (r: ComparedResult) => !!(r.failed?.left.length || r.failed?.right.length); +/** A stylesheet or font face failed on a side, or its load was stopped; older results.json files lack `failed`. */ +const broken = (r: ComparedResult) => + !!(r.failed?.left.length || r.failed?.right.length || r.stopped); export const reusable = (r: RouteResult) => r.status === "compared" && @@ -636,11 +637,9 @@ export async function run({ ...made.map((c, i) => write(cropped[i]!.path, c.png)), ]); - if (!rightShot.failed.length) recorded.push(shotId(route, width)); + if (!rightShot.failed.length && !rightShot.stopped) recorded.push(shotId(route, width)); const marked = [...new Set([...leftShot.marked, ...rightShot.marked])]; - const stopped = (["left", "right"] as const).filter( - (s) => ({ left: leftShot, right: rightShot })[s].stopped, - ); + const stopped = (["left", "right"] as const).filter((s) => shots[s].stopped); const fonts = { added: leftShot.fonts.filter((f) => !rightShot.fonts.includes(f)), removed: rightShot.fonts.filter((f) => !leftShot.fonts.includes(f)), @@ -724,8 +723,8 @@ export async function run({ if (progress) { console.log(`Capturing ${jobs.length} pair(s) with ${cfg.concurrency} worker(s)...\n`); } - if (cfg.computed && engine.statesNote && jobs.length) console.log(engine.statesNote); if (launched) { + if (cfg.computed && engine.statesNote) console.log(engine.statesNote); await Promise.all( Array.from({ length: cfg.concurrency }, (_, i) => worker(launched.browser, i)), ); @@ -1280,8 +1279,8 @@ export function compact( for (const route of routes) { const rows = results.filter((r) => name(r) === route); const ok = rows.filter((r): r is ComparedResult => r.status === "compared"); - // Unstable pixels went unmeasured and a failed asset rendered a fallback, so a shot with - // either is not clean, only unchanged. + // Unstable pixels went unmeasured, and a failed asset or stopped load rendered a fallback, + // so a shot with any is not clean, only unchanged. const same = ok.filter((r) => r.diffPixels === 0 && r.unstablePixels === 0 && !broken(r)); const acc = ok.filter((r) => r.accepted); const changed = ok.filter((r) => !same.includes(r) && !r.accepted); @@ -1322,6 +1321,8 @@ export function compact( right: [...new Set(g.flatMap((r) => r.failed?.right ?? []))], }); if (failed) out.push(` failed ${failed}`); + const stopped = [...new Set(g.flatMap((r) => r.stopped ?? []))].sort(); + if (stopped.length) out.push(` stopped loading on ${stopped.join(" and ")}`); const fonts = fmtFonts({ added: [...new Set(g.flatMap((r) => r.fonts?.added ?? []))], removed: [...new Set(g.flatMap((r) => r.fonts?.removed ?? []))], diff --git a/tests/unit/report.test.ts b/tests/unit/report.test.ts index 6bb9a4b..5c3cf10 100644 --- a/tests/unit/report.test.ts +++ b/tests/unit/report.test.ts @@ -128,6 +128,9 @@ describe("writeEvidence", () => { const under = await evidence([differs("/a")], { threshold: 0.5 }); assert.equal(banner(under), "Not verified"); assert.match(under, /1 shot\(s\) differ under threshold/); + const stopped = await evidence([shot("/", { stopped: ["right"] })]); + assert.equal(banner(stopped), "Not verified"); + assert.match(stopped, /1 shot\(s\) with a stopped load/); const rerun = await evidence([shot("/")], { rerun: { rerun: 1, reused: 4 } }); assert.equal(banner(rerun), "Not verified"); const full = await evidence([shot("/")], { rerun: { rerun: 5, reused: 0 } }); diff --git a/tests/visual/webkit.test.ts b/tests/visual/webkit.test.ts index bdc54f4..4faec4a 100644 --- a/tests/visual/webkit.test.ts +++ b/tests/visual/webkit.test.ts @@ -7,7 +7,10 @@ import assert from "node:assert/strict"; import fs from "node:fs"; import path from "node:path"; import { after, before, test } from "node:test"; +import { setTimeout as sleep } from "node:timers/promises"; +import type { Page } from "playwright"; import sharp from "sharp"; +import { engines } from "../../src/engine.ts"; import { cleanupTempDirs, context, @@ -67,11 +70,13 @@ test("a page past the one-shot height is tiled and stitched to its full height", test("a frame that never loads is stopped after the shot times out, and the stop recorded", async () => { const t0 = Date.now(); - const r = await run(["/stalled-frame"], ["--progress"]); + const r = await run(["/stalled-frame"]); assert.equal(r.code, 0, context(r)); assert.ok(Date.now() - t0 < 45_000, `took ${Date.now() - t0}ms${context(r)}`); assert.deepEqual(result(results(r.outDir), "/stalled-frame", 1440).stopped, ["left", "right"]); - assert.match(r.stdout, /stopped loading on left and right/, context(r)); + assert.match(r.stdout, /^\s+stopped loading on left and right$/m, context(r)); + assert.doesNotMatch(r.stdout, /clean/, context(r)); + assert.match(r.report()!, /stopped loading on left and right/, context(r)); }); test("a page that finishes loading is never stopped", async () => { @@ -79,6 +84,19 @@ test("a page that finishes loading is never stopped", async () => { assert.equal(result(results(r.outDir), "/", 1440).stopped, undefined, context(r)); }); +test("a shot past 5s stops only an open load, and keeps its PNG if the stop is lost", async () => { + const { browser } = await engines.webkit.launch(); + const page = await browser.newPage(); + await page.goto(site.left + "/"); + const png = Buffer.from("png"); + const slow = () => sleep(5_200).then(() => png); + const stuck = { evaluate: () => new Promise(() => {}) } as unknown as Page; + const rejects = { evaluate: () => Promise.reject(new Error("gone")) } as unknown as Page; + const shots = [page, stuck, rejects].map((p) => engines.webkit.shot(p, slow)); + assert.deepEqual(await Promise.all(shots), Array(3).fill({ png, stopped: false })); + await browser.close(); +}); + test("an animated GIF isn't frozen: its pixels are unstable, not a difference", async () => { const r = await run(["/animated"]); assert.equal(r.code, 0, context(r));