From 04c0b0a08a44e3ff19437a00a2a99975a0c52d0d Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Sun, 1 Nov 2026 17:27:56 -0800 Subject: [PATCH] sitesmith capture: show the whole page as side-by-side strips (max 9000 px) The capture stopped at 3000 CSS px, so the reviewer never saw the bottom or footer. Now the full page height H is rendered at its normal width (1280). H <= 3000: one image, as before. Otherwise N = min(3, ceil(H/3000)) equal-height strips (each <= 3000 px) are laid out side by side, left to right (strip 1 = page top), with 24 px #808080 gutters, in one JPEG; H > 9000 shows the top 9000 px (cropped). Geometry is the pure stripGeometry() (unit-tested); JPEG quality steps 0.8 -> 0.5 before "too large" (3 MB cap, server limit 8 MB). CaptureResult gains strips/pageHeight; runSelfReview returns them and says when only the top of a long page was reviewed. Real eval pages: 2 strips, ~0.8-1.1 s, 150-270 KB in Chromium and Firefox; e2e checks strip order/alignment/gutters/cropping. Co-Authored-By: Claude Opus 5.5 --- craft/scripts/e2e-capture-fidelity-entry.ts | 4 +- craft/scripts/e2e-capture-fidelity.mjs | 23 +++++ craft/src/panels/sitesmith/selfReview.test.ts | 12 +++ craft/src/panels/sitesmith/selfReview.ts | 17 +++- .../utils/self-review-capture.strips.test.ts | 56 ++++++++++++ craft/src/utils/self-review-capture.ts | 87 ++++++++++++++++--- 6 files changed, 186 insertions(+), 13 deletions(-) create mode 100644 craft/src/utils/self-review-capture.strips.test.ts diff --git a/craft/scripts/e2e-capture-fidelity-entry.ts b/craft/scripts/e2e-capture-fidelity-entry.ts index a9f7c75..ab72791 100644 --- a/craft/scripts/e2e-capture-fidelity-entry.ts +++ b/craft/scripts/e2e-capture-fidelity-entry.ts @@ -5,7 +5,9 @@ import { sanitizeCaptureHtml } from '../src/utils/sanitize-capture-html'; (window as any).sanitizeCaptureHtml = sanitizeCaptureHtml; /** Capture through the real path, then decode the JPEG and return RGB samples at the given points. */ (window as any).captureAndSample = async (html: string, points: Array<[number, number]>) => { + const t0 = performance.now(); const r = await captureHtml(html, { settleMs: 200, injectFontAwesome: false }); + const ms = Math.round(performance.now() - t0); if (!r.ok) return { ok: false, reason: r.reason }; const url = URL.createObjectURL(r.blob); const img = new Image(); @@ -15,5 +17,5 @@ import { sanitizeCaptureHtml } from '../src/utils/sanitize-capture-html'; const ctx = c.getContext('2d')!; ctx.drawImage(img, 0, 0); URL.revokeObjectURL(url); - return { ok: true, w: c.width, h: c.height, samples: points.map(([x, y]) => Array.from(ctx.getImageData(x, y, 1, 1).data.slice(0, 3))) }; + return { ok: true, w: c.width, h: c.height, strips: r.strips, cropped: r.cropped, pageHeight: r.pageHeight, bytes: r.blob.size, ms, samples: points.map(([x, y]) => Array.from(ctx.getImageData(x, y, 1, 1).data.slice(0, 3))) }; }; diff --git a/craft/scripts/e2e-capture-fidelity.mjs b/craft/scripts/e2e-capture-fidelity.mjs index 911b983..6758b28 100644 --- a/craft/scripts/e2e-capture-fidelity.mjs +++ b/craft/scripts/e2e-capture-fidelity.mjs @@ -80,6 +80,21 @@ const SCANS = [ ['placehold.co CSS background shows its label in the fg colour', 1290, 300, 980, [0xee, 0xee, 0xee]], ]; +// Tall pages -> side-by-side strips (the geometry the server-side review prompt describes): bands of 2500 px +// (or 4000 px) each with a 100 px black bar on top, so each strip's colour AND its top edge can be checked. +const band = (h, colors) => `${colors.map((c) => `
`).join('')}`; +const TALL = [ + { label: '7500 px page', html: band(2500, ['#c03030', '#30c030', '#3030c0']), strips: 3, w: 3888, h: 2500, cropped: false, + pts: [[640, 1250, [0xc0, 0x30, 0x30]], [1292, 1250, [0x80, 0x80, 0x80]], [1944, 1250, [0x30, 0xc0, 0x30]], [2596, 1250, [0x80, 0x80, 0x80]], [3248, 1250, [0x30, 0x30, 0xc0]], + [640, 50, [0, 0, 0]], [1944, 50, [0, 0, 0]], [3248, 50, [0, 0, 0]], [1944, 2490, [0x30, 0xc0, 0x30]]] }, + { label: '5000 px page', html: band(2500, ['#c03030', '#30c030']), strips: 2, w: 2584, h: 2500, cropped: false, + pts: [[640, 1250, [0xc0, 0x30, 0x30]], [1292, 1250, [0x80, 0x80, 0x80]], [1944, 1250, [0x30, 0xc0, 0x30]], [1944, 50, [0, 0, 0]]] }, + // 12000 px: top 9000 only, 3 x 3000; strip 2 starts at page y 3000 (inside band 1: red), strip 3 at 6000 (green), bars at 0/4000/8000. + { label: '12000 px page (cropped)', html: band(4000, ['#c03030', '#30c030', '#3030c0']), strips: 3, w: 3888, h: 3000, cropped: true, + pts: [[640, 50, [0, 0, 0]], [1944, 50, [0xc0, 0x30, 0x30]], [1944, 1050, [0, 0, 0]], [3248, 50, [0x30, 0xc0, 0x30]], [3248, 2050, [0, 0, 0]], [3248, 2990, [0x30, 0x30, 0xc0]]] }, + { label: '1500 px page (no strips)', html: band(1500, ['#c03030']), strips: 1, w: 1280, h: 1500, cropped: false, pts: [[640, 750, [0xc0, 0x30, 0x30]]] }, +]; + const srv = http.createServer((q, r) => { r.setHeader('content-type', 'text/html'); r.end('hosthost'); }).listen(0); const port = srv.address().port; let failed = 0; @@ -120,6 +135,14 @@ async function run(name, launcher, exe) { check(name, label, hits >= 3, `(${hits} pixels near rgb(${want.join(',')}))`); } } + for (const t of TALL) { + const r = await page.evaluate(([h, pts]) => window.captureAndSample(h, pts), [t.html, t.pts.map((p) => [p[0], p[1]])]); + check(name, `${t.label}: captured`, r.ok === true, r.ok ? `(${r.bytes} bytes, ${r.ms} ms)` : r.reason); + if (!r.ok) continue; + check(name, `${t.label}: ${t.strips} strip(s), ${t.w}x${t.h}, cropped=${t.cropped}`, r.strips === t.strips && r.w === t.w && r.h === t.h && r.cropped === t.cropped, + `got ${r.strips} strip(s) ${r.w}x${r.h} cropped=${r.cropped} pageHeight=${r.pageHeight}`); + t.pts.forEach(([x, y, want], i) => check(name, `${t.label}: pixel (${x},${y})`, near(r.samples[i], want), `got rgb(${r.samples[i].join(',')}) want rgb(${want.join(',')})`)); + } await browser.close(); } await run('chromium', chromium, process.env.CHROMIUM_PATH); diff --git a/craft/src/panels/sitesmith/selfReview.test.ts b/craft/src/panels/sitesmith/selfReview.test.ts index 2d0cf7f..774041c 100644 --- a/craft/src/panels/sitesmith/selfReview.test.ts +++ b/craft/src/panels/sitesmith/selfReview.test.ts @@ -15,6 +15,18 @@ const deps = (o: Partial = {}): SelfReviewDeps => ({ }); describe('runSelfReview', () => { + test('reports the screenshot geometry; a cropped (very long) page says only its top was reviewed', async () => { + const strips = await runSelfReview(deps({ capture: vi.fn(async () => ({ ok: true as const, blob: blob(), width: 2584, height: 2500, cropped: false, strips: 2, pageHeight: 5000 })) })); + expect(strips.capture).toEqual({ strips: 2, cropped: false, pageHeight: 5000 }); + expect(strips.note).not.toMatch(/top 9000/); + const cropped = await runSelfReview(deps({ capture: vi.fn(async () => ({ ok: true as const, blob: blob(), width: 3888, height: 3000, cropped: true, strips: 3, pageHeight: 14000 })) })); + expect(cropped.status).toBe('fixed'); + expect(cropped.capture).toEqual({ strips: 3, cropped: true, pageHeight: 14000 }); + expect(cropped.note).toMatch(/only the top 9000 px of this 14000 px page/); + const skipped = await runSelfReview(deps({ capture: vi.fn(async () => ({ ok: false as const, reason: 'capture timed out' })) })); + expect(skipped.capture).toBeUndefined(); + }); + test('round trip: capture -> upload -> ONE review call -> apply -> render image deleted', async () => { const d = deps(); const out = await runSelfReview(d); diff --git a/craft/src/panels/sitesmith/selfReview.ts b/craft/src/panels/sitesmith/selfReview.ts index 3dc73ea..16db384 100644 --- a/craft/src/panels/sitesmith/selfReview.ts +++ b/craft/src/panels/sitesmith/selfReview.ts @@ -41,7 +41,12 @@ export interface SelfReviewDeps { * - `skipped`: the review did not run, or failed (the note says why). */ export type SelfReviewStatus = 'fixed' | 'fine' | 'mismatch' | 'not-applied' | 'claimed-no-ops' | 'skipped'; -export interface SelfReviewOutcome { status: SelfReviewStatus; note: string } +export interface SelfReviewOutcome { + status: SelfReviewStatus; + note: string; + /** The screenshot that was reviewed: side-by-side strips (1 = plain page), and whether only the page's top was shown. */ + capture?: { strips: number; cropped: boolean; pageHeight: number }; +} const skip = (why: string): SelfReviewOutcome => ({ status: 'skipped', note: `Self-review skipped: ${why}. Your page was built as normal.` }); @@ -87,6 +92,15 @@ const quote = (m: string | undefined | null): string => { /** One review round, never more. Never throws; any failure is a non-blocking note. */ export async function runSelfReview(d: SelfReviewDeps): Promise { + let shot: SelfReviewOutcome['capture']; + const out = await reviewOnce(d, (c) => { shot = c; }); + if (!shot) return out; + const tail = shot.cropped && out.status !== 'skipped' + ? ` The page is long, so the screenshot showed only the top ${Math.min(shot.pageHeight, 9000)} px of this ${shot.pageHeight} px page; check the rest yourself.` : ''; + return { ...out, note: out.note + tail, capture: shot }; +} + +async function reviewOnce(d: SelfReviewDeps, onShot: (c: NonNullable) => void): Promise { let imageId: string | null = null; try { let html: string; @@ -95,6 +109,7 @@ export async function runSelfReview(d: SelfReviewDeps): Promise 9000: top 9000 px only (cropped). + */ +describe('stripGeometry', () => { + test('short page: one image, as before', () => { + expect(stripGeometry(1800, 1280)).toEqual({ + strips: 1, stripHeight: 1800, shownHeight: 1800, cropped: false, width: 1280, height: 1800, gutter: 24, + rects: [{ sy: 0, sh: 1800, dx: 0 }], + }); + expect(stripGeometry(3000, 1280)).toMatchObject({ strips: 1, width: 1280, height: 3000, cropped: false }); + }); + test('3001..6000: two strips side by side with one 24 px gutter', () => { + expect(stripGeometry(5000, 1280)).toEqual({ + strips: 2, stripHeight: 2500, shownHeight: 5000, cropped: false, width: 2584, height: 2500, gutter: 24, + rects: [{ sy: 0, sh: 2500, dx: 0 }, { sy: 2500, sh: 2500, dx: 1304 }], + }); + expect(stripGeometry(3001, 1280)).toMatchObject({ strips: 2, stripHeight: 1501, rects: [{ sy: 0, sh: 1501 }, { sy: 1501, sh: 1500 }] }); + expect(stripGeometry(6000, 1280)).toMatchObject({ strips: 2, stripHeight: 3000, width: 2584 }); + }); + test('6001..9000: three strips', () => { + expect(stripGeometry(7500, 1280)).toEqual({ + strips: 3, stripHeight: 2500, shownHeight: 7500, cropped: false, width: 3888, height: 2500, gutter: 24, + rects: [{ sy: 0, sh: 2500, dx: 0 }, { sy: 2500, sh: 2500, dx: 1304 }, { sy: 5000, sh: 2500, dx: 2608 }], + }); + expect(stripGeometry(9000, 1280)).toMatchObject({ strips: 3, stripHeight: 3000, cropped: false }); + }); + test('taller than 9000: top 9000 px in three 3000 px strips, cropped', () => { + expect(stripGeometry(14321, 1280)).toEqual({ + strips: 3, stripHeight: 3000, shownHeight: 9000, cropped: true, width: 3888, height: 3000, gutter: 24, + rects: [{ sy: 0, sh: 3000, dx: 0 }, { sy: 3000, sh: 3000, dx: 1304 }, { sy: 6000, sh: 3000, dx: 2608 }], + }); + }); + test('invariants over a range of heights', () => { + for (let h = 1; h <= 20000; h += 37) { + const g = stripGeometry(h, 1280); + expect(g.stripHeight).toBeLessThanOrEqual(3000); + expect(g.strips).toBe(h <= 3000 ? 1 : Math.min(3, Math.ceil(h / 3000))); + expect(g.shownHeight).toBe(Math.min(h, 9000)); + expect(g.cropped).toBe(h > 9000); + expect(g.rects.reduce((s, r) => s + r.sh, 0)).toBe(g.shownHeight); // every shown row exactly once + g.rects.forEach((r, i) => { expect(r.sy).toBe(i * g.stripHeight); expect(r.dx).toBe(i * (1280 + 24)); expect(r.sh).toBeGreaterThan(0); }); + expect(g.width).toBe(g.strips * 1280 + (g.strips - 1) * 24); + } + }); + test('non-integer / degenerate heights', () => { + expect(stripGeometry(0, 1280)).toMatchObject({ strips: 1, height: 1 }); + expect(stripGeometry(Number.NaN, 1280)).toMatchObject({ strips: 1, height: 1 }); + expect(stripGeometry(3000.4, 1280)).toMatchObject({ strips: 1, height: 3000 }); + expect(stripGeometry(3000.6, 1280)).toMatchObject({ strips: 2 }); + }); +}); diff --git a/craft/src/utils/self-review-capture.ts b/craft/src/utils/self-review-capture.ts index 52b5555..a7a1a7c 100644 --- a/craft/src/utils/self-review-capture.ts +++ b/craft/src/utils/self-review-capture.ts @@ -12,7 +12,7 @@ * Never throws: every failure is a `{ ok: false, reason }` so the caller can * skip the review with a non-blocking note. */ -import { toJpeg } from 'html-to-image'; +import { toCanvas } from 'html-to-image'; import { sanitizeCaptureHtml, checkUrl, makePolicy, BLOCKED_IMAGE, FONT_FILE_HOSTS, type Policy } from './sanitize-capture-html'; import { parsePlaceholdUrl } from './capture-placeholder'; @@ -21,8 +21,10 @@ export const FONT_AWESOME_CSS = 'https://cdnjs.cloudflare.com/ajax/libs/font-awe export interface CaptureOptions { /** Render width in CSS px (default 1280). */ width?: number; - /** The page is cropped to this height (default 3000 px). */ + /** Tallest strip in CSS px (default 3000). A taller page is cut into side-by-side strips (see stripGeometry). */ maxHeight?: number; + /** Most strips (default 3): a page taller than maxStrips * maxHeight shows only its top (cropped). */ + maxStrips?: number; /** Whole-capture time limit (default 20 s). */ timeoutMs?: number; /** JPEG quality 0..1 (default 0.8). */ @@ -38,9 +40,65 @@ export interface CaptureOptions { } export type CaptureResult = - | { ok: true; blob: Blob; width: number; height: number; cropped: boolean } + /** width/height: the image; cropped: only the top of the page is shown; strips: side-by-side strips (1 = plain + * page) and pageHeight: full page height -- always set by captureHtml (optional only for older test doubles). */ + | { ok: true; blob: Blob; width: number; height: number; cropped: boolean; strips?: number; pageHeight?: number } | { ok: false; reason: string }; +export const STRIP_GUTTER = 24; +export const STRIP_GUTTER_COLOR = '#808080'; + +export interface StripGeometry { + strips: number; + stripHeight: number; + /** Page rows shown (top of the page). */ + shownHeight: number; + cropped: boolean; + /** Output image size. */ + width: number; + height: number; + gutter: number; + /** Per strip: source rows [sy, sy + sh) of the page, drawn at x = dx, y = 0. */ + rects: Array<{ sy: number; sh: number; dx: number }>; +} + +/** + * Screenshot geometry (the server-side review prompt describes exactly this): a page of height H rendered at + * `width`. H <= maxStrip: one image. Else N = min(maxStrips, ceil(H / maxStrip)) equal-height strips (each + * <= maxStrip), laid out side by side left to right (strip 1 = top of the page, on the left) with `gutter` px + * of #808080 between them. A page taller than maxStrips * maxStrip shows only that top part (cropped). + */ +export function stripGeometry(pageHeight: number, width: number, maxStrip = 3000, maxStrips = 3, gutter = STRIP_GUTTER): StripGeometry { + const H = Math.max(1, Math.round(Number.isFinite(pageHeight) ? pageHeight : 1)); + if (H <= maxStrip) return { strips: 1, stripHeight: H, shownHeight: H, cropped: false, width, height: H, gutter, rects: [{ sy: 0, sh: H, dx: 0 }] }; + const n = Math.min(maxStrips, Math.ceil(H / maxStrip)); + const shown = Math.min(H, n * maxStrip); + const sh = Math.ceil(shown / n); + const rects = Array.from({ length: n }, (_, i) => ({ sy: i * sh, sh: Math.min(sh, shown - i * sh), dx: i * (width + gutter) })); + return { strips: n, stripHeight: sh, shownHeight: shown, cropped: H > shown, width: n * width + (n - 1) * gutter, height: sh, gutter, rects }; +} + +/** Lay the rendered page (one tall canvas) out as the strips of `g`. */ +export function composeStrips(page: HTMLCanvasElement, g: StripGeometry, pageWidth: number, background = '#ffffff'): HTMLCanvasElement { + if (g.strips === 1) return page; + const out = document.createElement('canvas'); + out.width = g.width; out.height = g.height; + const ctx = out.getContext('2d'); + if (!ctx) throw new Error('canvas unavailable'); + ctx.fillStyle = STRIP_GUTTER_COLOR; + ctx.fillRect(0, 0, g.width, g.height); + for (const r of g.rects) { + ctx.fillStyle = background; // a last strip a few px shorter than the others ends in page background + ctx.fillRect(r.dx, 0, pageWidth, g.height); + ctx.drawImage(page, 0, r.sy, pageWidth, r.sh, r.dx, 0, pageWidth, r.sh); + } + return out; +} + +const canvasToJpeg = (c: HTMLCanvasElement, q: number) => new Promise((resolve) => { + try { c.toBlob((b) => resolve(b), 'image/jpeg', q); } catch { resolve(null); } +}); + /** Make `html` safe and complete for capture: FA link if missing, scripts and head code are not executed anyway (sandbox). */ export function prepareCaptureHtml(html: string, injectFontAwesome = true): string { // A cross-origin stylesheet without CORS is unreadable (cssRules throws), so its @font-face @@ -224,6 +282,7 @@ const sleep = (ms: number) => new Promise((r) => setTimeout(r, ms)); export async function captureHtml(html: string, opts: CaptureOptions = {}): Promise { const width = opts.width ?? 1280; const maxHeight = opts.maxHeight ?? 3000; + const maxStrips = opts.maxStrips ?? 3; const timeoutMs = opts.timeoutMs ?? 20000; const maxBytes = opts.maxBytes ?? 3 * 1024 * 1024; if (typeof document === 'undefined' || !document.body) return { ok: false, reason: 'no document' }; @@ -259,23 +318,29 @@ export async function captureHtml(html: string, opts: CaptureOptions = {}): Prom await sleep(opts.settleMs ?? 600); const full = Math.max(doc.documentElement.scrollHeight, doc.body.scrollHeight, 1); - const height = Math.min(full, maxHeight); - frame.style.height = `${height}px`; + const g = stripGeometry(full, width, maxHeight, maxStrips); + frame.style.height = `${g.shownHeight}px`; await sleep(50); const fonts = await buildFontEmbedCss(doc, pol); - const dataUrl = await toJpeg(doc.body, { - quality: opts.quality ?? 0.8, - width, height, canvasWidth: width, canvasHeight: height, pixelRatio: 1, + const page = await toCanvas(doc.body, { + width, height: g.shownHeight, canvasWidth: width, canvasHeight: g.shownHeight, pixelRatio: 1, backgroundColor: '#ffffff', cacheBust: false, fetchRequestInit: { credentials: 'omit', cache: 'no-store', redirect: 'error' }, fontEmbedCSS: fonts.css, style: { margin: '0' }, }); - const blob = await (await fetch(dataUrl)).blob(); - if (blob.size < 2000) return { ok: false, reason: 'capture came out empty' }; + const img = composeStrips(page, g, width); + // A strip sheet has up to 3x the pixels of one page: step the JPEG quality down before giving up on size. + const q0 = opts.quality ?? 0.8; + let blob: Blob | null = null; + for (const q of [q0, ...[0.7, 0.6, 0.5].filter((x) => x < q0)]) { + blob = await canvasToJpeg(img, q); + if (!blob || blob.size <= maxBytes) break; + } + if (!blob || blob.size < 2000) return { ok: false, reason: 'capture came out empty' }; if (blob.size > maxBytes) return { ok: false, reason: 'capture too large' }; - return { ok: true, blob, width, height, cropped: full > maxHeight }; + return { ok: true, blob, width: g.width, height: g.height, cropped: g.cropped, strips: g.strips, pageHeight: Math.round(full) }; })(); const timeout = new Promise((resolve) => {