chore: do not use attributes for trace target annotation (#22075)

Fixes: https://github.com/microsoft/playwright/issues/22004
This commit is contained in:
Pavel Feldman 2023-03-29 23:17:17 -07:00 committed by GitHub
parent a4f67c64e3
commit 968abd27d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 66 additions and 25 deletions

View File

@ -78,7 +78,6 @@ export class InjectedScript {
readonly isUnderTest: boolean;
private _sdkLanguage: Language;
private _testIdAttributeNameForStrictErrorAndConsoleCodegen: string = 'data-testid';
private _markedTargetElements = new Set<Element>();
// eslint-disable-next-line no-restricted-globals
readonly window: Window & typeof globalThis;
readonly document: Document;
@ -1018,7 +1017,7 @@ export class InjectedScript {
const attrs = [];
for (let i = 0; i < element.attributes.length; i++) {
const { name, value } = element.attributes[i];
if (name === 'style' || name.startsWith('__playwright'))
if (name === 'style')
continue;
if (!value && booleanAttributes.has(name))
attrs.push(` ${name}`);
@ -1096,15 +1095,14 @@ export class InjectedScript {
}
markTargetElements(markedElements: Set<Element>, callId: string) {
for (const e of this._markedTargetElements) {
if (!markedElements.has(e))
e.removeAttribute('__playwright_target__');
}
for (const e of markedElements) {
if (!this._markedTargetElements.has(e))
e.setAttribute('__playwright_target__', callId);
}
this._markedTargetElements = markedElements;
const customEvent = new CustomEvent('__playwright_target__', {
bubbles: true,
cancelable: true,
detail: callId,
composed: false,
});
for (const element of markedElements)
element.dispatchEvent(customEvent);
}
private _setupGlobalListenersRemovalDetection() {

View File

@ -110,7 +110,13 @@ export class Snapshotter {
// In a best-effort manner, without waiting for it, mark target element.
element?.callFunctionNoReply((element: Element, callId: string) => {
element.setAttribute('__playwright_target__', callId);
const customEvent = new CustomEvent('__playwright_target__', {
bubbles: true,
cancelable: true,
detail: callId,
composed: false,
});
element.dispatchEvent(customEvent);
}, callId);
// In each frame, in a non-stalling manner, capture the snapshots.

View File

@ -102,6 +102,42 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
this._observer = new MutationObserver(list => this._handleMutations(list));
const observerConfig = { attributes: true, subtree: true };
this._observer.observe(document, observerConfig);
this._refreshListenersWhenNeeded();
}
private _refreshListenersWhenNeeded() {
this._refreshListeners();
const customEventName = '__playwright_snapshotter_global_listeners_check__';
let seenEvent = false;
const handleCustomEvent = () => seenEvent = true;
window.addEventListener(customEventName, handleCustomEvent);
const observer = new MutationObserver(entries => {
// Check for new documentElement in case we need to reinstall document listeners.
const newDocumentElement = entries.some(entry => Array.from(entry.addedNodes).includes(document.documentElement));
if (newDocumentElement) {
// New documentElement - let's check whether listeners are still here.
seenEvent = false;
window.dispatchEvent(new CustomEvent(customEventName));
if (!seenEvent) {
// Listener did not fire. Reattach the listener and notify.
window.addEventListener(customEventName, handleCustomEvent);
this._refreshListeners();
}
}
});
observer.observe(document, { childList: true });
}
private _refreshListeners() {
(document as any).addEventListener('__playwright_target__', (event: CustomEvent) => {
if (!event.detail)
return;
const callId = event.detail as string;
(event.target as any).__playwright_target__ = callId;
});
}
private _interceptNativeMethod(obj: any, method: string, cb: (thisObj: any, result: any) => void) {
@ -283,7 +319,7 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
// TODO: remove after chromium is fixed?
const elementsToRestoreScrollPosition = new Set<Node>();
const findElementsToRestoreScrollPositionRecursively = (element: Element) => {
let shouldAdd = element.hasAttribute(kTargetAttribute);
let shouldAdd = '__playwright_target__' in element;
for (let child = element.firstElementChild; child; child = child.nextElementSibling)
shouldAdd = shouldAdd || findElementsToRestoreScrollPositionRecursively(child);
if (element.shadowRoot) {
@ -423,6 +459,11 @@ export function frameSnapshotStreamer(snapshotStreamer: string) {
visitChild(element.shadowRoot);
--shadowDomNesting;
}
if ('__playwright_target__' in element) {
expectValue(kTargetAttribute);
expectValue(element['__playwright_target__']);
attrs[kTargetAttribute] = element['__playwright_target__'] as string;
}
}
if (nodeName === 'HEAD') {

View File

@ -212,7 +212,7 @@ test.describe('cli codegen', () => {
expect(locator).toBe(`getByText('Some long text here')`);
const divContents = await page.$eval('div', div => div.outerHTML);
expect(divContents.replace(/\s__playwright_target__="[^"]+"/, '')).toBe(`<div onclick="console.log('click')"> Some long text here </div>`);
expect(divContents).toBe(`<div onclick="console.log('click')"> Some long text here </div>`);
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),

View File

@ -115,15 +115,15 @@ it('should support has:locator', async ({ page, trace }) => {
await expect(page.locator(`div`, {
has: page.locator(`text=world`)
})).toHaveCount(1);
expect(removeHighlight(await page.locator(`div`, {
expect(await page.locator(`div`, {
has: page.locator(`text=world`)
}).evaluate(e => e.outerHTML))).toBe(`<div><span>world</span></div>`);
}).evaluate(e => e.outerHTML)).toBe(`<div><span>world</span></div>`);
await expect(page.locator(`div`, {
has: page.locator(`text="hello"`)
})).toHaveCount(1);
expect(removeHighlight(await page.locator(`div`, {
expect(await page.locator(`div`, {
has: page.locator(`text="hello"`)
}).evaluate(e => e.outerHTML))).toBe(`<div><span>hello</span></div>`);
}).evaluate(e => e.outerHTML)).toBe(`<div><span>hello</span></div>`);
await expect(page.locator(`div`, {
has: page.locator(`xpath=./span`)
})).toHaveCount(2);
@ -133,9 +133,9 @@ it('should support has:locator', async ({ page, trace }) => {
await expect(page.locator(`div`, {
has: page.locator(`span`, { hasText: 'wor' })
})).toHaveCount(1);
expect(removeHighlight(await page.locator(`div`, {
expect(await page.locator(`div`, {
has: page.locator(`span`, { hasText: 'wor' })
}).evaluate(e => e.outerHTML))).toBe(`<div><span>world</span></div>`);
}).evaluate(e => e.outerHTML)).toBe(`<div><span>world</span></div>`);
await expect(page.locator(`div`, {
has: page.locator(`span`),
hasText: 'wor',
@ -205,7 +205,3 @@ it('alias methods coverage', async ({ page }) => {
await expect(page.locator('div').getByRole('button')).toHaveCount(1);
await expect(page.mainFrame().locator('button')).toHaveCount(1);
});
function removeHighlight(markup: string) {
return markup.replace(/\s__playwright_target__="[^"]+"/, '');
}

View File

@ -405,7 +405,7 @@ it('should absolutize relative selectors', async ({ page, server }) => {
await page.setContent(`<div><span>Hi</span></div>`);
expect(await page.$eval('div >> >span', e => e.textContent)).toBe('Hi');
expect(await page.locator('div').locator('>span').textContent()).toBe('Hi');
expect((await page.$eval('div:has(> span)', e => e.outerHTML)).replace(/\s__playwright_target__="[^"]+"/, '')).toBe('<div><span>Hi</span></div>');
expect(await page.$eval('div:has(> span)', e => e.outerHTML)).toBe('<div><span>Hi</span></div>');
expect(await page.$('div:has(> div)')).toBe(null);
});