fix(trace viewer): retain currentSrc of all images (#26841)

When `<source>` or `srcset=` are involved, the actual image src is
determinted at runtime based on factors like `devicePixelRatio` and
media queries that depend on width/height.

Since these factors may differ in the Trace Viewer itself, we should
preserve the `currentSrc`, use it as an actual `src`, and disable
various `<source>` and `srcset=`.
This commit is contained in:
Dmitry Gozman 2023-09-05 12:48:07 -07:00 committed by GitHub
parent 1fda6d1536
commit 740472ce8f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 61 additions and 14 deletions

View file

@ -46,6 +46,7 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
const kStyleSheetAttribute = '__playwright_style_sheet_';
const kTargetAttribute = '__playwright_target__';
const kCustomElementsAttribute = '__playwright_custom_elements__';
const kCurrentSrcAttribute = '__playwright_current_src__';
// Symbols for our own info on Nodes/StyleSheets.
const kSnapshotFrameId = Symbol('__playwright_snapshot_frameid_');
@ -485,6 +486,14 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
attrs[kCustomElementsAttribute] = value;
}
// Process currentSrc before bailing out since it depends on JS, not the DOM.
if (nodeName === 'IMG' || nodeName === 'PICTURE') {
const value = nodeName === 'PICTURE' ? '' : this._sanitizeUrl((node as HTMLImageElement).currentSrc);
expectValue(kCurrentSrcAttribute);
expectValue(value);
attrs[kCurrentSrcAttribute] = value;
}
// We can skip attributes comparison because nothing else has changed,
// and mutation observer didn't tell us about the attributes.
if (equals && data.attributesCached && !shadowDomNesting)

View file

@ -42,7 +42,7 @@ export class SnapshotRenderer {
}
render(): RenderedFrameSnapshot {
const visit = (n: NodeSnapshot, snapshotIndex: number, parentTag: string | undefined): string => {
const visit = (n: NodeSnapshot, snapshotIndex: number, parentTag: string | undefined, parentAttrs: [string, string][] | undefined): string => {
// Text node.
if (typeof n === 'string') {
const text = escapeText(n);
@ -61,29 +61,45 @@ export class SnapshotRenderer {
const nodes = snapshotNodes(this._snapshots[referenceIndex]);
const nodeIndex = n[0][1];
if (nodeIndex >= 0 && nodeIndex < nodes.length)
(n as any)._string = visit(nodes[nodeIndex], referenceIndex, parentTag);
(n as any)._string = visit(nodes[nodeIndex], referenceIndex, parentTag, parentAttrs);
}
} else if (typeof n[0] === 'string') {
// Element node.
const builder: string[] = [];
builder.push('<', n[0]);
// Never set relative URLs as <iframe src> - they start fetching frames immediately.
const attrs = Object.entries(n[1] || {});
const kCurrentSrcAttribute = '__playwright_current_src__';
const isFrame = n[0] === 'IFRAME' || n[0] === 'FRAME';
const isAnchor = n[0] === 'A';
for (const [attr, value] of Object.entries(n[1] || {})) {
const attrName = isFrame && attr.toLowerCase() === 'src' ? '__playwright_src__' : attr;
let attrValue = value;
if (attr.toLowerCase() === 'href' || attr.toLowerCase() === 'src') {
if (isAnchor)
attrValue = 'link://' + value;
else
attrValue = rewriteURLForCustomProtocol(value);
const isImg = n[0] === 'IMG';
const isImgWithCurrentSrc = isImg && attrs.some(a => a[0] === kCurrentSrcAttribute);
const isSourceInsidePictureWithCurrentSrc = n[0] === 'SOURCE' && parentTag === 'PICTURE' && parentAttrs?.some(a => a[0] === kCurrentSrcAttribute);
for (const [attr, value] of attrs) {
let attrName = attr;
if (isFrame && attr.toLowerCase() === 'src') {
// Never set relative URLs as <iframe src> - they start fetching frames immediately.
attrName = '__playwright_src__';
}
builder.push(' ', attrName, '="', escapeAttribute(attrValue as string), '"');
if (isImg && attr === kCurrentSrcAttribute) {
// Render currentSrc for images, so that trace viewer does not accidentally
// resolve srcset to a different source.
attrName = 'src';
}
if (['src', 'srcset'].includes(attr.toLowerCase()) && (isImgWithCurrentSrc || isSourceInsidePictureWithCurrentSrc)) {
// Disable actual <img src>, <img srcset>, <source src> and <source srcset> if
// we will be using the currentSrc instead.
attrName = '_' + attrName;
}
let attrValue = value;
if (isAnchor && attr.toLowerCase() === 'href')
attrValue = 'link://' + value;
else if (attr.toLowerCase() === 'href' || attr.toLowerCase() === 'src' || attr === kCurrentSrcAttribute)
attrValue = rewriteURLForCustomProtocol(value);
builder.push(' ', attrName, '="', escapeAttribute(attrValue), '"');
}
builder.push('>');
for (let i = 2; i < n.length; i++)
builder.push(visit(n[i], snapshotIndex, n[0]));
builder.push(visit(n[i], snapshotIndex, n[0], attrs));
if (!autoClosing.has(n[0]))
builder.push('</', n[0], '>');
(n as any)._string = builder.join('');
@ -96,7 +112,7 @@ export class SnapshotRenderer {
};
const snapshot = this._snapshot;
let html = visit(snapshot.html, this._index, undefined);
let html = visit(snapshot.html, this._index, undefined, undefined);
if (!html)
return { html: '', pageId: snapshot.pageId, frameId: snapshot.frameId, index: this._index };

View file

@ -552,6 +552,28 @@ test('should handle src=blob', async ({ page, server, runAndTrace, browserName }
expect(size).toBe(10);
});
test('should preserve currentSrc', async ({ browser, server, showTraceViewer }) => {
const traceFile = test.info().outputPath('trace.zip');
const page = await browser.newPage({ deviceScaleFactor: 3 });
await page.context().tracing.start({ snapshots: true, screenshots: true, sources: true });
await page.setViewportSize({ width: 300, height: 300 });
await page.goto(server.EMPTY_PAGE);
await page.setContent(`
<picture>
<source srcset="digits/1.png 1x, digits/2.png 2x, digits/3.png 3x">
<img id=target1 src="digits/0.png">
</picture>
<img id=target2 srcset="digits/4.png 1x, digits/5.png 2x, digits/6.png 3x">
`);
await page.context().tracing.stop({ path: traceFile });
await page.close();
const traceViewer = await showTraceViewer([traceFile]);
const frame = await traceViewer.snapshotFrame('page.setContent');
await expect(frame.locator('#target1')).toHaveAttribute('src', server.PREFIX + '/digits/3.png');
await expect(frame.locator('#target2')).toHaveAttribute('src', server.PREFIX + '/digits/6.png');
});
test('should register custom elements', async ({ page, server, runAndTrace }) => {
const traceViewer = await runAndTrace(async () => {
await page.goto(server.EMPTY_PAGE);