From 3a69ad09063f57ef0103c414715ec74d11a25183 Mon Sep 17 00:00:00 2001 From: rcourtman <8825017+rcourtman@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:09:55 +0100 Subject: [PATCH] Keep Retry focus handoff working in Chromium; repair seven proofs The focus handoff from #2925 tracked Retry focus with focus/blur handlers. Chromium blurs a focused element when it leaves the DOM, and the Show branch removes the button before the error effect runs, so the flag was already false and focus fell to the page. jsdom never blurs on removal, which is why the unit test passed. Read document.activeElement when the Retry branch is disposed instead, and make the refresh test model the removal blur so it fails against the old code. The worker-browser proofs for history response, pointer, inspection, window and the three PBS fixtures still clicked the removed History tab and Refresh button. They now read the charts on Overview, drive reads through the 30-second poll, select series paths rather than every path (bands and dots add paths), and mount fixtures in a shared app runtime (fixtureRuntime.tsx) because drawers read the WebSocket and theme contexts. The PBS refresh proof is what caught the focus regression. --- .../subsystems/frontend-primitives.md | 5 +- .../subsystems/performance-and-scalability.md | 5 +- .../browser-tests/fixtureRuntime.tsx | 25 ++++++ .../browser-tests/history-inspection.cjs | 32 +++---- .../browser-tests/history-pointer.cjs | 88 +++++++++++++------ .../browser-tests/history-response.cjs | 9 +- .../browser-tests/history-window.cjs | 76 +++++++++++----- .../browser-tests/pbs-history-refresh.cjs | 72 ++++++++------- .../browser-tests/pbs-history-refresh.tsx | 4 +- .../pbs-history-source-isolation.cjs | 18 ++-- .../browser-tests/pbs-identity-boundary.cjs | 17 +++- .../browser-tests/pbs-identity-boundary.tsx | 4 +- .../f49d0d6d125bd3bf.json | 42 +++++++++ .../Workloads/GuestDrawerHistory.tsx | 55 +++++++----- .../GuestDrawerHistory.refresh.test.tsx | 32 +++++++ 15 files changed, 344 insertions(+), 140 deletions(-) create mode 100644 frontend-modern/browser-tests/fixtureRuntime.tsx create mode 100644 frontend-modern/browser-verification/f49d0d6d125bd3bf.json diff --git a/docs/release-control/v6/internal/subsystems/frontend-primitives.md b/docs/release-control/v6/internal/subsystems/frontend-primitives.md index c864c521e0..fea003582d 100644 --- a/docs/release-control/v6/internal/subsystems/frontend-primitives.md +++ b/docs/release-control/v6/internal/subsystems/frontend-primitives.md @@ -5484,7 +5484,10 @@ and one full-width "Show " button (`aria-expanded`, hidden from `sm` up) reveals the rest; wider layouts always show every panel. The charts poll every 30 seconds and offer no manual refresh; a "Retry history" button appears only while a read has failed, and when a successful retry removes it -keyboard focus moves to the history region rather than the page. A chart +keyboard focus moves to the history region rather than the page. Whether +Retry held focus is read as its branch is disposed, before the node leaves the +DOM, because Chromium blurs a removed element and a blur-tracked flag would +already be clear (the refresh test models that removal blur). A chart draws alert triggers only from the resource's own alerts (`getOwnAlertsForUnifiedResource`): a Docker host's Needs attention lists its containers' alerts, but a container's memory trigger never lands on the host's diff --git a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md index 1972b22717..8a46d63109 100644 --- a/docs/release-control/v6/internal/subsystems/performance-and-scalability.md +++ b/docs/release-control/v6/internal/subsystems/performance-and-scalability.md @@ -4023,8 +4023,9 @@ from one lookup table (`formatHistoryRangeOptionLabel`). The phone collapse of later chart panels is a CSS class below `sm` on panels that stay mounted: it hides nothing from the shared history read and adds no read or timer. The removed "Refresh history" button took no read away: the 30-second poll is -unchanged and "Retry history" still appears for a failed read. None adds a -read or timer. +unchanged and "Retry history" still appears for a failed read. Its focus +handoff reads `document.activeElement` once, as the Retry branch is disposed. +None adds a read or timer. The default stored-history legend resolves one latest timestamp from that same bounded, normalized panel index, then looks up each configured series at diff --git a/frontend-modern/browser-tests/fixtureRuntime.tsx b/frontend-modern/browser-tests/fixtureRuntime.tsx new file mode 100644 index 0000000000..c12ea6d27f --- /dev/null +++ b/frontend-modern/browser-tests/fixtureRuntime.tsx @@ -0,0 +1,25 @@ +// Shared app runtime for browser fixtures. Drawers read active alerts and the +// theme from the app runtime contexts and some components use router links, so +// a fixture that mounts them without these providers throws before rendering. +// The WebSocket context is an explicit offline one: no live socket, no alerts. +import type { Component } from 'solid-js'; +import { render } from 'solid-js/web'; +import { Route, Router } from '@solidjs/router'; + +import { DarkModeContext, WebSocketContext } from '../src/contexts/appRuntime'; + +export const renderInAppRuntime = (Fixture: Component, root = document.getElementById('root')) => + render( + () => ( + false } as never} + > + document.documentElement.classList.contains('dark')}> + + + + + + ), + root as HTMLElement, + ); diff --git a/frontend-modern/browser-tests/history-inspection.cjs b/frontend-modern/browser-tests/history-inspection.cjs index 339f087a01..d725fdd0ad 100644 --- a/frontend-modern/browser-tests/history-inspection.cjs +++ b/frontend-modern/browser-tests/history-inspection.cjs @@ -57,6 +57,8 @@ const { chromium, firefox, webkit } = require('playwright'); ...(touch ? { isMobile: engine !== 'firefox', hasTouch: true } : {}), }); page.setDefaultTimeout(20_000); + // Time flows normally; runFor advances it to fire the 30-second poll. + await page.clock.install(); const requests = [], errors = []; diagnosticPage = page; @@ -201,16 +203,10 @@ const { chromium, firefox, webkit } = require('playwright'); for (const control of await controls.all()) await collapsed(control); } await screenshot('default-input-controls'); - // Tab reaches a native slider after the existing range/refresh controls. + // Tab reaches a native slider straight after the range control (there is + // no manual refresh while reads succeed). await fixture.getByTestId('guest-history-range-control').focus(); await page.keyboard.press('Tab'); - assert.equal( - await fixture - .getByRole('button', { name: 'Refresh history' }) - .evaluate((el) => document.activeElement === el), - true, - ); - await page.keyboard.press('Tab'); assert.equal(await slider.evaluate((el) => document.activeElement === el), true); const focusStyle = await slider.evaluate((el) => { const style = getComputedStyle(el); @@ -247,7 +243,7 @@ const { chromium, firefox, webkit } = require('playwright'); await slider.press('ArrowRight'); assert.equal(await slider.inputValue(), '1'); await assertValue(/CPU no observation\. Memory 30\.0%\. Disk no observation\./); - assert.match(await utilization.innerText(), /CPU\s*-/); + assert.match(await utilization.innerText(), /CPU\s*no reading/); assert.doesNotMatch(await utilization.innerText(), /53\.0%/); await layout(); await screenshot('selected-sparse-time'); @@ -261,9 +257,9 @@ const { chromium, firefox, webkit } = require('playwright'); 'inspection must not start another API read', ); const selectedBeforePoll = await value(); - // Exercise an actual same-source refresh without moving focus, as a background poll would. + // Exercise an actual same-source background poll without moving focus. action = 'updated'; - await fixture.getByRole('button', { name: 'Refresh history' }).evaluate((el) => el.click()); + await page.clock.runFor(30_000); await page.waitForFunction( () => document.querySelector('input[aria-label="Inspect Utilization history"]')?.max === '5', @@ -296,7 +292,7 @@ const { chromium, firefox, webkit } = require('playwright'); let hybridTouch = false; if (!touch && engine === 'chromium') { - await fixture.getByRole('button', { name: 'Refresh history' }).focus(); + await fixture.getByTestId('guest-history-range-control').focus(); const session = await page.context().newCDPSession(page); const tap = async (x, y) => { await session.send('Input.dispatchTouchEvent', { @@ -333,7 +329,7 @@ const { chromium, firefox, webkit } = require('playwright'); await screenshot('hybrid-touch-inspection'); // Touch focus is browser-owned. Exercise a return to mouse without a // focused slider, then preserve an explicit keyboard selection too. - await fixture.getByRole('button', { name: 'Refresh history' }).focus(); + await fixture.getByTestId('guest-history-range-control').focus(); await page.mouse.move(box.x + (34 / 360) * box.width, box.y + box.height / 2); for (const control of await controls.all()) await collapsed(control); assert.match(await utilization.innerText(), /CPU\s*8\.0%/); @@ -345,7 +341,7 @@ const { chromium, firefox, webkit } = require('playwright'); assert.equal(await slider.evaluate((el) => document.activeElement === el), true); assert.equal(await value(), keyboardSelection); await layout(); - await fixture.getByRole('button', { name: 'Refresh history' }).focus(); + await fixture.getByTestId('guest-history-range-control').focus(); for (const control of await controls.all()) await collapsed(control); await screenshot('hybrid-mouse-resumed'); await session.detach(); @@ -366,7 +362,7 @@ const { chromium, firefox, webkit } = require('playwright'); assert.match(await utilization.innerText(), /CPU\s*66\.0%/); } action = 'failure'; - await fixture.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await fixture .getByRole('status', { name: 'History refresh status' }) .getByText('History refresh failed. Showing previously loaded history.', { exact: true }) @@ -387,9 +383,9 @@ const { chromium, firefox, webkit } = require('playwright'); .getByRole('img') .evaluate((el) => document.getElementById(el.getAttribute('aria-describedby')).textContent); assert.match(description, /1 stored observation time\. .*CPU 11\.0%/); - assert.equal(await utilization.locator('path').count(), 0); + assert.equal(await utilization.locator('path[data-history-series]').count(), 0); action = 'empty'; - await fixture.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await page.waitForFunction(() => { const image = document.querySelector('[data-history-group="utilization"] svg[role="img"]'); return ( @@ -400,7 +396,7 @@ const { chromium, firefox, webkit } = require('playwright'); assert.equal(await fixture.getByRole('slider').count(), 0); // Changing resource cannot carry an inspected former-host point into the new target. action = 'original'; - await fixture.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await slider.waitFor(); await slider.focus(); await slider.press('Home'); diff --git a/frontend-modern/browser-tests/history-pointer.cjs b/frontend-modern/browser-tests/history-pointer.cjs index 4880ba1500..50617b64a4 100644 --- a/frontend-modern/browser-tests/history-pointer.cjs +++ b/frontend-modern/browser-tests/history-pointer.cjs @@ -139,9 +139,12 @@ const { chromium, firefox, webkit } = require('playwright'); assert.match(await text(), expected); assert.match(await text(), absent); const markers = await plot - .locator('circle[r="3"]') + .locator('[data-history-hover-marker]') .evaluateAll((nodes) => - nodes.map((node) => ({ x: node.getAttribute('cx'), y: node.getAttribute('cy') })), + nodes.map((node) => ({ + x: node.getAttribute('data-x'), + y: node.getAttribute('data-y'), + })), ); assert.equal(new Set(markers.map((marker) => marker.x)).size, 1); return markers; @@ -153,39 +156,61 @@ const { chromium, firefox, webkit } = require('playwright'); // Element screenshots scroll a tall phone fixture and end its hover. // Capture the current viewport without disturbing the active pointer. await page.screenshot({ path: path.join(artifacts, `sparse-pointer-${theme}.png`) }); - assert.match(await text(), /CPU\s*- Memory\s*30\.0% Disk\s*-/); - assert.equal(await plot.locator('circle[r="3"]').count(), 1); - assert.equal(await utilization.getByTestId('guest-history-hover-time').innerText(), '22:13'); - const descriptionId = await plot.getAttribute('aria-describedby'); + assert.match(await text(), /CPU\s*no reading Memory\s*30\.0% Disk\s*no reading/); + assert.equal(await plot.locator('[data-history-hover-marker]').count(), 1); + assert.equal( + await utilization.getByTestId('guest-history-hover-time').innerText(), + 'Stored · 22:13', + ); + // The plot names its description first, then any missing-reading note. + const descriptionId = (await plot.getAttribute('aria-describedby')).split(' ')[0]; assert.match( await page.locator(`[id="${descriptionId}"]`).innerText(), /14\/11\/2023, 22:13:50\. CPU no observation\. Memory 30\.0%\. Disk no observation\./, ); - await sparseTime(0, /CPU\s*11\.0%/, /Memory\s*- Disk\s*-/); - await sparseTime(45_000, /Disk\s*0\.0%/, /CPU\s*- Memory\s*-/); + await sparseTime(0, /CPU\s*11\.0%/, /Memory\s*no reading Disk\s*no reading/); + await sparseTime(45_000, /Disk\s*0\.0%/, /CPU\s*no reading Memory\s*no reading/); assert.equal( - await plot.locator('path').count(), - 2, + await plot.locator('path[data-history-series="disk"]').count(), + 0, 'lone disk sample must not become a path', ); await hover(utilization, 120_000); - assert.match(await text(), /CPU\s*43\.0% Memory\s*55\.0% Disk\s*-/); - assert.equal(await plot.locator('circle[r="3"]').count(), 2); - assert.equal(await utilization.getByTestId('guest-history-hover-time').innerText(), '22:15'); + assert.match(await text(), /CPU\s*43\.0% Memory\s*55\.0% Disk\s*no reading/); + assert.equal(await plot.locator('[data-history-hover-marker]').count(), 2); + assert.equal( + await utilization.getByTestId('guest-history-hover-time').innerText(), + 'Stored · 22:15', + ); const network = fixture.locator('[data-history-group="network"]'); await hover(network, 0); - assert.match((await network.innerText()).replace(/\s+/g, ' '), /In\s*0 B\/s Out\s*-/); + assert.match( + (await network.innerText()).replace(/\s+/g, ' '), + /In\s*0 B\/s Out\s*no reading/, + ); await hover(network, 30_000); - assert.match((await network.innerText()).replace(/\s+/g, ' '), /In\s*- Out\s*4\.00 KB\/s/); + assert.match( + (await network.innerText()).replace(/\s+/g, ' '), + /In\s*no reading Out\s*4\.00 KB\/s/, + ); await page.mouse.move(1, 1); - assert.match(await text(), /CPU\s*43\.0% Memory\s*55\.0% Disk\s*0\.0%/); - assert.equal(await fixture.getByTestId('guest-history-hover-time').count(), 0); + // The default readout is one stored time; disk has no point at the latest. + assert.match(await text(), /CPU\s*43\.0% Memory\s*55\.0% Disk\s*no reading/); + // Not inspecting: any readout names the latest stored time, never "Stored ·". + assert.deepEqual( + await fixture + .getByTestId('guest-history-hover-time') + .evaluateAll( + (nodes) => nodes.filter((node) => node.textContent.startsWith('Stored ·')).length, + ), + 0, + ); assert.equal(requests.length, readsBefore, 'pointer inspection must not request data'); // Native keyboard selection remains authoritative even with the pointer on another time. await slider.focus(); await slider.press('Home'); await hover(utilization, 30_000); - assert.match(await text(), /CPU\s*11\.0% Memory\s*- Disk\s*-/); + assert.match(await text(), /CPU\s*11\.0% Memory\s*no reading Disk\s*no reading/); await slider.press('ArrowRight'); assert.match( await slider.getAttribute('aria-valuetext'), @@ -193,7 +218,7 @@ const { chromium, firefox, webkit } = require('playwright'); ); await slider.press('Tab'); await hover(utilization, 45_000); - assert.match(await text(), /CPU\s*- Memory\s*- Disk\s*0\.0%/); + assert.match(await text(), /CPU\s*no reading Memory\s*no reading Disk\s*0\.0%/); if (width === 390) { const box = await slider.boundingBox(); assert.ok(box.height >= 44); @@ -208,11 +233,12 @@ const { chromium, firefox, webkit } = require('playwright'); } await page.mouse.move(1, 1); kind = 'failure'; - await fixture.getByRole('button', { name: 'Refresh history' }).evaluate((el) => el.click()); + // No manual refresh: the next 30-second poll reads the failing body. + await page.waitForTimeout(31_000); await fixture .getByText('History refresh failed. Showing previously loaded history.') .waitFor(); - await sparseTime(30_000, /Memory\s*30\.0%/, /CPU\s*-/); + await sparseTime(30_000, /Memory\s*30\.0%/, /CPU\s*no reading/); await page.screenshot({ path: path.join(artifacts, `retained-sparse-pointer-${theme}.png`), }); @@ -224,7 +250,7 @@ const { chromium, firefox, webkit } = require('playwright'); (el) => el.textContent === '31.0%', ), ); - assert.match(await text(), /CPU\s*- Memory\s*31\.0% Disk\s*-/); + assert.match(await text(), /CPU\s*no reading Memory\s*31\.0% Disk\s*no reading/); kind = 'hold'; await page .getByRole('button', { name: 'Switch to service target' }) @@ -233,15 +259,25 @@ const { chromium, firefox, webkit } = require('playwright'); () => [...document.querySelectorAll('[data-history-group="utilization"] path')].length === 0, ); - assert.equal(await fixture.getByTestId('guest-history-hover-time').count(), 0); - assert.equal(await plot.locator('circle[r="3"]').count(), 0); - assert.match(await text(), /CPU\s*42\.0% Memory\s*53\.0% Disk\s*-/); + // Not inspecting: any readout names the latest stored time, never "Stored ·". + assert.deepEqual( + await fixture + .getByTestId('guest-history-hover-time') + .evaluateAll( + (nodes) => nodes.filter((node) => node.textContent.startsWith('Stored ·')).length, + ), + 0, + ); + assert.equal(await plot.locator('[data-history-hover-marker]').count(), 0); + // The new target's read is held, so the legend shows qualified live + // readings and a plain dash for disk, which has none. + assert.match(await text(), /CPU\s*42\.0%\s*current Memory\s*53\.0%\s*current Disk\s*-/); assert.ok(held); kind = 'service'; await held.fulfill({ json: response('pbs-three', '24h', 'service') }); await slider.waitFor(); await hover(utilization, 0); - assert.match(await text(), /CPU\s*77\.0% Memory\s*- Disk\s*-/); + assert.match(await text(), /CPU\s*77\.0% Memory\s*no reading Disk\s*no reading/); const dimensions = await page.evaluate(() => ({ scroll: document.documentElement.scrollWidth, inner: innerWidth, diff --git a/frontend-modern/browser-tests/history-response.cjs b/frontend-modern/browser-tests/history-response.cjs index 7c17a290ac..916437044c 100644 --- a/frontend-modern/browser-tests/history-response.cjs +++ b/frontend-modern/browser-tests/history-response.cjs @@ -170,7 +170,8 @@ const { chromium, webkit } = require('playwright'); ); mode = 'wrong-range'; value = 97; - await service.getByRole('button', { name: 'Refresh history' }).click(); + // No manual refresh: the next 30-second poll reads the new body. + await page.clock.runFor(30_000); await service .getByText('History refresh failed. Showing previously loaded history.', { exact: true }) .waitFor(); @@ -212,7 +213,8 @@ const { chromium, webkit } = require('playwright'); ); } mode = 'denied'; - await service.getByRole('button', { name: 'Refresh history' }).click(); + // No manual refresh: the next 30-second poll reads the new body. + await page.clock.runFor(30_000); await service .getByText('Access denied. Check your permissions and license plan.', { exact: true }) .waitFor(); @@ -240,7 +242,8 @@ const { chromium, webkit } = require('playwright'); }); await capture('zero-recovered'); mode = 'empty'; - await service.getByRole('button', { name: 'Refresh history' }).click(); + // No manual refresh: the next 30-second poll reads the new body. + await page.clock.runFor(30_000); await service.getByText('No stored history in this range', { exact: true }).first().waitFor(); await page.clock.runFor(30_000); await check('valid empty success is distinct from the initial invalid body', async () => { diff --git a/frontend-modern/browser-tests/history-window.cjs b/frontend-modern/browser-tests/history-window.cjs index c04342a8e1..301f6ef673 100644 --- a/frontend-modern/browser-tests/history-window.cjs +++ b/frontend-modern/browser-tests/history-window.cjs @@ -71,6 +71,8 @@ const { chromium, firefox, webkit } = require('playwright'); ...(width === 390 ? { isMobile: true, hasTouch: true } : {}), }); page.setDefaultTimeout(25_000); + // Time flows normally; runFor advances it to fire the 30-second poll. + await page.clock.install(); const requests = [], errors = []; page.on('pageerror', (error) => errors.push(error.message)); @@ -172,10 +174,30 @@ const { chromium, firefox, webkit } = require('playwright'); ); await page.getByRole('button', { name: 'Corroborate links', exact: true }).click(); progress('linked-table-ready', { theme }); - const xs = async (plot) => - [ - ...(await plot.locator('path').first().getAttribute('d')).matchAll(/[ML](-?[\d.]+),/g), - ].map((match) => Number(match[1])); + // Lines break where a reading is missing, so sparse series draw single + // observation dots instead of a path. Read x from whichever is drawn. + const drawn = (plot) => plot.locator('path[data-history-series], [data-history-observation]'); + const xs = async (plot) => { + const series = plot.locator('path[data-history-series]'); + if ((await series.count()) > 0) { + return [...(await series.first().getAttribute('d')).matchAll(/[ML](-?[\d.]+),/g)].map( + (match) => Number(match[1]), + ); + } + return ( + ( + await plot + .locator('[data-history-observation]') + // The first series drawn, as the path branch reads the first path. + .evaluateAll((nodes) => { + const metric = nodes[0]?.getAttribute('data-history-observation'); + return nodes + .filter((node) => node.getAttribute('data-history-observation') === metric) + .map((node) => Number(node.getAttribute('data-x'))); + }) + ).sort((a, b) => a - b) + ); + }; const expectedX = (minutes, range) => 34 + ((durations[range] - advance - minutes * 60_000) / durations[range]) * 318; const verifyWindow = async (detail, range) => { @@ -220,10 +242,10 @@ const { chromium, firefox, webkit } = require('playwright'); 'true', ); detail = page.locator(`[data-inline-platform-resource-detail-for="pbs-${suffix}"]`); - await detail.getByRole('tab', { name: 'History', exact: true }).click(); + // History charts sit on the drawer's Overview. const utilization = detail.locator('[data-history-group="utilization"]'); const plot = utilization.getByTestId('guest-history-plot'); - await plot.locator('path').first().waitFor(); + await drawn(plot).first().waitFor({ state: 'attached' }); const observedXs = await xs(plot); const observed = { suffix, @@ -265,16 +287,16 @@ const { chromium, firefox, webkit } = require('playwright'); ); const utilizationX = await utilization .getByTestId('guest-history-plot') - .locator('circle[r="3"]') + .locator('[data-history-hover-marker]') .first() - .getAttribute('cx'); + .getAttribute('data-x'); await networkSlider.focus(); await networkSlider.press('End'); const networkX = await network .getByTestId('guest-history-plot') - .locator('circle[r="3"]') + .locator('[data-history-hover-marker]') .first() - .getAttribute('cx'); + .getAttribute('data-x'); assert.equal(networkX, utilizationX); assert.match( (await network.innerText()).replace(/\s+/g, ' '), @@ -294,7 +316,7 @@ const { chromium, firefox, webkit } = require('playwright'); progress('inspection-verified', { theme }); await networkSlider.press('Tab'); state = 'failure'; - await detail.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await detail .getByText('History refresh failed. Showing previously loaded history.') .waitFor(); @@ -310,7 +332,7 @@ const { chromium, firefox, webkit } = require('playwright'); advance = 10 * 60_000; state = 'success'; await detail.getByRole('button', { name: 'Retry history' }).click(); - await detail.getByRole('button', { name: 'Refresh history' }).waitFor(); + await detail.getByRole('button', { name: 'Retry history' }).waitFor({ state: 'detached' }); await page.waitForFunction( (expected) => document @@ -330,12 +352,17 @@ const { chromium, firefox, webkit } = require('playwright'); await detail.getByRole('combobox', { name: 'History range' }).selectOption('1h'); await detail.getByText('Loading history', { exact: true }).first().waitFor(); assert.equal(await detail.getByTestId('guest-history-time-window').count(), 0); - assert.equal(await detail.getByTestId('guest-history-plot').locator('path').count(), 0); + assert.equal( + await detail.getByTestId('guest-history-plot').locator('path[data-history-series]').count(), + 0, + ); assert.equal(await detail.getByRole('slider').count(), 0); assert.ok(held); state = 'success'; await held.route.fulfill({ json: response(held.type, held.id, held.range) }); - await utilization.getByTestId('guest-history-plot').locator('path').first().waitFor(); + await drawn(utilization.getByTestId('guest-history-plot')) + .first() + .waitFor({ state: 'attached' }); await verifyWindow(detail, '1h'); assert.ok( Math.abs( @@ -345,9 +372,11 @@ const { chromium, firefox, webkit } = require('playwright'); await detail.screenshot({ path: path.join(artifacts, `one-hour-${theme}.png`) }); progress('replacement-window-verified', { theme }); state = 'sparse'; - await detail.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await page.waitForFunction( - () => document.querySelectorAll('[data-testid="guest-history-plot"] path').length === 0, + () => + document.querySelectorAll('[data-testid="guest-history-plot"] path[data-history-series]') + .length === 0, ); await detail.screenshot({ path: path.join(artifacts, `sparse-${theme}.png`) }); const sparseObservation = { @@ -368,12 +397,12 @@ const { chromium, firefox, webkit } = require('playwright'); assert.equal(await detail.getByRole('slider').count(), 0); assert.equal(await detail.getByText('Single observation. No trend yet.').count(), 4); for (const dot of await detail.locator('[data-history-observation]').all()) { - assert.ok(Math.abs(Number(await dot.getAttribute('cx')) - expectedX(5, '1h')) < 0.01); - assert.ok(Number.isFinite(Number(await dot.getAttribute('cy')))); + assert.ok(Math.abs(Number(await dot.getAttribute('data-x')) - expectedX(5, '1h')) < 0.01); + assert.ok(Number.isFinite(Number(await dot.getAttribute('data-y')))); } await verifyWindow(detail, '1h'); state = 'failure'; - await detail.getByRole('button', { name: 'Refresh history' }).click(); + await page.clock.runFor(30_000); await detail .getByText('History refresh failed. Showing previously loaded history.') .waitFor(); @@ -394,7 +423,9 @@ const { chromium, firefox, webkit } = require('playwright'); await detail.screenshot({ path: path.join(artifacts, `empty-${theme}.png`) }); state = 'success'; await detail.getByRole('combobox', { name: 'History range' }).selectOption('7d'); - await utilization.getByTestId('guest-history-plot').locator('path').first().waitFor(); + await drawn(utilization.getByTestId('guest-history-plot')) + .first() + .waitFor({ state: 'attached' }); await verifyWindow(detail, '7d'); const readsBeforeLock = requests.length; await detail.getByRole('combobox', { name: 'History range' }).selectOption('14d'); @@ -417,7 +448,10 @@ const { chromium, firefox, webkit } = require('playwright'); ); assert.match((await liveCPU.innerText()).replace(/\s+/g, ' '), /CPU\s*42\.0%\s*current/); assert.equal(await detail.locator('[data-history-observation]').count(), 0); - assert.equal(await detail.getByTestId('guest-history-plot').locator('path').count(), 0); + assert.equal( + await detail.getByTestId('guest-history-plot').locator('path[data-history-series]').count(), + 0, + ); await detail.screenshot({ path: path.join(artifacts, `current-empty-${theme}.png`) }); observations.push({ theme, requests, errors, utilizationX, networkX }); progress('completed-theme', { theme }); diff --git a/frontend-modern/browser-tests/pbs-history-refresh.cjs b/frontend-modern/browser-tests/pbs-history-refresh.cjs index 9a61945e5b..be9cfd64b2 100644 --- a/frontend-modern/browser-tests/pbs-history-refresh.cjs +++ b/frontend-modern/browser-tests/pbs-history-refresh.cjs @@ -109,13 +109,16 @@ const { chromium } = require('playwright'); ); console.log(JSON.stringify({ width, theme, stage: 'page loaded' })); const detail = page.getByTestId('history-refresh-fixture'); - const plots = detail.locator('[data-testid="guest-history-plot"] path'); + const plots = detail.locator( + '[data-testid="guest-history-plot"] path[data-history-series]', + ); console.log(JSON.stringify({ width, theme, stage: 'renderer available' })); const waitForPaths = async (count) => page.waitForFunction( (expected) => - document.querySelectorAll('[data-testid="guest-history-plot"] path').length === - expected, + document.querySelectorAll( + '[data-testid="guest-history-plot"] path[data-history-series]', + ).length === expected, count, ); await waitForPaths(7); @@ -132,17 +135,21 @@ const { chromium } = require('playwright'); inner: innerWidth, })); assert.ok(dimensions.scroll <= dimensions.inner + 1, JSON.stringify(dimensions)); - const button = detail.getByRole('button', { name: /^(Retry|Refresh) history$/ }); - const bounds = await button.boundingBox(); - assert.ok(bounds && bounds.x >= 0 && bounds.x + bounds.width <= width + 1); - if (width === 390) assert.ok(bounds.height >= 44, JSON.stringify(bounds)); + // Retry exists only while a read has failed; polling replaced manual refresh. + const button = detail.getByRole('button', { name: 'Retry history', exact: true }); + if ((await button.count()) > 0) { + const bounds = await button.boundingBox(); + assert.ok(bounds && bounds.x >= 0 && bounds.x + bounds.width <= width + 1); + if (width === 390) assert.ok(bounds.height >= 44, JSON.stringify(bounds)); + } return dimensions; }; const screenshot = (state) => detail.screenshot({ path: path.join(artifacts, `${state}-${theme}-${width}.png`) }); const waitForHeld = async () => { - for (let i = 0; !held && i < 100; i++) await page.waitForTimeout(10); - assert.ok(held, 'manual refresh was not intercepted'); + // Background reads arrive on the 30s poll, so allow one full interval. + for (let i = 0; !held && i < 400; i++) await page.waitForTimeout(100); + assert.ok(held, 'history read was not intercepted'); }; // A failed background read must keep this source's observations, labelled honestly. @@ -172,28 +179,29 @@ const { chromium } = require('playwright'); historyValue = 12; await held.fulfill({ json: response('agent-three', '24h') }); await status.getByText(/History refresh failed/).waitFor({ state: 'hidden' }); - const refresh = detail.getByRole('button', { name: 'Refresh history', exact: true }); - assert.equal(await refresh.evaluate((button) => document.activeElement === button), true); + // Retry leaves with the failure; focus moves to the history region, not the page body. + await retry.waitFor({ state: 'detached' }); + assert.equal( + await detail + .getByTestId('guest-history-region') + .evaluate((region) => document.activeElement === region), + true, + ); assert.notDeepEqual(await paths(), original); - assert.equal(await refresh.getAttribute('aria-busy'), 'false'); await screenshot('retry-recovered'); console.log(JSON.stringify({ width, theme, stage: 'retry recovered' })); - // A slow manual refresh replaced by polling must release its loading ownership. + // A slow poll still in flight when the target changes must not paint the old host. nextAction = 'hold'; held = undefined; - await refresh.click(); await waitForHeld(); const obsoleteRoute = held; - historyValue = 25; - await page.waitForFunction( - () => document.querySelector('button[aria-busy="false"]') !== null, - ); - for (let i = 0; (await refresh.getAttribute('aria-busy')) !== 'false' && i < 100; i++) - await page.waitForTimeout(10); - assert.equal(await refresh.getAttribute('aria-busy'), 'false'); - assert.equal(await refresh.getAttribute('aria-disabled'), 'false'); - const latest = await paths(); + assert.equal(requests.at(-1).id, 'agent-three'); + + // A different target has no right to the old host's history, including after a failure. + nextAction = 'failure'; + await page.getByRole('button', { name: 'Switch to service target', exact: true }).click(); + await status.getByText('Failed to load history data', { exact: true }).waitFor(); let lateResponse; try { await obsoleteRoute.fulfill({ json: response('agent-three', '24h', 99) }); @@ -202,12 +210,6 @@ const { chromium } = require('playwright'); lateResponse = String(error); } await page.evaluate(() => new Promise(requestAnimationFrame)); - assert.deepEqual(await paths(), latest); - - // A different target has no right to the old host's history, including after a failure. - nextAction = 'failure'; - await page.getByRole('button', { name: 'Switch to service target', exact: true }).click(); - await status.getByText('Failed to load history data', { exact: true }).waitFor(); assert.equal(await plots.count(), 0); assert.equal( await detail.getByText('No stored history in this range', { exact: true }).count(), @@ -220,14 +222,20 @@ const { chromium } = require('playwright'); await waitForPaths(2); assert.equal(requests.at(-1).id, 'pbs-three'); assert.equal(requests.at(-1).range, '24h'); - assert.equal(await detail.locator('[data-history-group="network"] path').count(), 0); - assert.equal(await detail.locator('[data-history-group="disk-io"] path').count(), 0); + assert.equal( + await detail.locator('[data-history-group="network"] path[data-history-series]').count(), + 0, + ); + assert.equal( + await detail.locator('[data-history-group="disk-io"] path[data-history-series]').count(), + 0, + ); const dimensions = await checkLayout(); const beforeLocked = requests.length; await detail.getByTestId('guest-history-range-control').selectOption('14d'); await detail.getByText(/14 days history requires a higher license plan/).waitFor(); assert.equal( - await detail.getByRole('button', { name: /^(Retry|Refresh) history$/ }).count(), + await detail.getByRole('button', { name: 'Retry history', exact: true }).count(), 0, ); assert.equal(requests.length, beforeLocked); diff --git a/frontend-modern/browser-tests/pbs-history-refresh.tsx b/frontend-modern/browser-tests/pbs-history-refresh.tsx index 77f2886739..eb82e804f8 100644 --- a/frontend-modern/browser-tests/pbs-history-refresh.tsx +++ b/frontend-modern/browser-tests/pbs-history-refresh.tsx @@ -1,12 +1,12 @@ // Direct production renderer proof; this is not the complete PBS table/drawer. import { createSignal } from 'solid-js'; -import { render } from 'solid-js/web'; import type { HistoryTimeRange } from '../src/api/charts'; import { GuestDrawerHistory, GuestDrawerHistoryRangeSelect, } from '../src/components/Workloads/GuestDrawerHistory'; import { HOST_METRICS_HISTORY_GROUPS } from '../src/components/shared/hostMetricsHistoryModel'; +import { renderInAppRuntime } from './fixtureRuntime'; import '../src/index.css'; const Fixture = () => { @@ -32,4 +32,4 @@ const Fixture = () => { ); }; -render(() => , document.getElementById('root') as HTMLElement); +renderInAppRuntime(Fixture); diff --git a/frontend-modern/browser-tests/pbs-history-source-isolation.cjs b/frontend-modern/browser-tests/pbs-history-source-isolation.cjs index 27ba7086bb..65a864f1b5 100644 --- a/frontend-modern/browser-tests/pbs-history-source-isolation.cjs +++ b/frontend-modern/browser-tests/pbs-history-source-isolation.cjs @@ -114,10 +114,12 @@ const { chromium } = require('playwright'); await page.keyboard.press('Enter'); } else await page.locator('td[title="backup-connection-three · tank"]').click(); const detail = page.locator('[data-inline-platform-resource-detail-for="pbs-three"]'); - const plots = detail.locator('[data-testid="guest-history-plot"] path'); - await detail.getByRole('tab', { name: 'History', exact: true }).click(); + // History charts sit on Overview, so the read starts when it opens. + const plots = detail.locator('[data-testid="guest-history-plot"] path[data-history-series]'); await page.waitForFunction( - () => document.querySelectorAll('[data-testid="guest-history-plot"] path').length === 7, + () => + document.querySelectorAll('[data-testid="guest-history-plot"] path[data-history-series]') + .length === 7, ); const originalPaths = await plots.evaluateAll((paths) => paths.map((p) => p.getAttribute('d')), @@ -130,7 +132,7 @@ const { chromium } = require('playwright'); assert.ok(dimensions.scroll <= dimensions.inner + 1, JSON.stringify(dimensions)); assert.equal( await detail - .getByRole('tab', { name: 'History', exact: true }) + .getByRole('tab', { name: 'Overview', exact: true }) .getAttribute('aria-selected'), 'true', ); @@ -153,7 +155,9 @@ const { chromium } = require('playwright'); await checkLayout(); await detail.getByTestId('guest-history-range-control').selectOption('1h'); await page.waitForFunction( - () => document.querySelectorAll('[data-testid="guest-history-plot"] path').length === 7, + () => + document.querySelectorAll('[data-testid="guest-history-plot"] path[data-history-series]') + .length === 7, ); const currentPaths = await plots.evaluateAll((paths) => paths.map((p) => p.getAttribute('d')), @@ -188,7 +192,9 @@ const { chromium } = require('playwright'); await screenshot('target-failed'); await detail.getByTestId('guest-history-range-control').selectOption('6h'); await page.waitForFunction( - () => document.querySelectorAll('[data-testid="guest-history-plot"] path').length === 2, + () => + document.querySelectorAll('[data-testid="guest-history-plot"] path[data-history-series]') + .length === 2, ); assert.equal(await detail.locator('[data-history-group="network"] path').count(), 0); assert.equal(await detail.locator('[data-history-group="disk-io"] path').count(), 0); diff --git a/frontend-modern/browser-tests/pbs-identity-boundary.cjs b/frontend-modern/browser-tests/pbs-identity-boundary.cjs index 01ab2a9b0a..8e8927193f 100644 --- a/frontend-modern/browser-tests/pbs-identity-boundary.cjs +++ b/frontend-modern/browser-tests/pbs-identity-boundary.cjs @@ -103,16 +103,25 @@ const { chromium } = require('playwright'); await detail.waitFor(); const overview = detail.getByRole('tab', { name: 'Overview', exact: true }); await overview.click(); + // History charts sit on Overview, so the read starts when it opens. + await detail + .locator( + '[data-testid="guest-history-plot"] path[data-history-series], [data-history-observation]', + ) + .first() + .waitFor({ state: 'attached' }); + // The Identity card shows each identifier once, so Metrics Target may be + // absent when it repeats one; when shown it must name the host series. const identity = detail .locator('[data-testid="resource-identity-section"] tr') .filter({ hasText: 'Metrics Target' }); - await identity.getByText(expected, { exact: true }).waitFor(); + if ((await identity.count()) > 0) { + await identity.getByText(expected, { exact: true }).waitFor(); + } assert.ok(!(await detail.innerText()).includes('/WRONG-HOST'), 'wrong-host disks rendered'); - await detail.getByRole('tab', { name: 'History', exact: true }).click(); - await detail.locator('[data-testid="guest-history-plot"] path').first().waitFor(); assert.equal( await detail - .getByRole('tab', { name: 'History', exact: true }) + .getByRole('tab', { name: 'Overview', exact: true }) .getAttribute('aria-selected'), 'true', ); diff --git a/frontend-modern/browser-tests/pbs-identity-boundary.tsx b/frontend-modern/browser-tests/pbs-identity-boundary.tsx index bc3881d00b..62a5609999 100644 --- a/frontend-modern/browser-tests/pbs-identity-boundary.tsx +++ b/frontend-modern/browser-tests/pbs-identity-boundary.tsx @@ -1,10 +1,10 @@ // Synthetic bounded Backups/Overview resources, not installed host evidence. import { createMemo, createSignal } from 'solid-js'; -import { render } from 'solid-js/web'; import { ProxmoxBackupServersTable } from '../src/features/proxmox/ProxmoxBackupServersTable'; import type { Resource } from '../src/types/resource'; import '../src/index.css'; +import { renderInAppRuntime } from './fixtureRuntime'; const seen = Date.now(); const suffixes = ['one', 'two', 'three']; @@ -113,4 +113,4 @@ const Fixture = () => { ); }; -render(() => , document.getElementById('root') as HTMLElement); +renderInAppRuntime(Fixture); diff --git a/frontend-modern/browser-verification/f49d0d6d125bd3bf.json b/frontend-modern/browser-verification/f49d0d6d125bd3bf.json new file mode 100644 index 0000000000..68bd908190 --- /dev/null +++ b/frontend-modern/browser-verification/f49d0d6d125bd3bf.json @@ -0,0 +1,42 @@ +{ + "version": 1, + "base_sha": "b49e5502f9dcc191d2194d925ee67e2066920683", + "verified_at": "2026-10-14T10:09:48Z", + "result": "passed", + "changed_paths": [ + "frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx" + ], + "content_sha256": { + "frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx": "211005e227bc233e710724906f310986898aa81faf8f2e0829bb945869445a44" + }, + "routes": [ + "/browser-tests/pbs-history-refresh.html (production GuestDrawerHistory renderer in the app runtime, Chromium via Playwright 1.56.1 on pulse-dev)" + ], + "viewports": [ + { + "width": 1365, + "height": 900 + }, + { + "width": 390, + "height": 844 + } + ], + "states": [ + "loaded 24h history (light and dark)", + "background poll failed: previous history kept with the \"History refresh failed\" status and Retry history", + "retry in flight: Retry aria-busy and aria-disabled, old paths kept", + "retry recovered: Retry detached, status empty, new paths drawn", + "phone: later panels collapsed behind the Show button", + "target switch with a held poll for the old host: late response not painted", + "replacement target failed with no previous history", + "14d range locked: no Retry" + ], + "interactions": [ + "Enter on focused Retry history starts one read; a second Enter is ignored while busy", + "successful retry moves keyboard focus to the guest-history-region element (Chromium blurs the removed button)", + "waited out the 30s poll to hold a read, switched target, then fulfilled the stale read", + "Retry after the replacement failure loads the new target", + "range select changed to 14 days" + ] +} diff --git a/frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx b/frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx index 4a6f532fcd..7535a3a309 100644 --- a/frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx +++ b/frontend-modern/src/components/Workloads/GuestDrawerHistory.tsx @@ -6,6 +6,7 @@ import { createSignal, createUniqueId, on, + onCleanup, onMount, type Component, } from 'solid-js'; @@ -846,16 +847,18 @@ export const GuestDrawerHistory: Component = (props) => const historyQuery = createGuestDrawerHistoryQuery(historySource); // A successful retry removes the focused Retry button with the error. Keep // keyboard focus in the history region instead of dropping it to the page. + // Focus is read when the button's branch is disposed, before its node leaves + // the DOM: Chromium blurs a removed element, so a blur-tracked flag is + // already cleared by the time the error effect runs. let regionRef: HTMLDivElement | undefined; - let retryHasFocus = false; + let retryRef: HTMLButtonElement | undefined; + let retryHadFocus = false; createEffect( on( () => historyQuery.error(), (error, previous) => { - if (previous && !error && retryHasFocus) { - retryHasFocus = false; - regionRef?.focus(); - } + if (previous && !error && retryHadFocus) regionRef?.focus(); + retryHadFocus = false; }, ), ); @@ -938,24 +941,30 @@ export const GuestDrawerHistory: Component = (props) => {/* The charts poll every 30 seconds, so a manual refresh is offered only to recover from a failed read. */} - + {(_error) => { + onCleanup(() => { + retryHadFocus = retryRef !== undefined && document.activeElement === retryRef; + }); + return ( + + ); + }} container.querySelectorAll('[data-testid="guest-history-plot"] path'); +// Chromium blurs a focused element as it leaves the DOM; jsdom does not. Model +// that so focus handoffs cannot depend on blur never firing on removal. +const blurFocusedOnRemoval = () => { + const blurIfInside = (node: Node) => { + const active = document.activeElement; + if (active instanceof HTMLElement && node.contains(active)) active.blur(); + }; + const remove = Element.prototype.remove; + const removeChild = Node.prototype.removeChild; + const replaceChild = Node.prototype.replaceChild; + vi.spyOn(Element.prototype, 'remove').mockImplementation(function (this: Element) { + blurIfInside(this); + remove.call(this); + }); + vi.spyOn(Node.prototype, 'removeChild').mockImplementation(function ( + this: Node, + child: T, + ) { + blurIfInside(child); + return removeChild.call(this, child) as T; + }); + vi.spyOn(Node.prototype, 'replaceChild').mockImplementation(function ( + this: Node, + node: Node, + child: T, + ) { + blurIfInside(child); + return replaceChild.call(this, node, child) as T; + }); +}; + function deferred() { let resolve!: (value: T) => void; let reject!: (error: Error) => void; @@ -99,6 +130,7 @@ describe('GuestDrawerHistory refresh recovery', () => { .mockResolvedValueOnce(response()) .mockRejectedValueOnce(new Error('Unavailable')) .mockReturnValueOnce(pending.promise); + blurFocusedOnRemoval(); const { container } = render(() => ); await vi.advanceTimersByTimeAsync(30_000); const original = paths(container)[0].getAttribute('d');