From 9badaf2c897a74e32dc54ee0897a12c844a39bab Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sun, 7 Jun 2026 08:07:52 -0700 Subject: [PATCH 1/2] fix: wait for SPA rendering before running axe scan The scanner ran axe immediately after page.goto, so single-page apps that render after load were scanned against a near-empty DOM and reported no violations. Wait for networkidle (30s cap) before scanning; if a page never reaches idle, warn and proceed so long-polling/websocket sites still scan. Closes #201 Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/actions/find/src/findForUrl.ts | 5 ++ .github/actions/find/tests/findForUrl.test.ts | 85 +++++++++++++++---- 2 files changed, 75 insertions(+), 15 deletions(-) diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index d9f1ea87..fc13b771 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -27,6 +27,11 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) + try { + await page.waitForLoadState('networkidle', {timeout: 30000}) + } catch (e) { + core.warning(`Unable to wait for ${url} to reach network idle before scanning: ${e}`) + } const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index 85299c5c..70861546 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -7,21 +7,43 @@ import * as pluginManager from '../src/pluginManager/index.js' import type {Plugin} from '../src/pluginManager/types.js' import {clearCache} from '../src/scansContextProvider.js' +const playwrightMocks = vi.hoisted(() => { + const pageGoto = vi.fn() + const pageWaitForLoadState = vi.fn() + const pageUrl = vi.fn() + const contextClose = vi.fn() + const browserClose = vi.fn() + const contextNewPage = vi.fn(() => ({ + goto: pageGoto, + waitForLoadState: pageWaitForLoadState, + url: pageUrl, + })) + const browserNewContext = vi.fn(() => ({ + newPage: contextNewPage, + close: contextClose, + })) + const browserLaunch = vi.fn(() => ({ + newContext: browserNewContext, + close: browserClose, + })) + + return { + browserLaunch, + browserNewContext, + contextNewPage, + pageGoto, + pageWaitForLoadState, + pageUrl, + contextClose, + browserClose, + } +}) + vi.mock('@actions/core', {spy: true}) vi.mock('playwright', () => ({ default: { chromium: { - launch: () => ({ - newContext: () => ({ - newPage: () => ({ - pageUrl: '', - goto: () => {}, - url: () => {}, - }), - close: () => {}, - }), - close: () => {}, - }), + launch: playwrightMocks.browserLaunch, }, }, })) @@ -39,6 +61,9 @@ let loadedPlugins: Plugin[] = [] function clearAll() { clearCache() vi.clearAllMocks() + playwrightMocks.pageGoto.mockResolvedValue(undefined) + playwrightMocks.pageWaitForLoadState.mockResolvedValue(undefined) + playwrightMocks.pageUrl.mockReturnValue('test.com') } describe('findForUrl', () => { @@ -49,12 +74,42 @@ describe('findForUrl', () => { async function axeOnlyTest() { clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(0) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(0) } + describe('page load handling', () => { + it('waits for network idle after navigation before scanning', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com'}) + + expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') + expect(playwrightMocks.pageWaitForLoadState).toHaveBeenCalledWith('networkidle', {timeout: 30000}) + expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( + playwrightMocks.pageWaitForLoadState.mock.invocationCallOrder[0], + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + }) + + it('logs a warning and proceeds with scanning when network idle times out', async () => { + const timeoutError = new Error('Timeout 30000ms exceeded') + actionInput = '' + clearAll() + playwrightMocks.pageWaitForLoadState.mockRejectedValueOnce(timeoutError) + + await findForUrl({url: 'test.com'}) + + expect(core.warning).toHaveBeenCalledWith( + `Unable to wait for test.com to reach network idle before scanning: ${timeoutError}`, + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) + }) + }) + describe('when no scans list is provided', () => { it('defaults to running only axe scan', async () => { actionInput = '' @@ -80,7 +135,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['axe', 'custom-scan-1']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(1) @@ -97,7 +152,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['custom-scan-1', 'custom-scan-2']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) expect(pluginManager.loadPlugins).toHaveBeenCalledTimes(1) expect(pluginManager.invokePlugin).toHaveBeenCalledTimes(2) @@ -112,7 +167,7 @@ describe('findForUrl', () => { actionInput = JSON.stringify(['custom-scan-1']) clearAll() - await findForUrl('test.com') + await findForUrl({url: 'test.com'}) expect(loadedPlugins[0].default).toHaveBeenCalledTimes(1) expect(loadedPlugins[1].default).toHaveBeenCalledTimes(0) }) From 450fddb738c1c99350a1c0ff7748460802318406 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Sat, 4 Jul 2026 09:21:28 -0700 Subject: [PATCH 2/2] fix: use web-assertion readiness instead of networkidle before axe scan Per review: Playwright discourages networkidle and recommends web assertions for readiness. Replaces the networkidle wait with a locator assertion on rendered visible content under body, with a warn-and-proceed fallback so minimal static pages still scan. The timeout is exposed as a configurable action input. --- .github/actions/find/README.md | 4 ++ .github/actions/find/action.yml | 4 ++ .github/actions/find/src/findForUrl.ts | 17 ++++-- .github/actions/find/src/index.ts | 22 +++++++- .github/actions/find/tests/findForUrl.test.ts | 52 ++++++++++++++++--- README.md | 2 + action.yml | 5 ++ 7 files changed, 93 insertions(+), 13 deletions(-) diff --git a/.github/actions/find/README.md b/.github/actions/find/README.md index b8bca81c..ab147f64 100644 --- a/.github/actions/find/README.md +++ b/.github/actions/find/README.md @@ -31,6 +31,10 @@ configuration option. [`colorScheme`](https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme) configuration option. +#### `rendered_content_timeout` + +**Optional** Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000`. + #### `include_screenshots` **Optional** Bool - whether to capture screenshots of scanned pages and include links to them in the issue diff --git a/.github/actions/find/action.yml b/.github/actions/find/action.yml index fb53d901..6bbdb156 100644 --- a/.github/actions/find/action.yml +++ b/.github/actions/find/action.yml @@ -25,6 +25,10 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false + rendered_content_timeout: + description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' + required: false + default: '30000' outputs: findings_file: diff --git a/.github/actions/find/src/findForUrl.ts b/.github/actions/find/src/findForUrl.ts index fc13b771..68cd0b30 100644 --- a/.github/actions/find/src/findForUrl.ts +++ b/.github/actions/find/src/findForUrl.ts @@ -7,12 +7,15 @@ import {loadPlugins, invokePlugin} from './pluginManager/index.js' import {getScansContext} from './scansContextProvider.js' import * as core from '@actions/core' +const DEFAULT_RENDERED_CONTENT_TIMEOUT = 30000 + export async function findForUrl( urlConfig: UrlConfig, authContext?: AuthContext, includeScreenshots: boolean = false, reducedMotion?: ReducedMotionPreference, colorScheme?: ColorSchemePreference, + renderedContentTimeout: number = DEFAULT_RENDERED_CONTENT_TIMEOUT, ): Promise { const {url, excludeSelectors} = urlConfig const browser = await playwright.chromium.launch({ @@ -27,11 +30,7 @@ export async function findForUrl( const context = await browser.newContext(contextOptions) const page = await context.newPage() await page.goto(url) - try { - await page.waitForLoadState('networkidle', {timeout: 30000}) - } catch (e) { - core.warning(`Unable to wait for ${url} to reach network idle before scanning: ${e}`) - } + await waitForRenderedContent({page, url, timeout: renderedContentTimeout}) const findings: Finding[] = [] const addFinding = async (findingData: Finding) => { @@ -72,6 +71,14 @@ export async function findForUrl( return findings } +async function waitForRenderedContent({page, url, timeout}: {page: playwright.Page; url: string; timeout: number}) { + try { + await page.locator('body *:visible').first().waitFor({state: 'visible', timeout}) + } catch (e) { + core.warning(`Unable to confirm rendered content for ${url} before scanning: ${e}`) + } +} + async function runAxeScan({ page, addFinding, diff --git a/.github/actions/find/src/index.ts b/.github/actions/find/src/index.ts index 675dff6f..bbf979fe 100644 --- a/.github/actions/find/src/index.ts +++ b/.github/actions/find/src/index.ts @@ -11,6 +11,7 @@ export default async function () { const urls = loadUrls({urlConfigs}) const reducedMotion = loadReducedMotion() const colorScheme = loadColorScheme() + const renderedContentTimeout = loadRenderedContentTimeout() const actualUrls = urlConfigs || urls || [] @@ -22,7 +23,14 @@ export default async function () { for (const urlConfig of actualUrls) { const {url} = urlConfig core.info(`Preparing to scan ${url}`) - const findingsForUrl = await findForUrl(urlConfig, authContext, includeScreenshots, reducedMotion, colorScheme) + const findingsForUrl = await findForUrl( + urlConfig, + authContext, + includeScreenshots, + reducedMotion, + colorScheme, + renderedContentTimeout, + ) if (findingsForUrl.length === 0) { core.info(`No accessibility gaps were found on ${url}`) continue @@ -101,3 +109,15 @@ function loadColorScheme() { return colorSchemeInput as ColorSchemePreference } + +function loadRenderedContentTimeout() { + const renderedContentTimeoutInput = core.getInput('rendered_content_timeout', {required: false}) + if (!renderedContentTimeoutInput) return + + const renderedContentTimeout = Number(renderedContentTimeoutInput) + if (!Number.isSafeInteger(renderedContentTimeout) || renderedContentTimeout <= 0) { + throw new Error("Input 'rendered_content_timeout' must be a positive integer number of milliseconds.") + } + + return renderedContentTimeout +} diff --git a/.github/actions/find/tests/findForUrl.test.ts b/.github/actions/find/tests/findForUrl.test.ts index 70861546..b9798c4d 100644 --- a/.github/actions/find/tests/findForUrl.test.ts +++ b/.github/actions/find/tests/findForUrl.test.ts @@ -10,12 +10,20 @@ import {clearCache} from '../src/scansContextProvider.js' const playwrightMocks = vi.hoisted(() => { const pageGoto = vi.fn() const pageWaitForLoadState = vi.fn() + const locatorWaitFor = vi.fn() + const locatorFirst = vi.fn(() => ({ + waitFor: locatorWaitFor, + })) + const pageLocator = vi.fn(() => ({ + first: locatorFirst, + })) const pageUrl = vi.fn() const contextClose = vi.fn() const browserClose = vi.fn() const contextNewPage = vi.fn(() => ({ goto: pageGoto, waitForLoadState: pageWaitForLoadState, + locator: pageLocator, url: pageUrl, })) const browserNewContext = vi.fn(() => ({ @@ -33,6 +41,9 @@ const playwrightMocks = vi.hoisted(() => { contextNewPage, pageGoto, pageWaitForLoadState, + pageLocator, + locatorFirst, + locatorWaitFor, pageUrl, contextClose, browserClose, @@ -63,6 +74,7 @@ function clearAll() { vi.clearAllMocks() playwrightMocks.pageGoto.mockResolvedValue(undefined) playwrightMocks.pageWaitForLoadState.mockResolvedValue(undefined) + playwrightMocks.locatorWaitFor.mockResolvedValue(undefined) playwrightMocks.pageUrl.mockReturnValue('test.com') } @@ -81,33 +93,59 @@ describe('findForUrl', () => { } describe('page load handling', () => { - it('waits for network idle after navigation before scanning', async () => { + it('waits for late-rendered SPA content after navigation before scanning', async () => { actionInput = '' clearAll() + let resolveRenderedContent!: () => void + const renderedContent = new Promise(resolve => { + resolveRenderedContent = resolve + }) + playwrightMocks.locatorWaitFor.mockReturnValueOnce(renderedContent) - await findForUrl({url: 'test.com'}) + const scan = findForUrl({url: 'test.com'}) + await new Promise(resolve => setTimeout(resolve, 0)) expect(playwrightMocks.pageGoto).toHaveBeenCalledWith('test.com') - expect(playwrightMocks.pageWaitForLoadState).toHaveBeenCalledWith('networkidle', {timeout: 30000}) + expect(playwrightMocks.pageWaitForLoadState).not.toHaveBeenCalled() + expect(playwrightMocks.pageLocator).toHaveBeenCalledWith('body *:visible') + expect(playwrightMocks.locatorFirst).toHaveBeenCalledTimes(1) + expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 30000}) expect(playwrightMocks.pageGoto.mock.invocationCallOrder[0]).toBeLessThan( - playwrightMocks.pageWaitForLoadState.mock.invocationCallOrder[0], + playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0], + ) + expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(0) + + resolveRenderedContent() + await scan + + expect(playwrightMocks.locatorWaitFor.mock.invocationCallOrder[0]).toBeLessThan( + playwrightMocks.pageUrl.mock.invocationCallOrder[0], ) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) - it('logs a warning and proceeds with scanning when network idle times out', async () => { + it('logs a warning and proceeds with scanning when minimal static pages do not render visible content', async () => { const timeoutError = new Error('Timeout 30000ms exceeded') actionInput = '' clearAll() - playwrightMocks.pageWaitForLoadState.mockRejectedValueOnce(timeoutError) + playwrightMocks.locatorWaitFor.mockRejectedValueOnce(timeoutError) await findForUrl({url: 'test.com'}) expect(core.warning).toHaveBeenCalledWith( - `Unable to wait for test.com to reach network idle before scanning: ${timeoutError}`, + `Unable to confirm rendered content for test.com before scanning: ${timeoutError}`, ) expect(AxeBuilder.prototype.analyze).toHaveBeenCalledTimes(1) }) + + it('uses the configured rendered content timeout', async () => { + actionInput = '' + clearAll() + + await findForUrl({url: 'test.com'}, undefined, false, undefined, undefined, 1000) + + expect(playwrightMocks.locatorWaitFor).toHaveBeenCalledWith({state: 'visible', timeout: 1000}) + }) }) describe('when no scans list is provided', () => { diff --git a/README.md b/README.md index 88b68c11..43215681 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ jobs: # open_grouped_issues: false # Optional: Set to true to open an issue grouping individual issues per violation # reduced_motion: no-preference # Optional: Playwright reduced motion configuration option # color_scheme: light # Optional: Playwright color scheme configuration option + # rendered_content_timeout: 30000 # Optional: Milliseconds to wait for visible rendered content before scanning # scans: '["axe","reflow-scan"]' # Optional: An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. # url_configs: '[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]' # Optional: Per-URL config with CSS selectors to exclude from the Axe scan. When provided, takes precedence over 'urls'. ``` @@ -130,6 +131,7 @@ Trigger the workflow manually or automatically based on your configuration. The | `open_grouped_issues` | No | Whether to create a tracking issue which groups filed issues together by violation type. Default: `false` | `true` | | `reduced_motion` | No | Playwright `reducedMotion` setting for scan contexts. Allowed values: `reduce`, `no-preference` | `reduce` | | `color_scheme` | No | Playwright `colorScheme` setting for scan contexts. Allowed values: `light`, `dark`, `no-preference` | `dark` | +| `rendered_content_timeout` | No | Milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues. Default: `30000` | `30000` | | `scans` | No | An array of scans (or plugins) to be performed. If not provided, only Axe will be performed. | `'["axe", "reflow-scan", ...other plugins]'` | | `url_configs` | No | A stringified JSON array of URL config objects. Each object must have a `url` field and may have an optional `excludeSelectors` field (array of CSS selectors to exclude from the Axe scan for that URL). When provided, takes precedence over the `urls` input. | `'[{"url":"https://example.com","excludeSelectors":["iframe","#widget"]}]'` | diff --git a/action.yml b/action.yml index b86a45e5..a48c44d6 100644 --- a/action.yml +++ b/action.yml @@ -51,6 +51,10 @@ inputs: color_scheme: description: 'Playwright colorScheme setting: https://playwright.dev/docs/api/class-browser#browser-new-context-option-color-scheme' required: false + rendered_content_timeout: + description: 'Timeout in milliseconds to wait for visible rendered content before scanning. If content is not detected in time, scanning continues.' + required: false + default: '30000' scans: description: 'Stringified JSON array of scans to perform. If not provided, only Axe will be performed' required: false @@ -117,6 +121,7 @@ runs: include_screenshots: ${{ inputs.include_screenshots }} reduced_motion: ${{ inputs.reduced_motion }} color_scheme: ${{ inputs.color_scheme }} + rendered_content_timeout: ${{ inputs.rendered_content_timeout }} scans: ${{ inputs.scans }} - name: File id: file