From 80c0832e0bcc32d80697c68ec642e8e1746a90df Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Tue, 21 Jul 2026 14:35:53 +0100 Subject: [PATCH] feat(recorder): record actions by letting page events through naturally The default recorder no longer intercepts and re-performs user actions. Events flow through to the page, and actions are recorded from them. Right-click stays intercepted to open the actions dialog, whose items kick off the action so the resulting page events are recorded naturally. Click vs double click is disambiguated in the RecorderSignalProcessor by briefly buffering a single left click and merging a following double click into it; the api recorder now shares the same path. --- .../injected/src/recorder/pollingRecorder.ts | 12 +- packages/injected/src/recorder/recorder.ts | 249 ++++++++---------- packages/isomorphic/codegen/actions.d.ts | 2 +- .../playwright-core/src/server/recorder.ts | 25 +- .../src/server/recorder/recorderRunner.ts | 105 +------- .../recorder/recorderSignalProcessor.ts | 70 ++++- .../src/server/recorder/recorderUtils.ts | 6 - tests/library/debug-controller.spec.ts | 2 +- tests/library/inspector/cli-codegen-1.spec.ts | 26 ++ tests/library/inspector/cli-codegen-3.spec.ts | 67 ++++- tests/library/inspector/recorder-api.spec.ts | 9 +- 11 files changed, 283 insertions(+), 290 deletions(-) diff --git a/packages/injected/src/recorder/pollingRecorder.ts b/packages/injected/src/recorder/pollingRecorder.ts index c3da732fde8c0..bc8abb956ce21 100644 --- a/packages/injected/src/recorder/pollingRecorder.ts +++ b/packages/injected/src/recorder/pollingRecorder.ts @@ -22,8 +22,8 @@ import type * as actions from '@isomorphic/codegen/actions'; import type { ElementInfo, Mode, OverlayState, UIState } from '@recorder/recorderTypes'; interface Embedder { - __pw_recorderPerformAction(action: actions.PerformOnRecordAction, preconditionSelector?: string): Promise; - __pw_recorderRecordAction(action: actions.Action): Promise; + __pw_recorderPerformAction(action: actions.PerformableAction): Promise; + __pw_recorderRecordAction(action: actions.Action, preconditionSelector?: string): Promise; __pw_recorderState(): Promise; __pw_recorderElementPicked(element: { selector: string, ariaSnapshot?: string }): Promise; __pw_recorderSetMode(mode: Mode): Promise; @@ -76,12 +76,12 @@ export class PollingRecorder implements RecorderDelegate { this._pollRecorderModeTimer = this._recorder.injectedScript.utils.builtins.setTimeout(() => this._pollRecorderMode(), pollPeriod); } - async performAction(action: actions.PerformOnRecordAction, preconditionSelector?: string) { - await this._embedder.__pw_recorderPerformAction(action, preconditionSelector); + async performAction(action: actions.PerformableAction) { + await this._embedder.__pw_recorderPerformAction(action); } - async recordAction(action: actions.Action): Promise { - await this._embedder.__pw_recorderRecordAction(action); + async recordAction(action: actions.Action, preconditionSelector?: string): Promise { + await this._embedder.__pw_recorderRecordAction(action, preconditionSelector); } async elementPicked(elementInfo: ElementInfo): Promise { diff --git a/packages/injected/src/recorder/recorder.ts b/packages/injected/src/recorder/recorder.ts index 91f146ea1ab61..5fa7719cdbc65 100644 --- a/packages/injected/src/recorder/recorder.ts +++ b/packages/injected/src/recorder/recorder.ts @@ -35,8 +35,8 @@ const HighlightColors = { }; export interface RecorderDelegate { - performAction?(action: actions.PerformOnRecordAction, preconditionSelector?: string): Promise; - recordAction?(action: actions.Action): Promise; + performAction?(action: actions.PerformableAction): Promise; + recordAction?(action: actions.Action, preconditionSelector?: string): Promise; elementPicked?(elementInfo: ElementInfo): Promise; setMode?(mode: Mode): Promise; setOverlayState?(state: OverlayState): Promise; @@ -189,12 +189,10 @@ class InspectTool implements RecorderTool { class RecordActionTool implements RecorderTool { private _recorder: Recorder; - private _performingActions: Set; + private _performingActions: Set; private _hoveredModel: HighlightModelWithSelector | null = null; private _hoveredElement: HTMLElement | null = null; private _activeModel: HighlightModelWithSelector | null = null; - private _expectProgrammaticKeyUp = false; - private _pendingClickAction: { action: actions.ClickAction, timeout: number } | undefined; private _observer: MutationObserver | null = null; private _dialog: Dialog; @@ -232,7 +230,6 @@ class RecordActionTool implements RecorderTool { this._hoveredModel = null; this._hoveredElement = null; this._activeModel = null; - this._expectProgrammaticKeyUp = false; this._dialog.close(); } @@ -252,44 +249,43 @@ class RecordActionTool implements RecorderTool { return; if (this._shouldIgnoreMouseEvent(event)) return; - if (this._actionInProgress(event)) - return; - if (this._consumedDueToNoModel(event, this._hoveredModel)) - return; if (event.button === 2 && event.type === 'auxclick') { - this._showActionListDialog(this._hoveredModel!, event); + // A right-click we are performing on behalf of the dialog is recorded via onContextMenu. + if (!this._performingActions.size) + this._showActionListDialog(event); return; } - const checkbox = asCheckbox(this._recorder.deepEventTarget(event)); + // Keyboard-activated clicks, e.g. Enter on a button or Space on a checkbox, + // come with zero detail and are recorded in onKeyDown instead. + if (event.detail === 0) + return; + + const target = this._recorder.deepEventTarget(event); + const checkbox = asCheckbox(target); if (checkbox && event.detail === 1) { - // Interestingly, inputElement.checked is reversed inside this event handler. - this._performAction({ + // Note: inputElement.checked already reflects the new state inside this event handler. + this._recordAction({ name: checkbox.checked ? 'check' : 'uncheck', - selector: this._hoveredModel!.selector, + selector: this._hoveredModel?.selector ?? this._selectorForElement(target), signals: [], - }); + }, { autoExpect: true }); 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: this._recorder.injectedScript.utils.builtins.setTimeout(() => this._commitPendingClickAction(), 200) - }; - } + // Only single clicks are recorded here; double clicks are recorded in onDblClick. + if (event.detail !== 1) + return; + this._recordAction({ + name: 'click', + selector: this._hoveredModel?.selector ?? this._selectorForElement(target), + position: positionForEvent(event), + signals: [], + button: buttonForEvent(event), + modifiers: modifiersForEvent(event), + clickCount: event.detail + }, { autoExpect: true }); } onDblClick(event: MouseEvent) { @@ -299,35 +295,17 @@ class RecordActionTool implements RecorderTool { 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({ + const target = this._recorder.deepEventTarget(event); + this._recordAction({ name: 'click', - selector: this._hoveredModel!.selector, + selector: this._hoveredModel?.selector ?? this._selectorForElement(target), position: positionForEvent(event), signals: [], button: buttonForEvent(event), modifiers: modifiersForEvent(event), clickCount: event.detail - }); - } - - private _commitPendingClickAction() { - if (this._pendingClickAction) - this._performAction(this._pendingClickAction.action); - this._cancelPendingClickAction(); - } - - private _cancelPendingClickAction() { - if (this._pendingClickAction) - this._recorder.injectedScript.utils.builtins.clearTimeout(this._pendingClickAction.timeout); - this._pendingClickAction = undefined; + }, { autoExpect: true }); } onContextMenu(event: MouseEvent) { @@ -339,11 +317,21 @@ class RecordActionTool implements RecorderTool { } if (this._shouldIgnoreMouseEvent(event)) return; - if (this._actionInProgress(event)) - return; - if (this._consumedDueToNoModel(event, this._hoveredModel)) + if (this._performingActions.size) { + // The dialog is performing a right-click for us; record it naturally instead of reopening the dialog. + const target = this._recorder.deepEventTarget(event); + this._recordAction({ + name: 'click', + selector: this._hoveredModel?.selector ?? this._selectorForElement(target), + position: positionForEvent(event), + signals: [], + button: 'right', + modifiers: modifiersForEvent(event), + clickCount: 1, + }, { autoExpect: true }); return; - this._showActionListDialog(this._hoveredModel!, event); + } + this._showActionListDialog(event); } onPointerDown(event: PointerEvent) { @@ -351,7 +339,7 @@ class RecordActionTool implements RecorderTool { return; if (this._shouldIgnoreMouseEvent(event)) return; - this._consumeWhenAboutToPerform(event); + this._consumeRightButtonEvent(event); } onPointerUp(event: PointerEvent) { @@ -359,7 +347,7 @@ class RecordActionTool implements RecorderTool { return; if (this._shouldIgnoreMouseEvent(event)) return; - this._consumeWhenAboutToPerform(event); + this._consumeRightButtonEvent(event); } onMouseDown(event: MouseEvent) { @@ -367,7 +355,7 @@ class RecordActionTool implements RecorderTool { return; if (this._shouldIgnoreMouseEvent(event)) return; - this._consumeWhenAboutToPerform(event); + this._consumeRightButtonEvent(event); this._activeModel = this._hoveredModel; } @@ -376,7 +364,7 @@ class RecordActionTool implements RecorderTool { return; if (this._shouldIgnoreMouseEvent(event)) return; - this._consumeWhenAboutToPerform(event); + this._consumeRightButtonEvent(event); } onMouseMove(event: MouseEvent) { @@ -415,9 +403,9 @@ class RecordActionTool implements RecorderTool { // When the file input is hidden and triggered by another element (e.g. a button with // onclick="input.click()"), the hover model points to the trigger, not the input. // Derive the selector from the actual target element in that case. - const selector = target === this._hoveredElement - ? this._hoveredModel!.selector - : this._recorder.injectedScript.generateSelector(target, { testIdAttributeName: this._recorder.state.testIdAttributeName }).selector; + const selector = target === this._hoveredElement && this._hoveredModel + ? this._hoveredModel.selector + : this._selectorForElement(target); this._recordAction({ name: 'setInputFiles', selector, @@ -431,7 +419,7 @@ class RecordActionTool implements RecorderTool { this._recordAction({ name: 'fill', // must use hoveredModel instead of activeModel for it to work in webkit - selector: this._hoveredModel!.selector, + selector: this._hoveredModel?.selector ?? this._selectorForElement(target), signals: [], text: target.value, }); @@ -440,16 +428,13 @@ class RecordActionTool implements RecorderTool { if (['INPUT', 'TEXTAREA'].includes(target.nodeName) || target.isContentEditable) { if (target.nodeName === 'INPUT' && ['checkbox', 'radio'].includes((target as HTMLInputElement).type.toLowerCase())) { - // Checkbox is handled in click, we can't let input trigger on checkbox - that would mean we dispatched click events while recording. + // Checkbox is handled in click, no need to record a duplicate action for the input event. return; } - // Non-navigating actions are simply recorded by Playwright. - if (this._consumedDueWrongTarget(event)) - return; this._recordAction({ name: 'fill', - selector: this._activeModel!.selector, + selector: this._activeSelectorForEvent(event), signals: [], text: target.isContentEditable ? target.innerText : (target as HTMLInputElement).value, }); @@ -459,7 +444,7 @@ class RecordActionTool implements RecorderTool { const selectElement = target as HTMLSelectElement; this._recordAction({ name: 'select', - selector: this._activeModel!.selector, + selector: this._activeSelectorForEvent(event), options: [...selectElement.selectedOptions].map(option => option.value), signals: [] }); @@ -471,46 +456,26 @@ class RecordActionTool implements RecorderTool { return; if (!this._shouldGenerateKeyPressFor(event)) return; - if (this._actionInProgress(event)) { - this._expectProgrammaticKeyUp = true; - return; - } - if (this._consumedDueWrongTarget(event)) - return; // Similarly to click, trigger checkbox on key event, not input. if (event.key === ' ') { const checkbox = asCheckbox(this._recorder.deepEventTarget(event)); if (checkbox && event.detail === 0) { - this._performAction({ + this._recordAction({ name: checkbox.checked ? 'uncheck' : 'check', - selector: this._activeModel!.selector, + selector: this._activeSelectorForEvent(event), signals: [], - }); + }, { autoExpect: true }); return; } } - this._performAction({ + this._recordAction({ name: 'press', - selector: this._activeModel!.selector, + selector: this._activeSelectorForEvent(event), signals: [], key: event.key, modifiers: modifiersForEvent(event), - }); - } - - onKeyUp(event: KeyboardEvent) { - if (this._dialog.isShowing()) - return; - if (!this._shouldGenerateKeyPressFor(event)) - return; - - // Only allow programmatic keyups, ignore user input. - if (!this._expectProgrammaticKeyUp) { - consumeEvent(event); - return; - } - this._expectProgrammaticKeyUp = false; + }, { autoExpect: true }); } onScroll(event: Event) { @@ -519,8 +484,12 @@ class RecordActionTool implements RecorderTool { this._resetHoveredModel(); } - private _showActionListDialog(model: HighlightModelWithSelector, event: MouseEvent) { + private _showActionListDialog(event: MouseEvent) { + // Right click is always intercepted and opens the actions dialog instead of being passed to the page. consumeEvent(event); + const model = this._hoveredModel ?? this._modelForElement(this._recorder.deepEventTarget(event)); + if (!model) + return; const actionPosition = positionForEvent(event); const actions: { title: string, cb: () => void }[] = [ { @@ -532,7 +501,7 @@ class RecordActionTool implements RecorderTool { signals: [], button: 'left', modifiers: 0, - clickCount: 0, + clickCount: 1, }), }, { @@ -544,7 +513,7 @@ class RecordActionTool implements RecorderTool { signals: [], button: 'right', modifiers: 0, - clickCount: 0, + clickCount: 1, }), }, { @@ -561,7 +530,7 @@ class RecordActionTool implements RecorderTool { }, { title: 'Hover', - cb: () => this._performAction({ + cb: () => this._recordAction({ name: 'hover', selector: model.selector, position: actionPosition, @@ -623,39 +592,26 @@ class RecordActionTool implements RecorderTool { return shouldIgnoreMouseEvent(this._recorder.deepEventTarget(event)); } - private _actionInProgress(event: Event): boolean { - // If Playwright is performing action for us, bail. - const isKeyEvent = event instanceof KeyboardEvent; - const isMouseOrPointerEvent = event instanceof MouseEvent || event instanceof PointerEvent; - for (const action of this._performingActions) { - if (isKeyEvent && action.name === 'press' && event.key === action.key) - return true; - if (isMouseOrPointerEvent && (action.name === 'click' || action.name === 'hover' || action.name === 'check' || action.name === 'uncheck')) - return true; - } - - // Consume event if action is not being executed. - consumeEvent(event); - return false; + private _consumeRightButtonEvent(event: MouseEvent) { + // Right click is intercepted to open the actions dialog, so the page should not see it. + if (event.button === 2 && !this._performingActions.size) + consumeEvent(event); } - private _consumedDueToNoModel(event: Event, model: HighlightModel | null): boolean { - if (model) - return false; - consumeEvent(event); - return true; + private _selectorForElement(element: HTMLElement): string { + return this._recorder.injectedScript.generateSelector(element, { testIdAttributeName: this._recorder.state.testIdAttributeName }).selector; } - private _consumedDueWrongTarget(event: Event): boolean { - if (this._activeModel && this._activeModel.elements[0] === this._recorder.deepEventTarget(event)) - return false; - consumeEvent(event); - return true; + private _modelForElement(element: HTMLElement): HighlightModelWithSelector | null { + const { selector, elements } = this._recorder.injectedScript.generateSelector(element, { testIdAttributeName: this._recorder.state.testIdAttributeName }); + return selector ? { selector, elements, color: HighlightColors.action } : null; } - private _consumeWhenAboutToPerform(event: Event) { - if (!this._performingActions.size) - consumeEvent(event); + private _activeSelectorForEvent(event: Event): string { + const target = this._recorder.deepEventTarget(event); + if (this._activeModel && this._activeModel.elements[0] === target) + return this._activeModel.selector; + return this._selectorForElement(target); } private _reportPerformedActionForTests() { @@ -669,11 +625,11 @@ class RecordActionTool implements RecorderTool { })); } - private _recordAction(action: actions.Action) { - void this._recorder.recordAction(action).then(() => this._reportPerformedActionForTests()); + private _recordAction(action: actions.Action, options?: { autoExpect?: boolean }) { + void this._recorder.recordAction(action, options).then(() => this._reportPerformedActionForTests()); } - private _performAction(action: actions.PerformOnRecordAction) { + private _performAction(action: actions.PerformableAction) { this._recorder.updateHighlight(null, false); this._performingActions.add(action); @@ -1695,22 +1651,25 @@ export class Recorder { return documentElement ? this.injectedScript.utils.generateAriaTree(documentElement, { mode: 'autoexpect' }) : undefined; } - async performAction(action: actions.PerformOnRecordAction) { + private _computeAutoExpectPrecondition(action: actions.Action, autoExpect: boolean): string | undefined { const previousSnapshot = this._lastActionAutoexpectSnapshot; this._lastActionAutoexpectSnapshot = this._captureAutoExpectSnapshot(); - let preconditionSelector: string | undefined; - if (!isAssertAction(action) && this._lastActionAutoexpectSnapshot) { - const element = this.injectedScript.utils.findNewElement(previousSnapshot?.root, this._lastActionAutoexpectSnapshot?.root); - preconditionSelector = element ? this.injectedScript.generateSelector(element, { testIdAttributeName: this.state.testIdAttributeName }).selector : undefined; - if (preconditionSelector === action.selector) - preconditionSelector = undefined; - } - await this._delegate.performAction?.(action, preconditionSelector).catch(() => {}); + if (!autoExpect || isAssertAction(action) || !this._lastActionAutoexpectSnapshot) + return; + const element = this.injectedScript.utils.findNewElement(previousSnapshot?.root, this._lastActionAutoexpectSnapshot.root); + let preconditionSelector = element ? this.injectedScript.generateSelector(element, { testIdAttributeName: this.state.testIdAttributeName }).selector : undefined; + if ('selector' in action && preconditionSelector === action.selector) + preconditionSelector = undefined; + return preconditionSelector; } - async recordAction(action: actions.Action) { - this._lastActionAutoexpectSnapshot = this._captureAutoExpectSnapshot(); - await this._delegate.recordAction?.(action); + async performAction(action: actions.PerformableAction) { + await this._delegate.performAction?.(action).catch(() => {}); + } + + async recordAction(action: actions.Action, options?: { autoExpect?: boolean }) { + const preconditionSelector = this._computeAutoExpectPrecondition(action, !!options?.autoExpect); + await this._delegate.recordAction?.(action, preconditionSelector); } setOverlayState(state: { offsetX: number; }) { diff --git a/packages/isomorphic/codegen/actions.d.ts b/packages/isomorphic/codegen/actions.d.ts index f6dbc6b51a115..e75e5234bbc74 100644 --- a/packages/isomorphic/codegen/actions.d.ts +++ b/packages/isomorphic/codegen/actions.d.ts @@ -128,7 +128,7 @@ export type AssertSnapshotAction = ActionWithSelector & { export type Action = ClickAction | HoverAction | CheckAction | ClosesPageAction | OpenPageAction | UncheckAction | FillAction | NavigateAction | PressAction | SelectAction | SetInputFilesAction | AssertTextAction | AssertValueAction | AssertCheckedAction | AssertVisibleAction | AssertSnapshotAction; export type AssertAction = AssertCheckedAction | AssertValueAction | AssertTextAction | AssertVisibleAction | AssertSnapshotAction; -export type PerformOnRecordAction = ClickAction | HoverAction | CheckAction | UncheckAction | PressAction | SelectAction; +export type PerformableAction = ClickAction; // Signals. diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 5ef7ffc08f1e3..a49150e239b5b 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -224,14 +224,11 @@ export class Recorder extends EventEmitter implements Instrume return false; }); - // Input actions that potentially lead to navigation are intercepted on the page and are - // performed by the Playwright. await this._context.exposeBinding(progress, '__pw_recorderPerformAction', - (source: BindingSource, action: actions.PerformOnRecordAction, preconditionSelector?: string) => this._performAction(progress, source.frame, action, preconditionSelector)); + (source: BindingSource, action: actions.PerformableAction) => this._performAction(progress, source.frame, action)); - // Other non-essential actions are simply being recorded. await this._context.exposeBinding(progress, '__pw_recorderRecordAction', - (source: BindingSource, action: actions.Action) => this._recordAction(progress, source.frame, action)); + (source: BindingSource, action: actions.Action, preconditionSelector?: string) => this._recordAction(progress, source.frame, action, preconditionSelector)); await progress.race(this._context.extendInjectedScript(rawRecorderSource.source, { recorderMode: this._recorderMode, hideToolbar: !!this._params.hideToolbar })); }); @@ -556,22 +553,16 @@ export class Recorder extends EventEmitter implements Instrume return actionInContext; } - private async _performAction(progress: Progress, frame: Frame, action: actions.PerformOnRecordAction, preconditionSelector?: string) { + private async _performAction(progress: Progress, frame: Frame, action: actions.PerformableAction) { const framePath = await generateFrameSelector(progress, frame); - if (preconditionSelector) - this._signalProcessor.signal(frame, { name: 'expect', selector: buildFullSelector(framePath, preconditionSelector) }); - const actionInContext = this._appendContextToAction(frame, action, framePath); - this._signalProcessor.addAction(actionInContext); - try { - if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage') - await performAction(progress, frame._page.mainFrame(), actionInContext); - } finally { - actionInContext.endTime = monotonicTime(); - } + const selector = buildFullSelector(framePath, action.selector); + await performAction(progress, frame._page.mainFrame(), { ...action, selector }); } - private async _recordAction(progress: Progress, frame: Frame, action: actions.Action) { + private async _recordAction(progress: Progress, frame: Frame, action: actions.Action, preconditionSelector?: string) { const framePath = await generateFrameSelector(progress, frame); + if (preconditionSelector) + this._signalProcessor.signal(frame, { name: 'expect', selector: buildFullSelector(framePath, preconditionSelector) }); const actionInContext = this._appendContextToAction(frame, action, framePath); this._signalProcessor.addAction(actionInContext); } diff --git a/packages/playwright-core/src/server/recorder/recorderRunner.ts b/packages/playwright-core/src/server/recorder/recorderRunner.ts index 6b2e92dee9e4f..0eba6bfff4f10 100644 --- a/packages/playwright-core/src/server/recorder/recorderRunner.ts +++ b/packages/playwright-core/src/server/recorder/recorderRunner.ts @@ -21,108 +21,9 @@ import type * as types from '../types'; import type * as actions from '@isomorphic/codegen/actions'; import type { Frame } from '../frames'; -export async function performAction(progress: Progress, mainFrame: Frame, actionInContext: actions.ActionInContext) { - const { action } = actionInContext; - - if (action.name === 'navigate') { - await mainFrame.goto(progress, action.url); - return; - } - - if (action.name === 'openPage') - throw Error('Not reached'); - - if (action.name === 'closePage') { - await mainFrame._page.close(progress); - return; - } - - const selector = action.selector; - - if (action.name === 'click') { - const options = toClickOptions(action); - await mainFrame.click(progress, selector, { ...options, strict: true }); - return; - } - - if (action.name === 'hover') { - await mainFrame.hover(progress, selector, { position: action.position, strict: true }); - return; - } - - if (action.name === 'press') { - const modifiers = toKeyboardModifiers(action.modifiers); - const shortcut = [...modifiers, action.key].join('+'); - await mainFrame.press(progress, selector, shortcut, { strict: true }); - return; - } - - if (action.name === 'fill') { - await mainFrame.fill(progress, selector, action.text, { strict: true }); - return; - } - - if (action.name === 'setInputFiles') { - await mainFrame.setInputFiles(progress, selector, { selector, payloads: [], strict: true }); - return; - } - - if (action.name === 'check') { - await mainFrame.check(progress, selector, { strict: true }); - return; - } - - if (action.name === 'uncheck') { - await mainFrame.uncheck(progress, selector, { strict: true }); - return; - } - - if (action.name === 'select') { - const values = action.options.map(value => ({ value })); - await mainFrame.selectOption(progress, selector, [], values, { strict: true }); - return; - } - - if (action.name === 'assertChecked') { - await mainFrame.expect(progress, selector, { - selector, - expression: 'to.be.checked', - expectedValue: { checked: action.checked }, - isNot: !action.checked, - }); - return; - } - - if (action.name === 'assertText') { - await mainFrame.expect(progress, selector, { - selector, - expression: 'to.have.text', - expectedText: [{ string: action.text, matchSubstring: true, normalizeWhiteSpace: true }], - isNot: false, - }); - return; - } - - if (action.name === 'assertValue') { - await mainFrame.expect(progress, selector, { - selector, - expression: 'to.have.value', - expectedValue: action.value, - isNot: false, - }); - return; - } - - if (action.name === 'assertVisible') { - await mainFrame.expect(progress, selector, { - selector, - expression: 'to.be.visible', - isNot: false, - }); - return; - } - - throw new Error('Internal error: unexpected action ' + (action as any).name); +export async function performAction(progress: Progress, mainFrame: Frame, action: actions.PerformableAction) { + const options = toClickOptions(action); + await mainFrame.click(progress, action.selector, { ...options, strict: true }); } export function toClickOptions(action: actions.ClickAction): types.MouseClickOptions { diff --git a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts index fb510fe657f26..aedc4a4278b3c 100644 --- a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts +++ b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts @@ -26,21 +26,85 @@ export interface ProcessorDelegate { addSignal(signalInContext: actions.SignalInContext): void; } +// How long a single click is held back, waiting for a double click to arrive and merge with it. +const kClickBufferTimeout = 500; + +type BufferedSignal = { frame: Frame, signal: Signal, timestamp: number }; + export class RecorderSignalProcessor { private _delegate: ProcessorDelegate; private _lastAction: actions.ActionInContext | null = null; + private _bufferedClick: { actionInContext: actions.ActionInContext, signals: BufferedSignal[], timeout: NodeJS.Timeout } | undefined; constructor(actionSink: ProcessorDelegate) { this._delegate = actionSink; } addAction(actionInContext: actions.ActionInContext) { - this._lastAction = actionInContext; - this._delegate.addAction(actionInContext); + if (this._bufferedClick) { + if (this._isDoubleClick(actionInContext, this._bufferedClick.actionInContext)) { + // A double click - merge it into the buffered single click and emit the result. + actionInContext.startTime = this._bufferedClick.actionInContext.startTime; + this._flushBufferedClick(actionInContext); + return; + } + // A different action - emit the buffered click before proceeding. + this._flushBufferedClick(); + } + + if (this._isBufferableClick(actionInContext)) { + this._bufferedClick = { + actionInContext, + signals: [], + timeout: setTimeout(() => this._flushBufferedClick(), kClickBufferTimeout), + }; + return; + } + + this._emitAction(actionInContext); } signal(frame: Frame, signal: Signal) { const timestamp = monotonicTime(); + if (this._bufferedClick) { + this._bufferedClick.signals.push({ frame, signal, timestamp }); + return; + } + this._processSignal(frame, signal, timestamp); + } + + private _isBufferableClick(actionInContext: actions.ActionInContext): boolean { + const action = actionInContext.action; + return action.name === 'click' && action.button === 'left' && action.clickCount === 1; + } + + private _isDoubleClick(actionInContext: actions.ActionInContext, bufferedClick: actions.ActionInContext): boolean { + const action = actionInContext.action; + const buffered = bufferedClick.action; + return action.name === 'click' && buffered.name === 'click' + && actionInContext.pageGuid === bufferedClick.pageGuid + && action.selector === buffered.selector + && action.clickCount > buffered.clickCount; + } + + private _emitAction(actionInContext: actions.ActionInContext) { + this._lastAction = actionInContext; + this._delegate.addAction(actionInContext); + } + + private _flushBufferedClick(replacement?: actions.ActionInContext) { + const buffered = this._bufferedClick; + if (!buffered) + return; + clearTimeout(buffered.timeout); + this._bufferedClick = undefined; + this._emitAction(replacement ?? buffered.actionInContext); + // Replay the signals with their original timestamps, so that they attach to the emitted action. + for (const { frame, signal, timestamp } of buffered.signals) + this._processSignal(frame, signal, timestamp); + } + + private _processSignal(frame: Frame, signal: Signal, timestamp: number) { if (signal.name === 'navigation' && frame._page.mainFrame() === frame) { const lastAction = this._lastAction; const signalThreshold = isUnderTest() ? 500 : 5000; @@ -54,7 +118,7 @@ export class RecorderSignalProcessor { generateGoto = true; if (generateGoto) { - this.addAction({ + this._emitAction({ pageGuid: frame._page.guid, action: { name: 'navigate', diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index fd66b37da8c9e..38abf76dde1b7 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -62,10 +62,6 @@ function isSameSelector(action: actions.ActionInContext, lastAction: actions.Act return 'selector' in action.action && 'selector' in lastAction.action && action.action.selector === lastAction.action.selector; } -function isShortlyAfter(action: actions.ActionInContext, lastAction: actions.ActionInContext): boolean { - return action.startTime - lastAction.startTime < 500; -} - export function shouldMergeAction(action: actions.ActionInContext, lastAction: actions.ActionInContext | undefined): boolean { if (!lastAction) return false; @@ -74,8 +70,6 @@ export function shouldMergeAction(action: actions.ActionInContext, lastAction: a return isSameAction(action, lastAction) && isSameSelector(action, lastAction); case 'navigate': return isSameAction(action, lastAction); - case 'click': - return isSameAction(action, lastAction) && isSameSelector(action, lastAction) && isShortlyAfter(action, lastAction) && action.action.clickCount > (lastAction.action as actions.ClickAction).clickCount; } return false; } diff --git a/tests/library/debug-controller.spec.ts b/tests/library/debug-controller.spec.ts index 0bdb25cc24658..784b5e00253c1 100644 --- a/tests/library/debug-controller.spec.ts +++ b/tests/library/debug-controller.spec.ts @@ -225,7 +225,7 @@ test('should record expect signal', async ({ backend, connectedBrowser }) => { `); await page.getByRole('button', { name: 'Show' }).click(); - // A click stalls for 200ms to detect a double click, and the next click cancels a pending one. + // A click is buffered for a while to detect a double click, so wait for it to be recorded. await expect.poll(() => events[events.length - 1]?.actions.length).toBe(2); await page.getByRole('button', { name: 'Other' }).click(); diff --git a/tests/library/inspector/cli-codegen-1.spec.ts b/tests/library/inspector/cli-codegen-1.spec.ts index d793079e48ff9..d228c7b7d7552 100644 --- a/tests/library/inspector/cli-codegen-1.spec.ts +++ b/tests/library/inspector/cli-codegen-1.spec.ts @@ -556,6 +556,32 @@ await page.GetByRole(AriaRole.Textbox).PressAsync("Shift+Enter");`); expect(messages[1].text()).toBe('up:ArrowDown'); }); + test('should not record a click on Enter press', async ({ openRecorder }) => { + const { page, recorder } = await openRecorder(); + + await recorder.setContentAndWait(``); + + const locator = await recorder.focusElement('button'); + expect(locator).toBe(`getByRole('button', { name: 'Submit' })`); + + const [message] = await Promise.all([ + page.waitForEvent('console', msg => msg.type() !== 'error'), + recorder.waitForOutput('JavaScript', `press('Enter')`), + page.keyboard.press('Enter'), + ]); + expect(message.text()).toBe('clicked'); + + // Wait for the next action to be recorded, to make sure the keyboard-activated + // click event that follows the Enter press did not produce a click action. + const [sources] = await Promise.all([ + recorder.waitForOutput('JavaScript', `press('Tab')`), + page.keyboard.press('Tab'), + ]); + expect(sources.get('JavaScript')!.text).toContain(` + await page.getByRole('button', { name: 'Submit' }).press('Enter');`); + expect(sources.get('JavaScript')!.text).not.toContain(`click()`); + }); + test('should check', async ({ openRecorder }) => { const { page, recorder } = await openRecorder(); diff --git a/tests/library/inspector/cli-codegen-3.spec.ts b/tests/library/inspector/cli-codegen-3.spec.ts index e6851934698e0..4dbf8dbe990dc 100644 --- a/tests/library/inspector/cli-codegen-3.spec.ts +++ b/tests/library/inspector/cli-codegen-3.spec.ts @@ -666,7 +666,7 @@ await page.GetByRole(AriaRole.Textbox, new() { Name = "Country" }).ClickAsync(); await page.GetByRole(AriaRole.Textbox, new() { Name = \"Coun\\\"try\" }).ClickAsync();`); }); - test('should consume pointer events', async ({ openRecorder }) => { + test('should pass through pointer events', async ({ openRecorder }) => { const { page, recorder } = await openRecorder(); await recorder.setContentAndWait(` @@ -690,8 +690,6 @@ await page.GetByRole(AriaRole.Textbox, new() { Name = \"Coun\\\"try\" }).ClickAs expect(message.text()).toBe('clicked'); expect(await page.evaluate('log')).toEqual([ 'pointermove', 'mousemove', - 'pointermove', - 'mousemove', 'pointerdown', 'mousedown', 'pointerup', 'mouseup', 'click', @@ -744,6 +742,52 @@ await page.GetByRole(AriaRole.Textbox, new() { Name = \"Coun\\\"try\" }).ClickAs expect(await page.evaluate('log')).toEqual((isWindows && browserName === 'chromium') ? ['button: auxclick', 'button: contextmenu'] : ['button: contextmenu']); }); + test('should generate click action from dialog', async ({ openRecorder }) => { + const { page, recorder } = await openRecorder(); + + await recorder.setContentAndWait(``); + await recorder.hoverOverElement('button'); + + const action = async () => { + await recorder.trustedClick({ button: 'right' }); + await recorder.page.getByRole('listitem', { name: 'Click', exact: true }).click(); + }; + + // The dialog kicks off a click; the page reacts and the click is recorded naturally. + const [message, sources] = await Promise.all([ + page.waitForEvent('console', msg => msg.type() !== 'error'), + recorder.waitForOutput('JavaScript', 'click'), + action(), + ]); + expect(message.text()).toBe('clicked'); + expect(sources.get('JavaScript')!.text).toContain(` + await page.getByRole('button', { name: 'Submit' }).click();`); + }); + + test('should generate double click action from dialog', async ({ openRecorder }) => { + const { page, recorder } = await openRecorder(); + + await recorder.setContentAndWait(``); + await recorder.hoverOverElement('button'); + + const action = async () => { + await recorder.trustedClick({ button: 'right' }); + await recorder.page.getByRole('listitem', { name: 'Double click' }).click(); + }; + + // The dialog kicks off a double click; it is recorded as a single dblclick action. + const [message, sources] = await Promise.all([ + page.waitForEvent('console', msg => msg.type() !== 'error' && msg.text() === 'dblclicked'), + recorder.waitForOutput('JavaScript', 'dblclick'), + action(), + ]); + expect(message.text()).toBe('dblclicked'); + const text = sources.get('JavaScript')!.text; + expect(text).toContain(` + await page.getByRole('button', { name: 'Submit' }).dblclick();`); + expect(text).not.toContain(`.click();`); + }); + test('should generate hover action', async ({ openRecorder }) => { const { recorder } = await openRecorder(); @@ -1082,9 +1126,22 @@ await page.GetByTestId("testid").HoverAsync();`); `); - await page.getByRole('button', { name: 'Go Fullscreen' }).click(); - await expect(page.getByRole('button', { name: 'Close Fullscreen' })).toBeVisible(); + const [sources] = await Promise.all([ + recorder.waitForOutput('JavaScript', 'Go Fullscreen'), + page.getByRole('button', { name: 'Go Fullscreen' }).click(), + ]); + expect(sources.get('JavaScript')!.text).toContain(`getByRole('button', { name: 'Go Fullscreen' }).click()`); + await page.waitForFunction(() => !!document.fullscreenElement); + + // Actions inside the fullscreen element are recorded. + const [sources2] = await Promise.all([ + recorder.waitForOutput('JavaScript', 'Close Fullscreen'), + page.getByRole('button', { name: 'Close Fullscreen' }).click(), + ]); + expect(sources2.get('JavaScript')!.text).toContain(`getByRole('button', { name: 'Close Fullscreen' }).click()`); + await page.waitForFunction(() => !document.fullscreenElement); + // After exiting fullscreen, the toolbar is clickable again. await page.getByTitle('Assert text').click(); }); }); diff --git a/tests/library/inspector/recorder-api.spec.ts b/tests/library/inspector/recorder-api.spec.ts index 568c09df57663..a86ff53fb45bc 100644 --- a/tests/library/inspector/recorder-api.spec.ts +++ b/tests/library/inspector/recorder-api.spec.ts @@ -52,8 +52,7 @@ test('should click', async ({ context, browserName, platform, channel }) => { await page.setContent(``); await page.getByRole('button', { name: 'Submit' }).click(); - const clickActions = log.action('click'); - expect(clickActions).toEqual([ + await expect.poll(() => log.action('click')).toEqual([ expect.objectContaining({ action: expect.objectContaining({ name: 'click', @@ -66,7 +65,7 @@ test('should click', async ({ context, browserName, platform, channel }) => { }) ]); - expect(normalizeCode(clickActions[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).click();`); + expect(normalizeCode(log.action('click')[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).click();`); }); test('should double click', async ({ context, browserName, platform, channel }) => { @@ -146,9 +145,11 @@ test('should disable recorder', async ({ context }) => { await page.setContent(``); await page.getByRole('button', { name: 'Submit' }).click(); await page.getByRole('button', { name: 'Submit' }).click(); - expect(log.action('click')).toHaveLength(2); + await expect.poll(() => log.action('click').length).toBe(2); await (context as any)._disableRecorder(); await page.getByRole('button', { name: 'Submit' }).click(); + // Give it some time to produce more actions - there should be none. + await page.waitForTimeout(2000); expect(log.action('click')).toHaveLength(2); });