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); });