From 4b877213a1c9d0170891f43d156add5fc313f221 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 21 Mar 2022 18:51:48 -0700 Subject: [PATCH] fix(tracing): preserve control values without modifying DOM (#12939) Previously, we preserved input/textarea values by providing `value` attribute or text child. This produces DOM that does not actually match the original page. This change starts using special attributes to modify values directly when rendering. Same treatment is also applied to options in `select` and `checked` property of checkboxes and radio buttons. --- .../trace/recorder/snapshotterInjected.ts | 72 ++++++++++--------- .../src/web/traceViewer/snapshotRenderer.ts | 39 ++++++---- tests/trace-viewer/trace-viewer.spec.ts | 53 ++++++++++++++ 3 files changed, 114 insertions(+), 50 deletions(-) diff --git a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts index 928c0b3b07..2dbbb28ee0 100644 --- a/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts +++ b/packages/playwright-core/src/server/trace/recorder/snapshotterInjected.ts @@ -38,6 +38,9 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { // Attributes present in the snapshot. const kShadowAttribute = '__playwright_shadow_root_'; + const kValueAttribute = '__playwright_value_'; + const kCheckedAttribute = '__playwright_checked_'; + const kSelectedAttribute = '__playwright_selected_'; const kScrollTopAttribute = '__playwright_scroll_top_'; const kScrollLeftAttribute = '__playwright_scroll_left_'; const kStyleSheetAttribute = '__playwright_style_sheet_'; @@ -363,15 +366,23 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { if (nodeType === Node.ELEMENT_NODE) { const element = node as Element; - if (nodeName === 'INPUT') { + if (nodeName === 'INPUT' || nodeName === 'TEXTAREA') { const value = (element as HTMLInputElement).value; - expectValue('value'); + expectValue(kValueAttribute); expectValue(value); - attrs['value'] = value; - if ((element as HTMLInputElement).checked) { - expectValue('checked'); - attrs['checked'] = ''; - } + attrs[kValueAttribute] = value; + } + if (nodeName === 'INPUT' && ['checkbox', 'radio'].includes((element as HTMLInputElement).type)) { + const value = (element as HTMLInputElement).checked ? 'true' : 'false'; + expectValue(kCheckedAttribute); + expectValue(value); + attrs[kCheckedAttribute] = value; + } + if (nodeName === 'OPTION') { + const value = (element as HTMLOptionElement).selected ? 'true' : 'false'; + expectValue(kSelectedAttribute); + expectValue(value); + attrs[kSelectedAttribute] = value; } if (element.scrollTop) { expectValue(kScrollTopAttribute); @@ -390,33 +401,26 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { } } - if (nodeName === 'TEXTAREA') { - const value = (node as HTMLTextAreaElement).value; - expectValue(value); - extraNodes++; // Compensate for the extra text node. - result.push(value); - } else { - if (nodeName === 'HEAD') { - ++headNesting; - // Insert fake first, to ensure all elements use the proper base uri. - this._fakeBase.setAttribute('href', document.baseURI); - visitChild(this._fakeBase); - } - for (let child = node.firstChild; child; child = child.nextSibling) - visitChild(child); - if (nodeName === 'HEAD') - --headNesting; + if (nodeName === 'HEAD') { + ++headNesting; + // Insert fake first, to ensure all elements use the proper base uri. + this._fakeBase.setAttribute('href', document.baseURI); + visitChild(this._fakeBase); + } + for (let child = node.firstChild; child; child = child.nextSibling) + visitChild(child); + if (nodeName === 'HEAD') + --headNesting; + expectValue(kEndOfList); + let documentOrShadowRoot = null; + if (node.ownerDocument!.documentElement === node) + documentOrShadowRoot = node.ownerDocument; + else if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) + documentOrShadowRoot = node; + if (documentOrShadowRoot) { + for (const sheet of (documentOrShadowRoot as any).adoptedStyleSheets || []) + visitChildStyleSheet(sheet); expectValue(kEndOfList); - let documentOrShadowRoot = null; - if (node.ownerDocument!.documentElement === node) - documentOrShadowRoot = node.ownerDocument; - else if (node.nodeType === Node.DOCUMENT_FRAGMENT_NODE) - documentOrShadowRoot = node; - if (documentOrShadowRoot) { - for (const sheet of (documentOrShadowRoot as any).adoptedStyleSheets || []) - visitChildStyleSheet(sheet); - expectValue(kEndOfList); - } } // Process iframe src attribute before bailing out since it depends on a symbol, not the DOM. @@ -439,8 +443,6 @@ export function frameSnapshotStreamer(snapshotStreamer: string) { const element = node as Element; for (let i = 0; i < element.attributes.length; i++) { const name = element.attributes[i].name; - if (name === 'value' && (nodeName === 'INPUT' || nodeName === 'TEXTAREA')) - continue; if (nodeName === 'LINK' && name === 'integrity') continue; if (nodeName === 'IFRAME' && (name === 'src' || name === 'sandbox')) diff --git a/packages/playwright-core/src/web/traceViewer/snapshotRenderer.ts b/packages/playwright-core/src/web/traceViewer/snapshotRenderer.ts index 3d3f1bf64d..b3fef09480 100644 --- a/packages/playwright-core/src/web/traceViewer/snapshotRenderer.ts +++ b/packages/playwright-core/src/web/traceViewer/snapshotRenderer.ts @@ -173,17 +173,30 @@ function snapshotNodes(snapshot: FrameSnapshot): NodeSnapshot[] { } function snapshotScript() { - function applyPlaywrightAttributes(shadowAttribute: string, scrollTopAttribute: string, scrollLeftAttribute: string, styleSheetAttribute: string) { + function applyPlaywrightAttributes() { const scrollTops: Element[] = []; const scrollLefts: Element[] = []; const visit = (root: Document | ShadowRoot) => { // Collect all scrolled elements for later use. - for (const e of root.querySelectorAll(`[${scrollTopAttribute}]`)) + for (const e of root.querySelectorAll(`[__playwright_scroll_top_]`)) scrollTops.push(e); - for (const e of root.querySelectorAll(`[${scrollLeftAttribute}]`)) + for (const e of root.querySelectorAll(`[__playwright_scroll_left_]`)) scrollLefts.push(e); + for (const element of root.querySelectorAll(`[__playwright_value_]`)) { + (element as HTMLInputElement | HTMLTextAreaElement).value = element.getAttribute('__playwright_value_')!; + element.removeAttribute('__playwright_value_'); + } + for (const element of root.querySelectorAll(`[__playwright_checked_]`)) { + (element as HTMLInputElement).checked = element.getAttribute('__playwright_checked_') === 'true'; + element.removeAttribute('__playwright_checked_'); + } + for (const element of root.querySelectorAll(`[__playwright_selected_]`)) { + (element as HTMLOptionElement).selected = element.getAttribute('__playwright_selected_') === 'true'; + element.removeAttribute('__playwright_selected_'); + } + for (const iframe of root.querySelectorAll('iframe, frame')) { const src = iframe.getAttribute('__playwright_src__'); if (!src) { @@ -202,7 +215,7 @@ function snapshotScript() { } } - for (const element of root.querySelectorAll(`template[${shadowAttribute}]`)) { + for (const element of root.querySelectorAll(`template[__playwright_shadow_root_]`)) { const template = element as HTMLTemplateElement; const shadowRoot = template.parentElement!.attachShadow({ mode: 'open' }); shadowRoot.appendChild(template.content); @@ -212,10 +225,10 @@ function snapshotScript() { if ('adoptedStyleSheets' in (root as any)) { const adoptedSheets: CSSStyleSheet[] = [...(root as any).adoptedStyleSheets]; - for (const element of root.querySelectorAll(`template[${styleSheetAttribute}]`)) { + for (const element of root.querySelectorAll(`template[__playwright_style_sheet_]`)) { const template = element as HTMLTemplateElement; const sheet = new CSSStyleSheet(); - (sheet as any).replaceSync(template.getAttribute(styleSheetAttribute)); + (sheet as any).replaceSync(template.getAttribute('__playwright_style_sheet_')); adoptedSheets.push(sheet); } (root as any).adoptedStyleSheets = adoptedSheets; @@ -225,12 +238,12 @@ function snapshotScript() { const onLoad = () => { window.removeEventListener('load', onLoad); for (const element of scrollTops) { - element.scrollTop = +element.getAttribute(scrollTopAttribute)!; - element.removeAttribute(scrollTopAttribute); + element.scrollTop = +element.getAttribute('__playwright_scroll_top_')!; + element.removeAttribute('__playwright_scroll_top_'); } for (const element of scrollLefts) { - element.scrollLeft = +element.getAttribute(scrollLeftAttribute)!; - element.removeAttribute(scrollLeftAttribute); + element.scrollLeft = +element.getAttribute('__playwright_scroll_left_')!; + element.removeAttribute('__playwright_scroll_left_'); } const search = new URL(window.location.href).searchParams; @@ -258,9 +271,5 @@ function snapshotScript() { window.addEventListener('DOMContentLoaded', onDOMContentLoaded); } - const kShadowAttribute = '__playwright_shadow_root_'; - const kScrollTopAttribute = '__playwright_scroll_top_'; - const kScrollLeftAttribute = '__playwright_scroll_left_'; - const kStyleSheetAttribute = '__playwright_style_sheet_'; - return `\n(${applyPlaywrightAttributes.toString()})('${kShadowAttribute}', '${kScrollTopAttribute}', '${kScrollLeftAttribute}', '${kStyleSheetAttribute}')`; + return `\n(${applyPlaywrightAttributes.toString()})()`; } diff --git a/tests/trace-viewer/trace-viewer.spec.ts b/tests/trace-viewer/trace-viewer.spec.ts index 3f5421e09f..a2741bcb36 100644 --- a/tests/trace-viewer/trace-viewer.spec.ts +++ b/tests/trace-viewer/trace-viewer.spec.ts @@ -484,6 +484,59 @@ test('should restore scroll positions', async ({ page, runAndTrace, browserName expect(await div.evaluate(div => div.scrollTop)).toBe(136); }); +test('should restore control values', async ({ page, runAndTrace }) => { + const traceViewer = await runAndTrace(async () => { + await page.setContent(` + + + + + + + `); + await page.click('input'); + }); + + // Render snapshot, check expectations. + const frame = await traceViewer.snapshotFrame('page.click'); + + const text = frame.locator('[type=text]'); + await expect(text).toHaveAttribute('value', 'old'); + await expect(text).toHaveValue('hi'); + + const checkbox = frame.locator('[type=checkbox]'); + await expect(checkbox).not.toBeChecked(); + expect(await checkbox.evaluate(c => c.hasAttribute('checked'))).toBe(true); + + const radio = frame.locator('[type=radio]'); + await expect(radio).toBeChecked(); + expect(await radio.evaluate(c => c.hasAttribute('checked'))).toBe(false); + + const textarea = frame.locator('textarea'); + await expect(textarea).toHaveText('old'); + await expect(textarea).toHaveValue('hello'); + + expect(await frame.$eval('option >> nth=0', o => o.hasAttribute('selected'))).toBe(false); + expect(await frame.$eval('option >> nth=1', o => o.hasAttribute('selected'))).toBe(true); + expect(await frame.$eval('option >> nth=2', o => o.hasAttribute('selected'))).toBe(false); + expect(await frame.locator('select').evaluate(s => { + const options = [...(s as HTMLSelectElement).selectedOptions]; + return options.map(option => option.value); + })).toEqual(['opt1', 'opt3']); +}); + test('should work with meta CSP', async ({ page, runAndTrace, browserName }) => { const traceViewer = await runAndTrace(async () => { await page.setContent(`