Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: ignore untrusted clicks when recording #18796

Merged
merged 1 commit into from
Nov 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions packages/playwright-core/src/server/injected/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,10 @@ class Recorder {
addEventListener(document, 'mouseup', event => this._onMouseUp(event as MouseEvent), true),
addEventListener(document, 'mousemove', event => this._onMouseMove(event as MouseEvent), true),
addEventListener(document, 'mouseleave', event => this._onMouseLeave(event as MouseEvent), true),
addEventListener(document, 'focus', () => this._onFocus(true), true),
addEventListener(document, 'scroll', () => {
addEventListener(document, 'focus', event => event.isTrusted && this._onFocus(true), true),
addEventListener(document, 'scroll', event => {
if (!event.isTrusted)
return;
this._hoveredModel = null;
this._highlight.hideActionPoint();
this._updateHighlight();
Expand Down Expand Up @@ -156,6 +158,8 @@ class Recorder {
}

private _onClick(event: MouseEvent) {
if (!event.isTrusted)
return;
if (this._mode === 'inspecting')
globalThis.__pw_recorderSetSelector(this._hoveredModel ? this._hoveredModel.selector : '');
if (this._shouldIgnoreMouseEvent(event))
Expand Down Expand Up @@ -204,6 +208,8 @@ class Recorder {
}

private _onMouseDown(event: MouseEvent) {
if (!event.isTrusted)
return;
if (this._shouldIgnoreMouseEvent(event))
return;
if (!this._performingAction)
Expand All @@ -212,13 +218,17 @@ class Recorder {
}

private _onMouseUp(event: MouseEvent) {
if (!event.isTrusted)
return;
if (this._shouldIgnoreMouseEvent(event))
return;
if (!this._performingAction)
consumeEvent(event);
}

private _onMouseMove(event: MouseEvent) {
if (!event.isTrusted)
return;
if (this._mode === 'none')
return;
const target = this._deepEventTarget(event);
Expand All @@ -229,6 +239,8 @@ class Recorder {
}

private _onMouseLeave(event: MouseEvent) {
if (!event.isTrusted)
return;
// Leaving iframe.
if (window.top !== window && this._deepEventTarget(event).nodeType === Node.DOCUMENT_NODE) {
this._hoveredElement = null;
Expand Down Expand Up @@ -339,6 +351,8 @@ class Recorder {
}

private _onKeyDown(event: KeyboardEvent) {
if (!event.isTrusted)
return;
if (this._mode === 'inspecting') {
consumeEvent(event);
return;
Expand Down Expand Up @@ -376,6 +390,8 @@ class Recorder {
}

private _onKeyUp(event: KeyboardEvent) {
if (!event.isTrusted)
return;
if (this._mode === 'none')
return;
if (!this._shouldGenerateKeyPressFor(event))
Expand Down
44 changes: 31 additions & 13 deletions tests/library/inspector/cli-codegen-1.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('button', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand All @@ -52,6 +52,27 @@ test.describe('cli codegen', () => {
expect(message.text()).toBe('click');
});


test('should ignore programmatic events', async ({ page, openRecorder }) => {
const recorder = await openRecorder();

await recorder.setContentAndWait(`<button onclick="console.log('click')">Submit</button>`);

const locator = await recorder.hoverOverElement('button');
expect(locator).toBe(`getByRole('button', { name: 'Submit' })`);

await page.dispatchEvent('button', 'click', { detail: 1 });

await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
recorder.trustedClick()
]);

const clicks = recorder.sources().get('Playwright Test').actions.filter(l => l.includes('Submit'));
expect(clicks.length).toBe(1);
});

test('should click after same-document navigation', async ({ page, openRecorder, server }) => {
const recorder = await openRecorder();

Expand All @@ -74,7 +95,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('button', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect(sources.get('JavaScript').text).toContain(`
Expand All @@ -95,16 +116,14 @@ test.describe('cli codegen', () => {
</script>
`);

const locator = await recorder.waitForHighlight(() => recorder.page.hover('canvas', {
const locator = await recorder.hoverOverElement('canvas', {
position: { x: 250, y: 250 },
}));
});
expect(locator).toBe(`locator('canvas')`);
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
recorder.page.click('canvas', {
position: { x: 250, y: 250 },
})
recorder.trustedClick(),
]);

expect(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -154,7 +173,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('button', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -182,13 +201,12 @@ test.describe('cli codegen', () => {

// Force highlight.
await recorder.hoverOverElement('span');

// Append text after highlight.
await page.evaluate(() => {
const div = document.createElement('div');
div.setAttribute('onclick', "console.log('click')");
div.textContent = ' Some long text here ';
document.documentElement.appendChild(div);
document.body.appendChild(div);
});

const locator = await recorder.hoverOverElement('div');
Expand All @@ -200,7 +218,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('div', 'click', { detail: 1 })
recorder.trustedMove('div').then(() => recorder.trustedClick()),
]);
expect(sources.get('JavaScript').text).toContain(`
await page.getByText('Some long text here').click();`);
Expand Down Expand Up @@ -585,7 +603,7 @@ test.describe('cli codegen', () => {
const [popup, sources] = await Promise.all([
page.context().waitForEvent('page'),
recorder.waitForOutput('JavaScript', 'waitForEvent'),
page.dispatchEvent('a', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -628,7 +646,7 @@ test.describe('cli codegen', () => {
const [, sources] = await Promise.all([
page.waitForNavigation(),
recorder.waitForOutput('JavaScript', '.click()'),
page.dispatchEvent('a', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down
14 changes: 7 additions & 7 deletions tests/library/inspector/cli-codegen-3.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('button', 'click', { detail: 1 })
recorder.trustedClick()
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -68,7 +68,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('button', 'click', { detail: 1 })
recorder.trustedClick()
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -240,7 +240,7 @@ test.describe('cli codegen', () => {
const [message, sources] = await Promise.all([
page.waitForEvent('console', msg => msg.type() !== 'error'),
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('div', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -271,7 +271,7 @@ test.describe('cli codegen', () => {

const [sources] = await Promise.all([
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('input', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -300,7 +300,7 @@ test.describe('cli codegen', () => {

const [sources] = await Promise.all([
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('input', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -329,7 +329,7 @@ test.describe('cli codegen', () => {

const [sources] = await Promise.all([
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('input', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down Expand Up @@ -358,7 +358,7 @@ test.describe('cli codegen', () => {

const [sources] = await Promise.all([
recorder.waitForOutput('JavaScript', 'click'),
page.dispatchEvent('input', 'click', { detail: 1 })
recorder.trustedClick(),
]);

expect.soft(sources.get('JavaScript').text).toContain(`
Expand Down
18 changes: 16 additions & 2 deletions tests/library/inspector/inspectorTest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,22 @@ class Recorder {
return new Promise(f => callback = f);
}

async hoverOverElement(selector: string): Promise<string> {
return this.waitForHighlight(() => this.page.dispatchEvent(selector, 'mousemove', { detail: 1 }));
async hoverOverElement(selector: string, options?: { position?: { x: number, y: number }}): Promise<string> {
return this.waitForHighlight(async () => {
const box = await this.page.locator(selector).first().boundingBox();
const offset = options?.position || { x: box.width / 2, y: box.height / 2 };
await this.page.mouse.move(box.x + offset.x, box.y + offset.y);
});
}

async trustedMove(selector: string) {
const box = await this.page.locator(selector).first().boundingBox();
await this.page.mouse.move(box.x + box.width / 2, box.y + box.height / 2);
}

async trustedClick() {
await this.page.mouse.down();
await this.page.mouse.up();
}

async focusElement(selector: string): Promise<string> {
Expand Down