feat(trace viewer): show Screenshot instead of Snapshot (#32248)
Closes https://github.com/microsoft/playwright/issues/23964. Trace snapshots are a best-effort snapshots of the browser DOM, but we can't guarantee them to be exactly what the browser showed. One example of this is `canvas` elements, where you just can't see their contents. That makes snapshots useful, but not perfect. For those cases where the snapshot doesn't show everything, this PR introduces a new setting to show a screenshot instead. You won't be able to scroll or inspect the DOM or select a locator anymore. But if the snapshot was missing something, or displaying something wrong, you can now check the screenshot instead.
This commit is contained in:
parent
ed303208b3
commit
3fe1263643
|
|
@ -41,6 +41,7 @@ export type ContextEntry = {
|
|||
};
|
||||
|
||||
export type PageEntry = {
|
||||
pageId: string,
|
||||
screencastFrames: {
|
||||
sha1: string,
|
||||
timestamp: number,
|
||||
|
|
|
|||
|
|
@ -44,7 +44,8 @@ export class SnapshotServer {
|
|||
const snapshot = this._snapshot(pathname.substring('/snapshotInfo'.length), searchParams);
|
||||
return this._respondWithJson(snapshot ? {
|
||||
viewport: snapshot.viewport(),
|
||||
url: snapshot.snapshot().frameUrl
|
||||
url: snapshot.snapshot().frameUrl,
|
||||
timestamp: snapshot.snapshot().timestamp,
|
||||
} : {
|
||||
error: 'No snapshot found'
|
||||
});
|
||||
|
|
|
|||
|
|
@ -58,6 +58,7 @@ export class TraceModernizer {
|
|||
let pageEntry = this._pageEntries.get(pageId);
|
||||
if (!pageEntry) {
|
||||
pageEntry = {
|
||||
pageId,
|
||||
screencastFrames: [],
|
||||
};
|
||||
this._pageEntries.set(pageId, pageEntry);
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@
|
|||
import { CodeMirrorWrapper } from '@web/components/codeMirrorWrapper';
|
||||
import type { Language } from '@web/components/codeMirrorWrapper';
|
||||
import { ToolbarButton } from '@web/components/toolbarButton';
|
||||
import { copy } from '@web/uiUtils';
|
||||
import { copy, useSetting } from '@web/uiUtils';
|
||||
import * as React from 'react';
|
||||
import './sourceTab.css';
|
||||
|
||||
|
|
@ -27,10 +27,12 @@ export const InspectorTab: React.FunctionComponent<{
|
|||
highlightedLocator: string,
|
||||
setHighlightedLocator: (locator: string) => void,
|
||||
}> = ({ sdkLanguage, setIsInspecting, highlightedLocator, setHighlightedLocator }) => {
|
||||
const [showScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
|
||||
|
||||
return <div className='vbox' style={{ backgroundColor: 'var(--vscode-sideBar-background)' }}>
|
||||
<div style={{ margin: '10px 0px 10px 10px', color: 'var(--vscode-editorCodeLens-foreground)', flex: 'none' }}>Locator</div>
|
||||
<div style={{ margin: '0 10px 10px', flex: 'auto' }}>
|
||||
<CodeMirrorWrapper text={highlightedLocator} language={sdkLanguage} focusOnChange={true} isFocused={true} wrapLines={true} onChange={text => {
|
||||
<CodeMirrorWrapper text={showScreenshot ? '/* disable "show screenshot" setting to pick locator */' : highlightedLocator} language={sdkLanguage} focusOnChange={true} isFocused={true} wrapLines={true} onChange={text => {
|
||||
// Updating text needs to go first - react can squeeze a render between the state updates.
|
||||
setHighlightedLocator(text);
|
||||
setIsInspecting(false);
|
||||
|
|
|
|||
|
|
@ -22,6 +22,7 @@ import type { ContextEntry, PageEntry } from '../entries';
|
|||
import type { StackFrame } from '@protocol/channels';
|
||||
|
||||
const contextSymbol = Symbol('context');
|
||||
const pageSymbol = Symbol('page');
|
||||
const nextInContextSymbol = Symbol('next');
|
||||
const prevInListSymbol = Symbol('prev');
|
||||
const eventsSymbol = Symbol('events');
|
||||
|
|
@ -148,6 +149,7 @@ function indexModel(context: ContextEntry) {
|
|||
for (let i = 0; i < context.actions.length; ++i) {
|
||||
const action = context.actions[i] as any;
|
||||
action[contextSymbol] = context;
|
||||
action[pageSymbol] = context.pages.find(page => page.pageId === action.pageId);
|
||||
}
|
||||
let lastNonRouteAction = undefined;
|
||||
for (let i = context.actions.length - 1; i >= 0; i--) {
|
||||
|
|
@ -356,6 +358,10 @@ export function prevInList(action: ActionTraceEvent): ActionTraceEvent {
|
|||
return (action as any)[prevInListSymbol];
|
||||
}
|
||||
|
||||
export function pageForAction(action: ActionTraceEvent): PageEntry {
|
||||
return (action as any)[pageSymbol];
|
||||
}
|
||||
|
||||
export function stats(action: ActionTraceEvent): { errors: number, warnings: number } {
|
||||
let errors = 0;
|
||||
let warnings = 0;
|
||||
|
|
|
|||
|
|
@ -17,10 +17,10 @@
|
|||
import './snapshotTab.css';
|
||||
import * as React from 'react';
|
||||
import type { ActionTraceEvent } from '@trace/trace';
|
||||
import { context, prevInList } from './modelUtil';
|
||||
import { context, type MultiTraceModel, pageForAction, prevInList } from './modelUtil';
|
||||
import { Toolbar } from '@web/components/toolbar';
|
||||
import { ToolbarButton } from '@web/components/toolbarButton';
|
||||
import { clsx, useMeasure } from '@web/uiUtils';
|
||||
import { clsx, useMeasure, useSetting } from '@web/uiUtils';
|
||||
import { InjectedScript } from '@injected/injectedScript';
|
||||
import { Recorder } from '@injected/recorder/recorder';
|
||||
import ConsoleAPI from '@injected/consoleApi';
|
||||
|
|
@ -30,8 +30,18 @@ import { locatorOrSelectorAsSelector } from '@isomorphic/locatorParser';
|
|||
import { TabbedPaneTab } from '@web/components/tabbedPane';
|
||||
import { BrowserFrame } from './browserFrame';
|
||||
|
||||
function findClosest<T extends { timestamp: number }>(items: T[], target: number) {
|
||||
return items.find((item, index) => {
|
||||
if (index === items.length - 1)
|
||||
return true;
|
||||
const next = items[index + 1];
|
||||
return Math.abs(item.timestamp - target) < Math.abs(next.timestamp - target);
|
||||
});
|
||||
}
|
||||
|
||||
export const SnapshotTab: React.FunctionComponent<{
|
||||
action: ActionTraceEvent | undefined,
|
||||
model?: MultiTraceModel,
|
||||
sdkLanguage: Language,
|
||||
testIdAttributeName: string,
|
||||
isInspecting: boolean,
|
||||
|
|
@ -39,9 +49,10 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
highlightedLocator: string,
|
||||
setHighlightedLocator: (locator: string) => void,
|
||||
openPage?: (url: string, target?: string) => Window | any,
|
||||
}> = ({ action, sdkLanguage, testIdAttributeName, isInspecting, setIsInspecting, highlightedLocator, setHighlightedLocator, openPage }) => {
|
||||
}> = ({ action, model, sdkLanguage, testIdAttributeName, isInspecting, setIsInspecting, highlightedLocator, setHighlightedLocator, openPage }) => {
|
||||
const [measure, ref] = useMeasure<HTMLDivElement>();
|
||||
const [snapshotTab, setSnapshotTab] = React.useState<'action'|'before'|'after'>('action');
|
||||
const [showScreenshotInsteadOfSnapshot] = useSetting('screenshot-instead-of-snapshot', false);
|
||||
|
||||
type Snapshot = { action: ActionTraceEvent, snapshotName: string, point?: { x: number, y: number } };
|
||||
const { snapshots } = React.useMemo(() => {
|
||||
|
|
@ -90,7 +101,7 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
|
||||
const iframeRef0 = React.useRef<HTMLIFrameElement>(null);
|
||||
const iframeRef1 = React.useRef<HTMLIFrameElement>(null);
|
||||
const [snapshotInfo, setSnapshotInfo] = React.useState({ viewport: kDefaultViewport, url: '' });
|
||||
const [snapshotInfo, setSnapshotInfo] = React.useState<{ viewport: typeof kDefaultViewport, url: string, timestamp?: number }>({ viewport: kDefaultViewport, url: '', timestamp: undefined });
|
||||
const loadingRef = React.useRef({ iteration: 0, visibleIframe: 0 });
|
||||
|
||||
React.useEffect(() => {
|
||||
|
|
@ -99,13 +110,14 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
const newVisibleIframe = 1 - loadingRef.current.visibleIframe;
|
||||
loadingRef.current.iteration = thisIteration;
|
||||
|
||||
const newSnapshotInfo = { url: '', viewport: kDefaultViewport };
|
||||
const newSnapshotInfo = { url: '', viewport: kDefaultViewport, timestamp: undefined };
|
||||
if (snapshotInfoUrl) {
|
||||
const response = await fetch(snapshotInfoUrl);
|
||||
const info = await response.json();
|
||||
if (!info.error) {
|
||||
newSnapshotInfo.url = info.url;
|
||||
newSnapshotInfo.viewport = info.viewport;
|
||||
newSnapshotInfo.timestamp = info.timestamp;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -154,6 +166,15 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
y: (measure.height - snapshotContainerSize.height) / 2,
|
||||
};
|
||||
|
||||
const page = action ? pageForAction(action) : undefined;
|
||||
const screencastFrame = React.useMemo(
|
||||
() => {
|
||||
if (snapshotInfo.timestamp && page?.screencastFrames)
|
||||
return findClosest(page.screencastFrames, snapshotInfo.timestamp);
|
||||
},
|
||||
[page?.screencastFrames, snapshotInfo.timestamp]
|
||||
);
|
||||
|
||||
return <div
|
||||
className='snapshot-tab'
|
||||
tabIndex={0}
|
||||
|
|
@ -181,7 +202,7 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
iframe={iframeRef1.current}
|
||||
iteration={loadingRef.current.iteration} />
|
||||
<Toolbar>
|
||||
<ToolbarButton className='pick-locator' title='Pick locator' icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} />
|
||||
<ToolbarButton className='pick-locator' title={showScreenshotInsteadOfSnapshot ? 'Disable "screenshots instead of snapshots" to pick a locator' : 'Pick locator'} icon='target' toggled={isInspecting} onClick={() => setIsInspecting(!isInspecting)} disabled={showScreenshotInsteadOfSnapshot} />
|
||||
{['action', 'before', 'after'].map(tab => {
|
||||
return <TabbedPaneTab
|
||||
key={tab}
|
||||
|
|
@ -192,7 +213,7 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
></TabbedPaneTab>;
|
||||
})}
|
||||
<div style={{ flex: 'auto' }}></div>
|
||||
<ToolbarButton icon='link-external' title='Open snapshot in a new tab' disabled={!popoutUrl} onClick={() => {
|
||||
<ToolbarButton icon='link-external' title={showScreenshotInsteadOfSnapshot ? 'Not available when showing screenshot' : 'Open snapshot in a new tab'} disabled={!popoutUrl || showScreenshotInsteadOfSnapshot} onClick={() => {
|
||||
if (!openPage)
|
||||
openPage = window.open;
|
||||
const win = openPage(popoutUrl || '', '_blank');
|
||||
|
|
@ -209,7 +230,8 @@ export const SnapshotTab: React.FunctionComponent<{
|
|||
transform: `translate(${translate.x}px, ${translate.y}px) scale(${scale})`,
|
||||
}}>
|
||||
<BrowserFrame url={snapshotInfo.url} />
|
||||
<div className='snapshot-switcher'>
|
||||
{(showScreenshotInsteadOfSnapshot && screencastFrame) && <img alt={`Screenshot of ${action?.apiName} > ${renderTitle(snapshotTab)}`} src={`sha1/${screencastFrame.sha1}`} width={screencastFrame.width} height={screencastFrame.height} />}
|
||||
<div className='snapshot-switcher' style={showScreenshotInsteadOfSnapshot ? { display: 'none' } : undefined}>
|
||||
<iframe ref={iframeRef0} name='snapshot' title='DOM Snapshot' className={clsx(loadingRef.current.visibleIframe === 0 && 'snapshot-visible')}></iframe>
|
||||
<iframe ref={iframeRef1} name='snapshot' title='DOM Snapshot' className={clsx(loadingRef.current.visibleIframe === 1 && 'snapshot-visible')}></iframe>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -107,6 +107,8 @@ export const UIModeView: React.FC<{}> = ({
|
|||
const [updateSnapshots, setUpdateSnapshots] = React.useState(queryParams.updateSnapshots === 'all');
|
||||
const [showRouteActions, setShowRouteActions] = useSetting('show-route-actions', true);
|
||||
const [darkMode, setDarkMode] = useDarkModeSetting();
|
||||
const [showScreenshot, setShowScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
|
||||
|
||||
|
||||
const inputRef = React.useRef<HTMLInputElement>(null);
|
||||
|
||||
|
|
@ -517,6 +519,7 @@ export const UIModeView: React.FC<{}> = ({
|
|||
{settingsVisible && <SettingsView settings={[
|
||||
{ value: darkMode, set: setDarkMode, title: 'Dark mode' },
|
||||
{ value: showRouteActions, set: setShowRouteActions, title: 'Show route actions' },
|
||||
{ value: showScreenshot, set: setShowScreenshot, title: 'Show screenshot instead of snapshot' },
|
||||
]} />}
|
||||
</div>
|
||||
}
|
||||
|
|
|
|||
|
|
@ -73,6 +73,8 @@ export const Workbench: React.FunctionComponent<{
|
|||
const [selectedTime, setSelectedTime] = React.useState<Boundaries | undefined>();
|
||||
const [sidebarLocation, setSidebarLocation] = useSetting<'bottom' | 'right'>('propertiesSidebarLocation', 'bottom');
|
||||
const [showRouteActions, setShowRouteActions] = useSetting('show-route-actions', true);
|
||||
const [showScreenshot, setShowScreenshot] = useSetting('screenshot-instead-of-snapshot', false);
|
||||
|
||||
|
||||
const filteredActions = React.useMemo(() => {
|
||||
return (model?.actions || []).filter(action => showRouteActions || !isRouteAction(action));
|
||||
|
|
@ -299,7 +301,10 @@ export const Workbench: React.FunctionComponent<{
|
|||
const settingsTab: TabbedPaneTabModel = {
|
||||
id: 'settings',
|
||||
title: 'Settings',
|
||||
component: <SettingsView settings={[{ value: showRouteActions, set: setShowRouteActions, title: 'Show route actions' }]}/>,
|
||||
component: <SettingsView settings={[
|
||||
{ value: showRouteActions, set: setShowRouteActions, title: 'Show route actions' },
|
||||
{ value: showScreenshot, set: setShowScreenshot, title: 'Show screenshot instead of snapshot' }
|
||||
]}/>,
|
||||
};
|
||||
|
||||
return <div className='vbox workbench' {...(inert ? { inert: 'true' } : {})}>
|
||||
|
|
@ -325,6 +330,7 @@ export const Workbench: React.FunctionComponent<{
|
|||
settingName='actionListSidebar'
|
||||
main={<SnapshotTab
|
||||
action={activeAction}
|
||||
model={model}
|
||||
sdkLanguage={sdkLanguage}
|
||||
testIdAttributeName={model?.testIdAttributeName || 'data-testid'}
|
||||
isInspecting={isInspecting}
|
||||
|
|
|
|||
|
|
@ -139,7 +139,7 @@ export function copy(text: string) {
|
|||
textArea.remove();
|
||||
}
|
||||
|
||||
export function useSetting<S>(name: string | undefined, defaultValue: S, title?: string): [S, React.Dispatch<React.SetStateAction<S>>] {
|
||||
export function useSetting<S>(name: string | undefined, defaultValue: S): [S, React.Dispatch<React.SetStateAction<S>>] {
|
||||
if (name)
|
||||
defaultValue = settings.getObject(name, defaultValue);
|
||||
const [value, setValue] = React.useState<S>(defaultValue);
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ type BaseWorkerFixtures = {
|
|||
|
||||
export type TraceViewerFixtures = {
|
||||
showTraceViewer: (trace: string[], options?: {host?: string, port?: number}) => Promise<TraceViewerPage>;
|
||||
runAndTrace: (body: () => Promise<void>) => Promise<TraceViewerPage>;
|
||||
runAndTrace: (body: () => Promise<void>, optsOverrides?: Parameters<BrowserContext['tracing']['start']>[0]) => Promise<TraceViewerPage>;
|
||||
};
|
||||
|
||||
class TraceViewerPage {
|
||||
|
|
@ -127,9 +127,9 @@ export const traceViewerFixtures: Fixtures<TraceViewerFixtures, {}, BaseTestFixt
|
|||
},
|
||||
|
||||
runAndTrace: async ({ context, showTraceViewer }, use, testInfo) => {
|
||||
await use(async (body: () => Promise<void>) => {
|
||||
await use(async (body: () => Promise<void>, optsOverrides = {}) => {
|
||||
const traceFile = testInfo.outputPath('trace.zip');
|
||||
await context.tracing.start({ snapshots: true, screenshots: true, sources: true });
|
||||
await context.tracing.start({ snapshots: true, screenshots: true, sources: true, ...optsOverrides });
|
||||
await body();
|
||||
await context.tracing.stop({ path: traceFile });
|
||||
return showTraceViewer([traceFile]);
|
||||
|
|
|
|||
|
|
@ -1465,3 +1465,33 @@ test('should serve css without content-type', async ({ page, runAndTrace, server
|
|||
const snapshotFrame = await traceViewer.snapshotFrame('page.goto');
|
||||
await expect(snapshotFrame.locator('body')).toHaveCSS('background-color', 'rgb(255, 0, 0)', { timeout: 0 });
|
||||
});
|
||||
|
||||
test('should allow showing screenshots instead of snapshots', async ({ runAndTrace, page, server }) => {
|
||||
const traceViewer = await runAndTrace(async () => {
|
||||
await page.goto(server.PREFIX + '/one-style.html');
|
||||
});
|
||||
|
||||
const screenshot = traceViewer.page.getByAltText(`Screenshot of page.goto > Action`);
|
||||
const snapshot = (await traceViewer.snapshotFrame('page.goto')).owner();
|
||||
await expect(snapshot).toBeVisible();
|
||||
await expect(screenshot).not.toBeVisible();
|
||||
|
||||
await traceViewer.page.getByTitle('Settings').click();
|
||||
await traceViewer.page.getByText('Show screenshot instead of snapshot').setChecked(true);
|
||||
|
||||
await expect(snapshot).not.toBeVisible();
|
||||
await expect(screenshot).toBeVisible();
|
||||
});
|
||||
|
||||
test('should handle case where neither snapshots nor screenshots exist', async ({ runAndTrace, page, server }) => {
|
||||
const traceViewer = await runAndTrace(async () => {
|
||||
await page.goto(server.PREFIX + '/one-style.html');
|
||||
}, { snapshots: false, screenshots: false });
|
||||
|
||||
await traceViewer.page.getByTitle('Settings').click();
|
||||
await traceViewer.page.getByText('Show screenshot instead of snapshot').setChecked(true);
|
||||
|
||||
const screenshot = traceViewer.page.getByAltText(`Screenshot of page.goto > Action`);
|
||||
await expect(screenshot).not.toBeVisible();
|
||||
});
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue