Skip to content

Commit 206a877

Browse files
pavelfeldmanpavelfeldman
authored
fix(trace-viewer): multiple iframe and UX fixes (microsoft#10486)
1 parent 5a8010c commit 206a877

File tree

7 files changed

+34
-17
lines changed

7 files changed

+34
-17
lines changed

Diff for: ‎packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts

+6
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,12 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
265265
return;
266266
if (nodeName === 'SCRIPT')
267267
return;
268+
// Don't preload resources.
269+
if (nodeName === 'LINK' && nodeType === Node.ELEMENT_NODE) {
270+
const rel = (node as Element).getAttribute('rel')?.toLowerCase();
271+
if (rel === 'preload' || rel === 'prefetch')
272+
return;
273+
}
268274
if (this._removeNoScript && nodeName === 'NOSCRIPT')
269275
return;
270276
if (nodeName === 'META' && (node as HTMLMetaElement).httpEquiv.toLowerCase() === 'content-security-policy')

Diff for: ‎packages/playwright-core/src/web/traceViewer/snapshotRenderer.ts

+13-4
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,12 @@ export class SnapshotRenderer {
5959
// Element node.
6060
const builder: string[] = [];
6161
builder.push('<', n[0]);
62-
for (const [attr, value] of Object.entries(n[1] || {}))
63-
builder.push(' ', attr, '="', escapeAttribute(value as string), '"');
62+
// Never set relative URLs as <iframe src> - they start fetching frames immediately.
63+
const isFrame = n[0] === 'IFRAME' || n[0] === 'FRAME';
64+
for (const [attr, value] of Object.entries(n[1] || {})) {
65+
const attrToSet = isFrame && attr.toLowerCase() === 'src' ? '__playwright_src__' : attr;
66+
builder.push(' ', attrToSet, '="', escapeAttribute(value as string), '"');
67+
}
6468
builder.push('>');
6569
for (let i = 2; i < n.length; i++)
6670
builder.push(visit(n[i], snapshotIndex));
@@ -181,14 +185,19 @@ function snapshotScript() {
181185
scrollLefts.push(e);
182186

183187
for (const iframe of root.querySelectorAll('iframe')) {
184-
const src = iframe.getAttribute('src');
188+
const src = iframe.getAttribute('__playwright_src__');
185189
if (!src) {
186190
iframe.setAttribute('src', 'data:text/html,<body style="background: #ddd"></body>');
187191
} else {
188192
// Append query parameters to inherit ?name= or ?time= values from parent.
189-
const url = new URL('/trace' + src + window.location.search, window.location.href);
193+
const url = new URL(window.location.href);
190194
url.searchParams.delete('pointX');
191195
url.searchParams.delete('pointY');
196+
// We can be loading iframe from within iframe, reset base to be absolute.
197+
const index = url.pathname.lastIndexOf('/snapshot/');
198+
if (index !== -1)
199+
url.pathname = url.pathname.substring(0, index + 1);
200+
url.pathname += src.substring(1);
192201
iframe.setAttribute('src', url.toString());
193202
}
194203
}

Diff for: ‎packages/playwright-core/src/web/traceViewer/snapshotServer.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ export class SnapshotServer {
8888
headers.delete('Content-Length');
8989
headers.set('Content-Length', String(content.size));
9090
headers.set('Cache-Control', 'public, max-age=31536000');
91-
return new Response(content, {
91+
const { status } = resource.response;
92+
const isNullBodyStatus = status === 101 || status === 204 || status === 205 || status === 304;
93+
return new Response(isNullBodyStatus ? null : content, {
9294
headers,
9395
status: resource.response.status,
9496
statusText: resource.response.statusText,

Diff for: ‎packages/playwright-core/src/web/traceViewer/ui/snapshotTab.css

+4-2
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,11 @@
7070
padding: 4px;
7171
border-radius: 3px;
7272
height: 28px;
73-
display: flex;
74-
align-items: center;
73+
white-space: nowrap;
74+
text-overflow: ellipsis;
7575
overflow: hidden;
76+
display: block;
77+
flex: none;
7678
}
7779

7880
.snapshot-container {

Diff for: ‎packages/playwright-core/src/web/traceViewer/ui/snapshotTab.tsx

+6-7
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
* limitations under the License.
1515
*/
1616

17-
import { Size } from '../geometry';
1817
import './snapshotTab.css';
1918
import './tabbedPane.css';
2019
import * as React from 'react';
@@ -23,8 +22,7 @@ import { ActionTraceEvent } from '../../../server/trace/common/traceEvents';
2322

2423
export const SnapshotTab: React.FunctionComponent<{
2524
action: ActionTraceEvent | undefined,
26-
defaultSnapshotInfo: { viewport: Size, url: string },
27-
}> = ({ action, defaultSnapshotInfo }) => {
25+
}> = ({ action }) => {
2826
const [measure, ref] = useMeasure<HTMLDivElement>();
2927
const [snapshotIndex, setSnapshotIndex] = React.useState(0);
3028

@@ -57,7 +55,7 @@ export const SnapshotTab: React.FunctionComponent<{
5755
}, [snapshotIndex, snapshots]);
5856

5957
const iframeRef = React.useRef<HTMLIFrameElement>(null);
60-
const [snapshotInfo, setSnapshotInfo] = React.useState(defaultSnapshotInfo);
58+
const [snapshotInfo, setSnapshotInfo] = React.useState({ viewport: kDefaultViewport, url: '' });
6159
React.useEffect(() => {
6260
(async () => {
6361
if (snapshotInfoUrl) {
@@ -66,8 +64,7 @@ export const SnapshotTab: React.FunctionComponent<{
6664
if (!info.error)
6765
setSnapshotInfo(info);
6866
} else {
69-
// Reset to default if snapshotInfoUrl was removed
70-
setSnapshotInfo(defaultSnapshotInfo);
67+
setSnapshotInfo({ viewport: kDefaultViewport, url: '' });
7168
}
7269
if (!iframeRef.current)
7370
return;
@@ -76,7 +73,7 @@ export const SnapshotTab: React.FunctionComponent<{
7673
} catch (e) {
7774
}
7875
})();
79-
}, [iframeRef, snapshotUrl, snapshotInfoUrl, pointX, pointY, defaultSnapshotInfo]);
76+
}, [iframeRef, snapshotUrl, snapshotInfoUrl, pointX, pointY]);
8077

8178
const snapshotSize = snapshotInfo.viewport;
8279
const scale = Math.min(measure.width / snapshotSize.width, measure.height / snapshotSize.height, 1);
@@ -124,3 +121,5 @@ function renderTitle(snapshotTitle: string): string {
124121
return 'Action';
125122
return snapshotTitle;
126123
}
124+
125+
const kDefaultViewport = { width: 1280, height: 720 };

Diff for: ‎packages/playwright-core/src/web/traceViewer/ui/workbench.tsx

+1-2
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,6 @@ export const Workbench: React.FunctionComponent<{
115115
})();
116116
}, [traceURL, uploadedTraceName]);
117117

118-
const defaultSnapshotInfo = { viewport: contextEntry.options.viewport || { width: 1280, height: 720 }, url: '' };
119118
const boundaries = { minimum: contextEntry.startTime, maximum: contextEntry.endTime };
120119

121120

@@ -153,7 +152,7 @@ export const Workbench: React.FunctionComponent<{
153152
</div>
154153
<SplitView sidebarSize={300} orientation='horizontal' sidebarIsFirst={true}>
155154
<SplitView sidebarSize={300} orientation='horizontal'>
156-
<SnapshotTab action={selectedAction} defaultSnapshotInfo={defaultSnapshotInfo} />
155+
<SnapshotTab action={selectedAction} />
157156
<TabbedPane tabs={tabs} selectedTab={selectedPropertiesTab} setSelectedTab={setSelectedPropertiesTab}/>
158157
</SplitView>
159158
<TabbedPane tabs={

Diff for: ‎tests/snapshotter.spec.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ it.describe('snapshots', () => {
142142
for (; ; ++counter) {
143143
snapshot = await snapshotter.captureSnapshot(toImpl(page), 'snapshot' + counter);
144144
const text = distillSnapshot(snapshot).replace(/frame@[^"]+["]/, '<id>"');
145-
if (text === '<IFRAME src=\"/snapshot/<id>\"></IFRAME>')
145+
if (text === '<IFRAME __playwright_src__=\"/snapshot/<id>\"></IFRAME>')
146146
break;
147147
await page.waitForTimeout(250);
148148
}

0 commit comments

Comments
 (0)