From d051495c7a95e2fdbd5c4b740efeca540310b5e3 Mon Sep 17 00:00:00 2001 From: Pavel Feldman Date: Thu, 12 Sep 2024 11:40:44 -0700 Subject: [PATCH] chore: perform double click while recording (#32576) --- .../src/server/codegen/csharp.ts | 4 +- .../src/server/codegen/java.ts | 4 +- .../src/server/codegen/javascript.ts | 4 +- .../src/server/codegen/language.ts | 3 +- .../src/server/codegen/python.ts | 4 +- .../src/server/injected/recorder/recorder.ts | 61 +++++++++++++++++++ .../src/server/recorder/recorderCollection.ts | 9 --- .../src/server/recorder/recorderRunner.ts | 18 +++++- tests/library/inspector/cli-codegen-1.spec.ts | 40 ++++++++++++ tests/library/inspector/inspectorTest.ts | 7 +++ 10 files changed, 135 insertions(+), 19 deletions(-) diff --git a/packages/playwright-core/src/server/codegen/csharp.ts b/packages/playwright-core/src/server/codegen/csharp.ts index f11435a0c2..2244a372fc 100644 --- a/packages/playwright-core/src/server/codegen/csharp.ts +++ b/packages/playwright-core/src/server/codegen/csharp.ts @@ -16,7 +16,7 @@ import type { BrowserContextOptions } from '../../../types/types'; import type { ActionInContext, Language, LanguageGenerator, LanguageGeneratorOptions } from './types'; -import { sanitizeDeviceOptions, toClickOptions, toKeyboardModifiers, toSignalMap } from './language'; +import { sanitizeDeviceOptions, toClickOptionsForSourceCode, toKeyboardModifiers, toSignalMap } from './language'; import { escapeWithQuotes, asLocator } from '../../utils'; import { deviceDescriptors } from '../deviceDescriptors'; @@ -112,7 +112,7 @@ export class CSharpLanguageGenerator implements LanguageGenerator { let method = 'Click'; if (action.clickCount === 2) method = 'DblClick'; - const options = toClickOptions(action); + const options = toClickOptionsForSourceCode(action); if (!Object.entries(options).length) return `await ${subject}.${this._asLocator(action.selector)}.${method}Async();`; const optionsString = formatObject(options, ' ', 'Locator' + method + 'Options'); diff --git a/packages/playwright-core/src/server/codegen/java.ts b/packages/playwright-core/src/server/codegen/java.ts index 47c6fa3619..3a640d36e2 100644 --- a/packages/playwright-core/src/server/codegen/java.ts +++ b/packages/playwright-core/src/server/codegen/java.ts @@ -17,7 +17,7 @@ import type { BrowserContextOptions } from '../../../types/types'; import type * as types from '../types'; import type { ActionInContext, Language, LanguageGenerator, LanguageGeneratorOptions } from './types'; -import { toClickOptions, toKeyboardModifiers, toSignalMap } from './language'; +import { toClickOptionsForSourceCode, toKeyboardModifiers, toSignalMap } from './language'; import { deviceDescriptors } from '../deviceDescriptors'; import { JavaScriptFormatter } from './javascript'; import { escapeWithQuotes, asLocator } from '../../utils'; @@ -101,7 +101,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { let method = 'click'; if (action.clickCount === 2) method = 'dblclick'; - const options = toClickOptions(action); + const options = toClickOptionsForSourceCode(action); const optionsText = formatClickOptions(options); return `${subject}.${this._asLocator(action.selector, inFrameLocator)}.${method}(${optionsText});`; } diff --git a/packages/playwright-core/src/server/codegen/javascript.ts b/packages/playwright-core/src/server/codegen/javascript.ts index 1c1ba3f1cb..c3ed05d4d4 100644 --- a/packages/playwright-core/src/server/codegen/javascript.ts +++ b/packages/playwright-core/src/server/codegen/javascript.ts @@ -16,7 +16,7 @@ import type { BrowserContextOptions } from '../../../types/types'; import type { ActionInContext, Language, LanguageGenerator, LanguageGeneratorOptions } from './types'; -import { sanitizeDeviceOptions, toSignalMap, toKeyboardModifiers, toClickOptions } from './language'; +import { sanitizeDeviceOptions, toSignalMap, toKeyboardModifiers, toClickOptionsForSourceCode } from './language'; import { deviceDescriptors } from '../deviceDescriptors'; import { escapeWithQuotes, asLocator } from '../../utils'; @@ -85,7 +85,7 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { let method = 'click'; if (action.clickCount === 2) method = 'dblclick'; - const options = toClickOptions(action); + const options = toClickOptionsForSourceCode(action); const optionsString = formatOptions(options, false); return `await ${subject}.${this._asLocator(action.selector)}.${method}(${optionsString});`; } diff --git a/packages/playwright-core/src/server/codegen/language.ts b/packages/playwright-core/src/server/codegen/language.ts index 72cfb9083d..3e0c8f71e5 100644 --- a/packages/playwright-core/src/server/codegen/language.ts +++ b/packages/playwright-core/src/server/codegen/language.ts @@ -69,13 +69,14 @@ export function toKeyboardModifiers(modifiers: number): types.SmartKeyboardModif return result; } -export function toClickOptions(action: actions.ClickAction): types.MouseClickOptions { +export function toClickOptionsForSourceCode(action: actions.ClickAction): types.MouseClickOptions { const modifiers = toKeyboardModifiers(action.modifiers); const options: types.MouseClickOptions = {}; if (action.button !== 'left') options.button = action.button; if (modifiers.length) options.modifiers = modifiers; + // Do not render clickCount === 2 for dblclick. if (action.clickCount > 2) options.clickCount = action.clickCount; if (action.position) diff --git a/packages/playwright-core/src/server/codegen/python.ts b/packages/playwright-core/src/server/codegen/python.ts index 6ed101bcf0..6c2b60dc70 100644 --- a/packages/playwright-core/src/server/codegen/python.ts +++ b/packages/playwright-core/src/server/codegen/python.ts @@ -16,7 +16,7 @@ import type { BrowserContextOptions } from '../../../types/types'; import type { ActionInContext, Language, LanguageGenerator, LanguageGeneratorOptions } from './types'; -import { sanitizeDeviceOptions, toSignalMap, toKeyboardModifiers, toClickOptions } from './language'; +import { sanitizeDeviceOptions, toSignalMap, toKeyboardModifiers, toClickOptionsForSourceCode } from './language'; import { escapeWithQuotes, toSnakeCase, asLocator } from '../../utils'; import { deviceDescriptors } from '../deviceDescriptors'; @@ -94,7 +94,7 @@ export class PythonLanguageGenerator implements LanguageGenerator { let method = 'click'; if (action.clickCount === 2) method = 'dblclick'; - const options = toClickOptions(action); + const options = toClickOptionsForSourceCode(action); const optionsString = formatOptions(options, false); return `${subject}.${this._asLocator(action.selector)}.${method}(${optionsString})`; } diff --git a/packages/playwright-core/src/server/injected/recorder/recorder.ts b/packages/playwright-core/src/server/injected/recorder/recorder.ts index 8cbf11964f..48639fefc8 100644 --- a/packages/playwright-core/src/server/injected/recorder/recorder.ts +++ b/packages/playwright-core/src/server/injected/recorder/recorder.ts @@ -36,6 +36,7 @@ interface RecorderTool { cursor(): string; cleanup?(): void; onClick?(event: MouseEvent): void; + onDblClick?(event: MouseEvent): void; onContextMenu?(event: MouseEvent): void; onDragStart?(event: DragEvent): void; onInput?(event: Event): void; @@ -210,6 +211,7 @@ class RecordActionTool implements RecorderTool { private _hoveredElement: HTMLElement | null = null; private _activeModel: HighlightModel | null = null; private _expectProgrammaticKeyUp = false; + private _pendingClickAction: { action: actions.ClickAction, timeout: NodeJS.Timeout } | undefined; constructor(recorder: Recorder) { this._recorder = recorder; @@ -252,6 +254,38 @@ class RecordActionTool implements RecorderTool { return; } + this._cancelPendingClickAction(); + + // Stall click in case we are observing double-click. + if (event.detail === 1) { + this._pendingClickAction = { + action: { + name: 'click', + selector: this._hoveredModel!.selector, + position: positionForEvent(event), + signals: [], + button: buttonForEvent(event), + modifiers: modifiersForEvent(event), + clickCount: event.detail + }, + timeout: setTimeout(() => this._commitPendingClickAction(), 200) + }; + } + } + + onDblClick(event: MouseEvent) { + if (isRangeInput(this._hoveredElement)) + return; + if (this._shouldIgnoreMouseEvent(event)) + return; + // Only allow double click dispatch while action is in progress. + if (this._actionInProgress(event)) + return; + if (this._consumedDueToNoModel(event, this._hoveredModel)) + return; + + this._cancelPendingClickAction(); + this._performAction({ name: 'click', selector: this._hoveredModel!.selector, @@ -263,6 +297,18 @@ class RecordActionTool implements RecorderTool { }); } + private _commitPendingClickAction() { + if (this._pendingClickAction) + this._performAction(this._pendingClickAction.action); + this._cancelPendingClickAction(); + } + + private _cancelPendingClickAction() { + if (this._pendingClickAction) + clearTimeout(this._pendingClickAction.timeout); + this._pendingClickAction = undefined; + } + onContextMenu(event: MouseEvent) { // the 'contextmenu' event is triggered by a right-click or equivalent action, // and it prevents the click event from firing for that action, so we always @@ -915,6 +961,10 @@ class Overlay { } return false; } + + onDblClick(event: MouseEvent) { + return false; + } } export class Recorder { @@ -970,6 +1020,7 @@ export class Recorder { this._listeners = [ addEventListener(this.document, 'click', event => this._onClick(event as MouseEvent), true), addEventListener(this.document, 'auxclick', event => this._onClick(event as MouseEvent), true), + addEventListener(this.document, 'dblclick', event => this._onDblClick(event as MouseEvent), true), addEventListener(this.document, 'contextmenu', event => this._onContextMenu(event as MouseEvent), true), addEventListener(this.document, 'dragstart', event => this._onDragStart(event as DragEvent), true), addEventListener(this.document, 'input', event => this._onInput(event), true), @@ -1043,6 +1094,16 @@ export class Recorder { this._currentTool.onClick?.(event); } + private _onDblClick(event: MouseEvent) { + if (!event.isTrusted) + return; + if (this.overlay?.onDblClick(event)) + return; + if (this._ignoreOverlayEvent(event)) + return; + this._currentTool.onDblClick?.(event); + } + private _onContextMenu(event: MouseEvent) { if (!event.isTrusted) return; diff --git a/packages/playwright-core/src/server/recorder/recorderCollection.ts b/packages/playwright-core/src/server/recorder/recorderCollection.ts index 29da778ffb..a98660af38 100644 --- a/packages/playwright-core/src/server/recorder/recorderCollection.ts +++ b/packages/playwright-core/src/server/recorder/recorderCollection.ts @@ -78,10 +78,6 @@ export class RecorderCollection extends EventEmitter { if (action.selector === lastAction.selector) eraseLastAction = true; } - if (lastAction && action.name === 'click' && lastAction.name === 'click') { - if (action.selector === lastAction.selector && action.clickCount > lastAction.clickCount) - eraseLastAction = true; - } if (lastAction && action.name === 'navigate' && lastAction.name === 'navigate') { if (action.url === lastAction.url) { // Already at a target URL. @@ -89,11 +85,6 @@ export class RecorderCollection extends EventEmitter { return; } } - // Check and uncheck erase click. - if (lastAction && (action.name === 'check' || action.name === 'uncheck') && lastAction.name === 'click') { - if (action.selector === lastAction.selector) - eraseLastAction = true; - } } this._lastAction = actionInContext; diff --git a/packages/playwright-core/src/server/recorder/recorderRunner.ts b/packages/playwright-core/src/server/recorder/recorderRunner.ts index b6bdfd1a72..d27d18d3e7 100644 --- a/packages/playwright-core/src/server/recorder/recorderRunner.ts +++ b/packages/playwright-core/src/server/recorder/recorderRunner.ts @@ -15,11 +15,13 @@ */ import { createGuid, monotonicTime, serializeExpectedTextValues } from '../../utils'; -import { toClickOptions, toKeyboardModifiers } from '../codegen/language'; +import { toKeyboardModifiers } from '../codegen/language'; import type { ActionInContext } from '../codegen/types'; import type { Frame } from '../frames'; import type { CallMetadata } from '../instrumentation'; import type { Page } from '../page'; +import type * as actions from './recorderActions'; +import type * as types from '../types'; import { buildFullSelector } from './recorderUtils'; async function innerPerformAction(mainFrame: Frame, action: string, params: any, cb: (callMetadata: CallMetadata) => Promise): Promise { @@ -126,3 +128,17 @@ export async function performAction(pageAliases: Map, actionInCont } throw new Error('Internal error: unexpected action ' + (action as any).name); } + +export function toClickOptions(action: actions.ClickAction): types.MouseClickOptions { + const modifiers = toKeyboardModifiers(action.modifiers); + const options: types.MouseClickOptions = {}; + if (action.button !== 'left') + options.button = action.button; + if (modifiers.length) + options.modifiers = modifiers; + if (action.clickCount > 1) + options.clickCount = action.clickCount; + if (action.position) + options.position = action.position; + return options; +} diff --git a/tests/library/inspector/cli-codegen-1.spec.ts b/tests/library/inspector/cli-codegen-1.spec.ts index a58a8a38b9..4320aac5e8 100644 --- a/tests/library/inspector/cli-codegen-1.spec.ts +++ b/tests/library/inspector/cli-codegen-1.spec.ts @@ -52,6 +52,46 @@ await page.GetByRole(AriaRole.Button, new() { Name = "Submit" }).ClickAsync();`) expect(message.text()).toBe('click'); }); + test('should double click', async ({ page, openRecorder }) => { + const recorder = await openRecorder(); + + await recorder.setContentAndWait(``); + + const locator = await recorder.hoverOverElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' })`); + + const messages: string[] = []; + page.on('console', message => { + if (message.text().includes('click')) + messages.push(message.text()); + }); + const [, sources] = await Promise.all([ + page.waitForEvent('console', msg => msg.type() !== 'error' && msg.text() === 'dblclick 2'), + recorder.waitForOutput('JavaScript', 'dblclick'), + recorder.trustedDblclick(), + ]); + + expect.soft(sources.get('JavaScript')!.text).toContain(` + await page.getByRole('button', { name: 'Submit' }).dblclick();`); + + expect.soft(sources.get('Python')!.text).toContain(` + page.get_by_role("button", name="Submit").dblclick()`); + + expect.soft(sources.get('Python Async')!.text).toContain(` + await page.get_by_role("button", name="Submit").dblclick()`); + + expect.soft(sources.get('Java')!.text).toContain(` + page.getByRole(AriaRole.BUTTON, new Page.GetByRoleOptions().setName("Submit")).dblclick()`); + + expect.soft(sources.get('C#')!.text).toContain(` +await page.GetByRole(AriaRole.Button, new() { Name = "Submit" }).DblClickAsync();`); + + expect(messages).toEqual([ + 'click 1', + 'click 2', + 'dblclick 2', + ]); + }); test('should ignore programmatic events', async ({ page, openRecorder }) => { const recorder = await openRecorder(); diff --git a/tests/library/inspector/inspectorTest.ts b/tests/library/inspector/inspectorTest.ts index 6ebbc1fdd1..02acdeb6fd 100644 --- a/tests/library/inspector/inspectorTest.ts +++ b/tests/library/inspector/inspectorTest.ts @@ -191,6 +191,13 @@ class Recorder { await this.page.mouse.up(options); } + async trustedDblclick() { + await this.page.mouse.down(); + await this.page.mouse.up(); + await this.page.mouse.down({ clickCount: 2 }); + await this.page.mouse.up(); + } + async focusElement(selector: string): Promise { return this.waitForHighlight(() => this.page.focus(selector)); }