diff --git a/app/screenshots/ScreenshotHelper.test.mjs b/app/screenshots/ScreenshotHelper.test.mjs index 527f85d..5913c5c 100644 --- a/app/screenshots/ScreenshotHelper.test.mjs +++ b/app/screenshots/ScreenshotHelper.test.mjs @@ -13,7 +13,7 @@ describe('ScreenshotHelper', () => { let page = { setViewport: vi.fn(), goto: vi.fn(), - waitForNetworkIdle: vi.fn(), + waitForSelector: vi.fn(), screenshot: vi.fn().mockResolvedValue(png), close: vi.fn(), }; @@ -54,9 +54,12 @@ describe('ScreenshotHelper', () => { }); expect(page.goto).toHaveBeenCalledWith( new URL('http://app:4321/screenshots/#schedules'), - { waitUntil: 'networkidle0' }, + { waitUntil: 'load' }, + ); + expect(page.waitForSelector).toHaveBeenCalledWith( + '[data-screenshot-ready="true"]', + { timeout: 30_000 }, ); - expect(page.waitForNetworkIdle).toHaveBeenCalledWith({ idleTime: 1000 }); expect(httpServer.close).toHaveBeenCalledOnce(); expect(page.close).toHaveBeenCalledOnce(); expect(browser.close).toHaveBeenCalledOnce(); @@ -90,14 +93,17 @@ describe('ScreenshotHelper', () => { 'Content-Type': 'application/json', }, body: JSON.stringify({ - url: 'https://splatoon3.ink/screenshots/#schedules?time=123®ion=NA', + url: 'https://splatoon3.ink/screenshots/index.html#schedules?time=123®ion=NA', viewport: { width: 600, height: 675, deviceScaleFactor: 2, }, - gotoOptions: { waitUntil: 'networkidle0' }, - waitForTimeout: 1000, + gotoOptions: { waitUntil: 'load' }, + waitForSelector: { + selector: '[data-screenshot-ready="true"]', + timeout: 30_000, + }, screenshotOptions: { type: 'png' }, }), }, diff --git a/app/screenshots/drivers/BrowserlessScreenshotDriver.mjs b/app/screenshots/drivers/BrowserlessScreenshotDriver.mjs index 4e61981..03bab9f 100644 --- a/app/screenshots/drivers/BrowserlessScreenshotDriver.mjs +++ b/app/screenshots/drivers/BrowserlessScreenshotDriver.mjs @@ -1,5 +1,6 @@ import { URL } from 'url'; import puppeteer from 'puppeteer-core'; +import { screenshotReadySelector, screenshotReadyTimeout } from '../../../src/common/screenshot.mjs'; import HttpServer from '../HttpServer.mjs'; export default class BrowserlessScreenshotDriver @@ -42,9 +43,11 @@ export default class BrowserlessScreenshotDriver await this._page.setViewport(viewport); await this._page.goto(url, { - waitUntil: 'networkidle0', + waitUntil: 'load', + }); + await this._page.waitForSelector(screenshotReadySelector, { + timeout: screenshotReadyTimeout, }); - await this._page.waitForNetworkIdle({ idleTime: 1000 }); return await this._page.screenshot(); } diff --git a/app/screenshots/drivers/CloudflareScreenshotDriver.mjs b/app/screenshots/drivers/CloudflareScreenshotDriver.mjs index d60e0ac..feaae3f 100644 --- a/app/screenshots/drivers/CloudflareScreenshotDriver.mjs +++ b/app/screenshots/drivers/CloudflareScreenshotDriver.mjs @@ -1,4 +1,5 @@ import { URL } from 'url'; +import { screenshotReadySelector, screenshotReadyTimeout } from '../../../src/common/screenshot.mjs'; export default class CloudflareScreenshotDriver { @@ -48,8 +49,11 @@ export default class CloudflareScreenshotDriver body: JSON.stringify({ url: url.toString(), viewport, - gotoOptions: { waitUntil: 'networkidle0' }, - waitForTimeout: 1000, + gotoOptions: { waitUntil: 'load' }, + waitForSelector: { + selector: screenshotReadySelector, + timeout: screenshotReadyTimeout, + }, screenshotOptions: { type: 'png' }, }), }); diff --git a/src/common/screenshot.mjs b/src/common/screenshot.mjs new file mode 100644 index 0000000..ef28323 --- /dev/null +++ b/src/common/screenshot.mjs @@ -0,0 +1,24 @@ +export const screenshotReadyAttribute = 'data-screenshot-ready'; +export const screenshotReadySelector = `[${screenshotReadyAttribute}="true"]`; +export const screenshotReadyTimeout = 30_000; + +function nextFrame(requestAnimationFrame) { + return new Promise(resolve => requestAnimationFrame(resolve)); +} + +export async function markScreenshotReady({ + document = globalThis.document, + isCurrent = () => true, + requestAnimationFrame = globalThis.requestAnimationFrame, +} = {}) { + await document.fonts?.ready; + await Promise.allSettled( + [...document.images].map(image => image.decode()), + ); + await nextFrame(requestAnimationFrame); + await nextFrame(requestAnimationFrame); + + if (isCurrent()) { + document.documentElement.setAttribute(screenshotReadyAttribute, 'true'); + } +} diff --git a/src/common/screenshot.test.mjs b/src/common/screenshot.test.mjs new file mode 100644 index 0000000..d28ff4f --- /dev/null +++ b/src/common/screenshot.test.mjs @@ -0,0 +1,66 @@ +import { describe, expect, it, vi } from 'vitest'; +import { markScreenshotReady } from './screenshot.mjs'; + +describe('markScreenshotReady', () => { + it('marks the page ready after fonts, images, and layout settle', async () => { + let resolveFonts; + let resolveImage; + let fontsReady = new Promise(resolve => { resolveFonts = resolve; }); + let imageReady = new Promise(resolve => { resolveImage = resolve; }); + let setAttribute = vi.fn(); + let document = { + documentElement: { setAttribute }, + fonts: { ready: fontsReady }, + images: [{ complete: true, decode: () => imageReady }], + }; + let frames = []; + let requestAnimationFrame = callback => frames.push(callback); + + let ready = markScreenshotReady({ document, requestAnimationFrame }); + await Promise.resolve(); + expect(setAttribute).not.toHaveBeenCalled(); + + resolveFonts(); + await Promise.resolve(); + expect(setAttribute).not.toHaveBeenCalled(); + + resolveImage(); + await Promise.resolve(); + await Promise.resolve(); + expect(frames).toHaveLength(1); + expect(setAttribute).not.toHaveBeenCalled(); + + frames.shift()(); + await Promise.resolve(); + expect(frames).toHaveLength(1); + expect(setAttribute).not.toHaveBeenCalled(); + + frames.shift()(); + await ready; + + expect(setAttribute).toHaveBeenCalledWith('data-screenshot-ready', 'true'); + }); + + it('does not mark a readiness run that became stale while assets settled', async () => { + let resolveImage; + let imageReady = new Promise(resolve => { resolveImage = resolve; }); + let setAttribute = vi.fn(); + let document = { + documentElement: { setAttribute }, + fonts: { ready: Promise.resolve() }, + images: [{ decode: () => imageReady }], + }; + let isCurrent = true; + let ready = markScreenshotReady({ + document, + isCurrent: () => isCurrent, + requestAnimationFrame: callback => callback(), + }); + + isCurrent = false; + resolveImage(); + await ready; + + expect(setAttribute).not.toHaveBeenCalled(); + }); +}); diff --git a/src/layouts/ScreenshotLayout.vue b/src/layouts/ScreenshotLayout.vue index 8f2c67e..186e6d2 100644 --- a/src/layouts/ScreenshotLayout.vue +++ b/src/layouts/ScreenshotLayout.vue @@ -40,8 +40,10 @@