fix(trace viewer): do not show multiple action points in iframes (#32537)

When action has an input target, we assume there is a target element in
one of the frames and show action point in its center.

Fixes #32453.
This commit is contained in:
Dmitry Gozman 2024-09-11 03:04:03 -07:00 committed by GitHub
parent a4bd551597
commit 7335fa602c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 39 additions and 6 deletions

View file

@ -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);
}
}
};

View file

@ -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 };

View file

@ -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(`<iframe src="/level-1.html" style="position: absolute; left: 100px"></iframe>`);
});
server.setRoute('/level-1.html', (req, res) => {
res.writeHead(200);
res.end(`<iframe src="/level-2.html"></iframe>`);
});
server.setRoute('/level-2.html', (req, res) => {
res.writeHead(200);
res.end(`<button>Click me</button>`);
});
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();
});