diff --git a/packages/trace-viewer/src/snapshotRenderer.ts b/packages/trace-viewer/src/snapshotRenderer.ts index 11e63de04f..0d2f2af502 100644 --- a/packages/trace-viewer/src/snapshotRenderer.ts +++ b/packages/trace-viewer/src/snapshotRenderer.ts @@ -346,6 +346,8 @@ function snapshotScript(...targetIds: (string | undefined)[]) { if (search.get('pointX') && search.get('pointY')) { const pointX = +search.get('pointX')!; const pointY = +search.get('pointY')!; + const hasInputTarget = search.has('hasInputTarget'); + const isTopFrame = window.location.pathname.match(/\/page@[a-z0-9]+$/); const hasTargetElements = targetElements.length > 0; const roots = document.documentElement ? [document.documentElement] : []; for (const target of (hasTargetElements ? targetElements : roots)) { @@ -370,7 +372,8 @@ function snapshotScript(...targetIds: (string | undefined)[]) { pointElement.style.left = centerX + 'px'; pointElement.style.top = centerY + 'px'; // "Warning symbol" indicates that action point is not 100% correct. - if (Math.abs(centerX - pointX) >= 10 || Math.abs(centerY - pointY) >= 10) { + // Note that action point is relative to the top frame, so we can only compare in the top frame. + if (isTopFrame && (Math.abs(centerX - pointX) >= 10 || Math.abs(centerY - pointY) >= 10)) { const warningElement = document.createElement('x-pw-pointer-warning'); warningElement.textContent = '⚠'; warningElement.style.fontSize = '19px'; @@ -380,13 +383,14 @@ function snapshotScript(...targetIds: (string | undefined)[]) { pointElement.appendChild(warningElement); pointElement.setAttribute('title', kPointerWarningTitle); } - } else { + document.documentElement.appendChild(pointElement); + } else if (isTopFrame && !hasInputTarget) { // For actions without a target element, e.g. page.mouse.move(), - // show the point at the recorder location. + // show the point at the recorded location, which is relative to the top frame. pointElement.style.left = pointX + 'px'; pointElement.style.top = pointY + 'px'; + document.documentElement.appendChild(pointElement); } - document.documentElement.appendChild(pointElement); } } }; diff --git a/packages/trace-viewer/src/ui/snapshotTab.tsx b/packages/trace-viewer/src/ui/snapshotTab.tsx index 00fcbf64e9..9dafa10b96 100644 --- a/packages/trace-viewer/src/ui/snapshotTab.tsx +++ b/packages/trace-viewer/src/ui/snapshotTab.tsx @@ -55,7 +55,7 @@ export const SnapshotTab: React.FunctionComponent<{ 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 } }; + type Snapshot = { action: ActionTraceEvent, snapshotName: string, point?: { x: number, y: number }, hasInputTarget?: boolean }; const { snapshots } = React.useMemo(() => { if (!action) return { snapshots: {} }; @@ -68,7 +68,7 @@ export const SnapshotTab: React.FunctionComponent<{ beforeSnapshot = a?.afterSnapshot ? { action: a, snapshotName: a?.afterSnapshot } : undefined; } const afterSnapshot: Snapshot | undefined = action.afterSnapshot ? { action, snapshotName: action.afterSnapshot } : beforeSnapshot; - const actionSnapshot: Snapshot | undefined = action.inputSnapshot ? { action, snapshotName: action.inputSnapshot } : afterSnapshot; + const actionSnapshot: Snapshot | undefined = action.inputSnapshot ? { action, snapshotName: action.inputSnapshot, hasInputTarget: true } : afterSnapshot; if (actionSnapshot) actionSnapshot.point = action.point; return { snapshots: { action: actionSnapshot, before: beforeSnapshot, after: afterSnapshot } }; @@ -85,6 +85,8 @@ export const SnapshotTab: React.FunctionComponent<{ if (snapshot.point) { params.set('pointX', String(snapshot.point.x)); params.set('pointY', String(snapshot.point.y)); + if (snapshot.hasInputTarget) + params.set('hasInputTarget', '1'); } const snapshotUrl = new URL(`snapshot/${snapshot.action.pageId}?${params.toString()}`, window.location.href).toString(); const snapshotInfoUrl = new URL(`snapshotInfo/${snapshot.action.pageId}?${params.toString()}`, window.location.href).toString(); @@ -95,6 +97,8 @@ export const SnapshotTab: React.FunctionComponent<{ if (snapshot.point) { popoutParams.set('pointX', String(snapshot.point.x)); popoutParams.set('pointY', String(snapshot.point.y)); + if (snapshot.hasInputTarget) + params.set('hasInputTarget', '1'); } const popoutUrl = new URL(`snapshot.html?${popoutParams.toString()}`, window.location.href).toString(); return { snapshots, snapshotInfoUrl, snapshotUrl, popoutUrl, point: snapshot.point }; diff --git a/tests/library/trace-viewer.spec.ts b/tests/library/trace-viewer.spec.ts index 8f8d69116f..2b00949b19 100644 --- a/tests/library/trace-viewer.spec.ts +++ b/tests/library/trace-viewer.spec.ts @@ -1496,3 +1496,28 @@ test('should handle case where neither snapshots nor screenshots exist', async ( await expect(screenshot).not.toBeVisible(); }); +test('should show only one pointer with multilevel iframes', async ({ page, runAndTrace, server, browserName }) => { + test.fixme(browserName !== 'chromium', 'Elements in iframe are not marked'); + + server.setRoute('/level-0.html', (req, res) => { + res.writeHead(200); + res.end(``); + }); + server.setRoute('/level-1.html', (req, res) => { + res.writeHead(200); + res.end(``); + }); + server.setRoute('/level-2.html', (req, res) => { + res.writeHead(200); + res.end(``); + }); + + const traceViewer = await runAndTrace(async () => { + await page.goto(server.PREFIX + '/level-0.html'); + await page.frameLocator('iframe').frameLocator('iframe').locator('button').click({ position: { x: 5, y: 5 } }); + }); + const snapshotFrame = await traceViewer.snapshotFrame('locator.click'); + await expect.soft(snapshotFrame.locator('x-pw-pointer')).not.toBeAttached(); + await expect.soft(snapshotFrame.frameLocator('iframe').locator('x-pw-pointer')).not.toBeAttached(); + await expect.soft(snapshotFrame.frameLocator('iframe').frameLocator('iframe').locator('x-pw-pointer')).toBeVisible(); +});