From 9d801c4aa3aa337317433171770ea96ae50c9513 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Fri, 10 Jul 2026 15:53:34 +0100 Subject: [PATCH 1/2] chore(recorder): unify double click handling, drop action timestamps JsonRecordActionTool recorded every click as it arrived, so a double click produced two actions that shouldMergeAction() had to collapse. Give it the same stalled-click handling RecordActionTool has, and drop the click branch of shouldMergeAction(). While stalling a click, a following click cancelled the pending one instead of committing it, silently swallowing the first click. Both tools now commit the pending click when a new single click arrives, and only cancel it when the click continues a multi-click, which 'dblclick' reports instead. Also remove ActionInContext.startTime/endTime and SignalInContext.timestamp. endTime and timestamp were never read; startTime had a single consumer, the navigation signal threshold, which now lives in RecorderSignalProcessor._lastActionTimestamp. --- packages/injected/src/recorder/recorder.ts | 61 ++++++++++++++++++- packages/isomorphic/codegen/actions.d.ts | 3 - packages/isomorphic/codegen/language.ts | 2 - .../playwright-core/src/server/recorder.ts | 12 +--- .../recorder/recorderSignalProcessor.ts | 8 +-- .../src/server/recorder/recorderUtils.ts | 16 +---- tests/library/debug-controller.spec.ts | 3 +- tests/library/inspector/recorder-api.spec.ts | 28 ++++----- 8 files changed, 79 insertions(+), 54 deletions(-) diff --git a/packages/injected/src/recorder/recorder.ts b/packages/injected/src/recorder/recorder.ts index 91f146ea1ab61..ca6805ed7bd43 100644 --- a/packages/injected/src/recorder/recorder.ts +++ b/packages/injected/src/recorder/recorder.ts @@ -262,6 +262,14 @@ class RecordActionTool implements RecorderTool { return; } + if (event.detail === 1) { + // A new click starts here, so the stalled one is not a double click after all. + this._commitPendingClickAction(); + } else { + // This click continues a multi-click, which is reported by 'dblclick' instead. + this._cancelPendingClickAction(); + } + const checkbox = asCheckbox(this._recorder.deepEventTarget(event)); if (checkbox && event.detail === 1) { // Interestingly, inputElement.checked is reversed inside this event handler. @@ -273,8 +281,6 @@ class RecordActionTool implements RecorderTool { return; } - this._cancelPendingClickAction(); - // Stall click in case we are observing double-click. if (event.detail === 1) { this._pendingClickAction = { @@ -741,6 +747,7 @@ class RecordActionTool implements RecorderTool { class JsonRecordActionTool implements RecorderTool { private _recorder: Recorder; + private _pendingClickAction: { action: actions.ClickAction, timeout: number } | undefined; constructor(recorder: Recorder) { this._recorder = recorder; @@ -752,6 +759,7 @@ class JsonRecordActionTool implements RecorderTool { } uninstall() { + this._cancelPendingClickAction(); this._recorder.highlight.install(); } @@ -767,6 +775,14 @@ class JsonRecordActionTool implements RecorderTool { if (this._shouldIgnoreMouseEvent(event)) return; + if (event.detail === 1) { + // A new click starts here, so the stalled one is not a double click after all. + this._commitPendingClickAction(); + } else { + // This click continues a multi-click, which is reported by 'dblclick' instead. + this._cancelPendingClickAction(); + } + const checkbox = asCheckbox(element); const { ariaSnapshot, selector, ref } = this._ariaSnapshot(element); if (checkbox && event.detail === 1) { @@ -781,6 +797,35 @@ class JsonRecordActionTool implements RecorderTool { return; } + // Stall click in case we are observing double-click. + if (event.detail === 1) { + this._pendingClickAction = { + action: { + name: 'click', + selector, + ref, + ariaSnapshot, + position: positionForEvent(event), + signals: [], + button: buttonForEvent(event), + modifiers: modifiersForEvent(event), + clickCount: event.detail, + }, + timeout: this._recorder.injectedScript.utils.builtins.setTimeout(() => this._commitPendingClickAction(), 200) + }; + } + } + + onDblClick(event: MouseEvent) { + const element = this._recorder.deepEventTarget(event); + if (isRangeInput(element)) + return; + if (this._shouldIgnoreMouseEvent(event)) + return; + + this._cancelPendingClickAction(); + + const { ariaSnapshot, selector, ref } = this._ariaSnapshot(element); void this._recorder.recordAction({ name: 'click', selector, @@ -794,6 +839,18 @@ class JsonRecordActionTool implements RecorderTool { }); } + private _commitPendingClickAction() { + if (this._pendingClickAction) + void this._recorder.recordAction(this._pendingClickAction.action); + this._cancelPendingClickAction(); + } + + private _cancelPendingClickAction() { + if (this._pendingClickAction) + this._recorder.injectedScript.utils.builtins.clearTimeout(this._pendingClickAction.timeout); + this._pendingClickAction = undefined; + } + onContextMenu(event: MouseEvent): void { const element = this._recorder.deepEventTarget(event); const { ariaSnapshot, selector, ref } = this._ariaSnapshot(element); diff --git a/packages/isomorphic/codegen/actions.d.ts b/packages/isomorphic/codegen/actions.d.ts index f6dbc6b51a115..73c28873bd72c 100644 --- a/packages/isomorphic/codegen/actions.d.ts +++ b/packages/isomorphic/codegen/actions.d.ts @@ -166,12 +166,9 @@ export type Signal = NavigationSignal | PopupSignal | DownloadSignal | DialogSig export type ActionInContext = { pageGuid: string; action: Action; - startTime: number; - endTime?: number; }; export type SignalInContext = { pageGuid: string; signal: Signal; - timestamp: number; }; diff --git a/packages/isomorphic/codegen/language.ts b/packages/isomorphic/codegen/language.ts index eb3df508efd3d..ee71f98ceb3cf 100644 --- a/packages/isomorphic/codegen/language.ts +++ b/packages/isomorphic/codegen/language.ts @@ -31,8 +31,6 @@ export function generateCode(actions: actions.ActionInContext[], languageGenerat export function expectSignalAction(actionInContext: actions.ActionInContext, signal: actions.ExpectSignal): actions.ActionInContext { return { pageGuid: actionInContext.pageGuid, - startTime: actionInContext.startTime, - endTime: actionInContext.startTime, action: { name: 'assertVisible', selector: signal.selector, diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 5ef7ffc08f1e3..63044779ba239 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -22,7 +22,6 @@ import { stringifySelector } from '@isomorphic/selectorParser'; import { ManualPromise } from '@isomorphic/manualPromise'; import { isUnderTest } from '@utils/debug'; import { eventsHelper } from '@utils/eventsHelper'; -import { monotonicTime } from '@isomorphic/time'; import { BrowserContext } from './browserContext'; import { Debugger } from './debugger'; import { buildFullSelector, generateFrameSelector, metadataToCallLog } from './recorder/recorderUtils'; @@ -501,7 +500,6 @@ export class Recorder extends EventEmitter implements Instrume name: 'closePage', signals: [], }, - startTime: monotonicTime() }); this._filePrimaryURLChanged(); }); @@ -523,7 +521,6 @@ export class Recorder extends EventEmitter implements Instrume url: page.mainFrame().url(), signals: [], }, - startTime: monotonicTime() }); } this._filePrimaryURLChanged(); @@ -551,7 +548,6 @@ export class Recorder extends EventEmitter implements Instrume const actionInContext: actions.ActionInContext = { pageGuid: frame._page.guid, action, - startTime: monotonicTime(), }; return actionInContext; } @@ -562,12 +558,8 @@ export class Recorder extends EventEmitter implements Instrume 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(); - } + if (actionInContext.action.name !== 'openPage' && actionInContext.action.name !== 'closePage') + await performAction(progress, frame._page.mainFrame(), actionInContext); } private async _recordAction(progress: Progress, frame: Frame, action: actions.Action) { diff --git a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts index fb510fe657f26..542538a9a1b12 100644 --- a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts +++ b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts @@ -29,6 +29,7 @@ export interface ProcessorDelegate { export class RecorderSignalProcessor { private _delegate: ProcessorDelegate; private _lastAction: actions.ActionInContext | null = null; + private _lastActionTimestamp = 0; constructor(actionSink: ProcessorDelegate) { this._delegate = actionSink; @@ -36,11 +37,11 @@ export class RecorderSignalProcessor { addAction(actionInContext: actions.ActionInContext) { this._lastAction = actionInContext; + this._lastActionTimestamp = monotonicTime(); this._delegate.addAction(actionInContext); } signal(frame: Frame, signal: Signal) { - const timestamp = monotonicTime(); if (signal.name === 'navigation' && frame._page.mainFrame() === frame) { const lastAction = this._lastAction; const signalThreshold = isUnderTest() ? 500 : 5000; @@ -50,7 +51,7 @@ export class RecorderSignalProcessor { generateGoto = true; else if (lastAction.action.name !== 'click' && lastAction.action.name !== 'press' && lastAction.action.name !== 'fill') generateGoto = true; - else if (timestamp - lastAction.startTime > signalThreshold) + else if (monotonicTime() - this._lastActionTimestamp > signalThreshold) generateGoto = true; if (generateGoto) { @@ -61,8 +62,6 @@ export class RecorderSignalProcessor { url: frame.url(), signals: [], }, - startTime: timestamp, - endTime: timestamp, }); } return; @@ -71,7 +70,6 @@ export class RecorderSignalProcessor { this._delegate.addSignal({ pageGuid: frame._page.guid, signal, - timestamp, }); } } diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index fd66b37da8c9e..531e2939504a4 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; } @@ -84,14 +78,10 @@ export function collapseActions(actions: actions.ActionInContext[]): actions.Act const result: actions.ActionInContext[] = []; for (const action of actions) { const lastAction = result[result.length - 1]; - const shouldMerge = shouldMergeAction(action, lastAction); - if (!shouldMerge) { + if (shouldMergeAction(action, lastAction)) + result[result.length - 1] = action; + else result.push(action); - continue; - } - const startTime = result[result.length - 1].startTime; - result[result.length - 1] = action; - result[result.length - 1].startTime = startTime; } return result; } diff --git a/tests/library/debug-controller.spec.ts b/tests/library/debug-controller.spec.ts index 0bdb25cc24658..aa572a994d1f6 100644 --- a/tests/library/debug-controller.spec.ts +++ b/tests/library/debug-controller.spec.ts @@ -225,8 +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. - await expect.poll(() => events[events.length - 1]?.actions.length).toBe(2); + await expect(page.getByRole('button', { name: 'Saved' })).toBeVisible(); await page.getByRole('button', { name: 'Other' }).click(); // The signal is attached to the "Show" click, so the assertion renders right after it. diff --git a/tests/library/inspector/recorder-api.spec.ts b/tests/library/inspector/recorder-api.spec.ts index 568c09df57663..d71998b52c666 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', @@ -62,11 +61,10 @@ test('should click', async ({ context, browserName, platform, channel }) => { // Safari does not focus after a click: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/button#clicking_and_focus ariaSnapshot: (browserName === 'webkit' && (platform === 'darwin' || (platform === 'win32' && channel !== 'webkit-wsl'))) ? '- button "Submit" [ref=e2]' : '- button "Submit" [active] [ref=e2]', }), - startTime: expect.any(Number), }) ]); - 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 }) => { @@ -75,8 +73,7 @@ test('should double click', async ({ context, browserName, platform, channel }) await page.setContent(``); await page.getByRole('button', { name: 'Submit' }).dblclick(); - const clickActions = log.action('click'); - expect(clickActions).toEqual([ + await expect.poll(() => log.action('click')).toEqual([ expect.objectContaining({ action: expect.objectContaining({ name: 'click', @@ -86,11 +83,10 @@ test('should double click', async ({ context, browserName, platform, channel }) // Safari does not focus after a click: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/button#clicking_and_focus ariaSnapshot: (browserName === 'webkit' && (platform === 'darwin' || (platform === 'win32' && channel !== 'webkit-wsl'))) ? '- button "Submit" [ref=e2]' : '- button "Submit" [active] [ref=e2]', }), - startTime: expect.any(Number), }) ]); - expect(normalizeCode(clickActions[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).dblclick();`); + expect(normalizeCode(log.action('click')[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).dblclick();`); }); test('should right click', async ({ context, browserName, platform, channel }) => { @@ -99,8 +95,7 @@ test('should right click', async ({ context, browserName, platform, channel }) = await page.setContent(``); await page.getByRole('button', { name: 'Submit' }).click({ button: 'right' }); - const clickActions = log.action('click'); - expect(clickActions).toEqual([ + await expect.poll(() => log.action('click')).toEqual([ expect.objectContaining({ action: expect.objectContaining({ name: 'click', @@ -110,11 +105,10 @@ test('should right click', async ({ context, browserName, platform, channel }) = // Safari does not focus after a click: https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/button#clicking_and_focus ariaSnapshot: (browserName === 'webkit' && (platform === 'darwin' || (platform === 'win32' && channel !== 'webkit-wsl'))) ? '- button "Submit" [ref=e2]' : '- button "Submit" [active] [ref=e2]', }), - startTime: expect.any(Number), }) ]); - expect(normalizeCode(clickActions[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).click({ button: 'right' });`); + expect(normalizeCode(log.action('click')[0].code)).toEqual(`await page.getByRole('button', { name: 'Submit' }).click({ button: 'right' });`); }); test('should type', async ({ context }) => { @@ -124,8 +118,7 @@ test('should type', async ({ context }) => { await page.getByRole('textbox').pressSequentially('Hello'); - const fillActions = log.action('fill'); - expect(fillActions).toEqual([ + await expect.poll(() => log.action('fill')).toEqual([ expect.objectContaining({ action: expect.objectContaining({ name: 'fill', @@ -133,11 +126,10 @@ test('should type', async ({ context }) => { ref: 'e2', ariaSnapshot: '- textbox [active] [ref=e2]: Hello', }), - startTime: expect.any(Number), }) ]); - expect(normalizeCode(fillActions[0].code)).toEqual(`await page.getByRole('textbox').fill('Hello');`); + expect(normalizeCode(log.action('fill')[0].code)).toEqual(`await page.getByRole('textbox').fill('Hello');`); }); test('should disable recorder', async ({ context }) => { @@ -146,9 +138,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(); + // Make sure no extra action is recorded. + await page.waitForTimeout(2000); expect(log.action('click')).toHaveLength(2); }); From 956093bc264af81908a8665035f16678e115f180 Mon Sep 17 00:00:00 2001 From: Dmitry Gozman Date: Mon, 13 Jul 2026 14:19:47 +0100 Subject: [PATCH 2/2] chore(recorder): move signals from the action onto the action context MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signals (popup, download, dialog, expect) are not part of a recorded action, so drop `ActionBase.signals` and put `signals` on `ActionInContext` instead. Each `RecorderEvent.SignalAdded` consumer keeps its own `ActionInContext[]` and appends the signal to the matching entry, and codegen reads `actionInContext.signals`. Collapse where it is consumed: remove the batch `collapseActions()` / `shouldMergeAction()` helpers and instead replace-or-push the last action inline in each client (RecorderApp, DebugController, recorder-api). `ProgrammaticRecorderApp` no longer merges — it always emits a fresh actionAdded/signalAdded, so `actionUpdated` is gone from the protocol, and signalAdded now carries the regenerated last-action code instead of ''. `RecorderEventSink` now hands clients pure `Action`/`Signal` rather than the in-context wrappers. --- packages/injected/src/recorder/recorder.ts | 27 --------------- packages/isomorphic/codegen/actions.d.ts | 4 +-- packages/isomorphic/codegen/csharp.ts | 2 +- packages/isomorphic/codegen/java.ts | 2 +- packages/isomorphic/codegen/javascript.ts | 2 +- packages/isomorphic/codegen/jsonl.ts | 3 +- packages/isomorphic/codegen/language.ts | 6 ++-- packages/isomorphic/codegen/python.ts | 2 +- .../src/client/browserContext.ts | 11 +++---- .../playwright-core/src/client/channels.d.ts | 2 +- .../src/server/browserContext.ts | 2 +- .../playwright-core/src/server/channels.d.ts | 2 +- .../src/server/debugController.ts | 15 ++++++--- .../dispatchers/browserContextDispatcher.ts | 2 +- .../playwright-core/src/server/recorder.ts | 5 +-- .../src/server/recorder/recorderApp.ts | 28 +++++++++------- .../recorder/recorderSignalProcessor.ts | 2 +- .../src/server/recorder/recorderUtils.ts | 33 ------------------- packages/protocol/spec/browserContext.yml | 1 - packages/protocol/src/validator.ts | 2 +- tests/library/inspector/recorder-api.spec.ts | 25 ++++++++++---- 21 files changed, 70 insertions(+), 108 deletions(-) diff --git a/packages/injected/src/recorder/recorder.ts b/packages/injected/src/recorder/recorder.ts index ca6805ed7bd43..df0ca895dd041 100644 --- a/packages/injected/src/recorder/recorder.ts +++ b/packages/injected/src/recorder/recorder.ts @@ -171,7 +171,6 @@ class InspectTool implements RecorderTool { void this._recorder.recordAction({ name: 'assertVisible', selector, - signals: [], }); this._recorder.setMode('recording'); this._recorder.overlay?.flashToolSucceeded('assertingVisibility'); @@ -276,7 +275,6 @@ class RecordActionTool implements RecorderTool { this._performAction({ name: checkbox.checked ? 'check' : 'uncheck', selector: this._hoveredModel!.selector, - signals: [], }); return; } @@ -288,7 +286,6 @@ class RecordActionTool implements RecorderTool { name: 'click', selector: this._hoveredModel!.selector, position: positionForEvent(event), - signals: [], button: buttonForEvent(event), modifiers: modifiersForEvent(event), clickCount: event.detail @@ -317,7 +314,6 @@ class RecordActionTool implements RecorderTool { name: 'click', selector: this._hoveredModel!.selector, position: positionForEvent(event), - signals: [], button: buttonForEvent(event), modifiers: modifiersForEvent(event), clickCount: event.detail @@ -427,7 +423,6 @@ class RecordActionTool implements RecorderTool { this._recordAction({ name: 'setInputFiles', selector, - signals: [], files: [...((target as HTMLInputElement).files || [])].map(file => file.name), }); return; @@ -438,7 +433,6 @@ class RecordActionTool implements RecorderTool { name: 'fill', // must use hoveredModel instead of activeModel for it to work in webkit selector: this._hoveredModel!.selector, - signals: [], text: target.value, }); return; @@ -456,7 +450,6 @@ class RecordActionTool implements RecorderTool { this._recordAction({ name: 'fill', selector: this._activeModel!.selector, - signals: [], text: target.isContentEditable ? target.innerText : (target as HTMLInputElement).value, }); } @@ -467,7 +460,6 @@ class RecordActionTool implements RecorderTool { name: 'select', selector: this._activeModel!.selector, options: [...selectElement.selectedOptions].map(option => option.value), - signals: [] }); } } @@ -490,7 +482,6 @@ class RecordActionTool implements RecorderTool { this._performAction({ name: checkbox.checked ? 'uncheck' : 'check', selector: this._activeModel!.selector, - signals: [], }); return; } @@ -499,7 +490,6 @@ class RecordActionTool implements RecorderTool { this._performAction({ name: 'press', selector: this._activeModel!.selector, - signals: [], key: event.key, modifiers: modifiersForEvent(event), }); @@ -535,7 +525,6 @@ class RecordActionTool implements RecorderTool { name: 'click', selector: model.selector, position: actionPosition, - signals: [], button: 'left', modifiers: 0, clickCount: 0, @@ -547,7 +536,6 @@ class RecordActionTool implements RecorderTool { name: 'click', selector: model.selector, position: actionPosition, - signals: [], button: 'right', modifiers: 0, clickCount: 0, @@ -559,7 +547,6 @@ class RecordActionTool implements RecorderTool { name: 'click', selector: model.selector, position: actionPosition, - signals: [], button: 'left', modifiers: 0, clickCount: 2, @@ -571,7 +558,6 @@ class RecordActionTool implements RecorderTool { name: 'hover', selector: model.selector, position: actionPosition, - signals: [], }), }, { @@ -791,7 +777,6 @@ class JsonRecordActionTool implements RecorderTool { name: checkbox.checked ? 'check' : 'uncheck', selector, ref, - signals: [], ariaSnapshot, }); return; @@ -806,7 +791,6 @@ class JsonRecordActionTool implements RecorderTool { ref, ariaSnapshot, position: positionForEvent(event), - signals: [], button: buttonForEvent(event), modifiers: modifiersForEvent(event), clickCount: event.detail, @@ -832,7 +816,6 @@ class JsonRecordActionTool implements RecorderTool { ref, ariaSnapshot, position: positionForEvent(event), - signals: [], button: buttonForEvent(event), modifiers: modifiersForEvent(event), clickCount: event.detail, @@ -860,7 +843,6 @@ class JsonRecordActionTool implements RecorderTool { ref, ariaSnapshot, position: positionForEvent(event), - signals: [], button: 'right', modifiers: modifiersForEvent(event), clickCount: 1, @@ -877,7 +859,6 @@ class JsonRecordActionTool implements RecorderTool { selector, ref, ariaSnapshot, - signals: [], text: element.value, }); return; @@ -894,7 +875,6 @@ class JsonRecordActionTool implements RecorderTool { ref, selector, ariaSnapshot, - signals: [], text: element.isContentEditable ? element.innerText : (element as HTMLInputElement).value, }); return; @@ -908,7 +888,6 @@ class JsonRecordActionTool implements RecorderTool { ref, ariaSnapshot, options: [...selectElement.selectedOptions].map(option => option.value), - signals: [] }); return; } @@ -930,7 +909,6 @@ class JsonRecordActionTool implements RecorderTool { selector, ref, ariaSnapshot, - signals: [], }); return; } @@ -941,7 +919,6 @@ class JsonRecordActionTool implements RecorderTool { selector, ref, ariaSnapshot, - signals: [], key: event.key, modifiers: modifiersForEvent(event), }); @@ -1093,7 +1070,6 @@ class TextAssertionTool implements RecorderTool { return { name: 'assertChecked', selector, - signals: [], // Interestingly, inputElement.checked is reversed inside this event handler. checked: !(target as HTMLInputElement).checked, }; @@ -1101,7 +1077,6 @@ class TextAssertionTool implements RecorderTool { return { name: 'assertValue', selector, - signals: [], value: (target as (HTMLInputElement | HTMLTextAreaElement | HTMLSelectElement)).value, }; } @@ -1114,7 +1089,6 @@ class TextAssertionTool implements RecorderTool { return { name: 'assertSnapshot', selector: this._hoverHighlight.selector, - signals: [], ariaSnapshot: this._recorder.injectedScript.ariaSnapshot(target, { mode: 'codegen' }), }; } else { @@ -1126,7 +1100,6 @@ class TextAssertionTool implements RecorderTool { return { name: 'assertText', selector: this._hoverHighlight.selector, - signals: [], text: this._recorder.injectedScript.utils.elementText(this._textCache, target).normalized, substring: true, }; diff --git a/packages/isomorphic/codegen/actions.d.ts b/packages/isomorphic/codegen/actions.d.ts index 73c28873bd72c..d929ab2e482f9 100644 --- a/packages/isomorphic/codegen/actions.d.ts +++ b/packages/isomorphic/codegen/actions.d.ts @@ -36,7 +36,6 @@ export type ActionName = export type ActionBase = { name: ActionName, - signals: Signal[], ariaSnapshot?: string, }; @@ -155,7 +154,7 @@ export type DialogSignal = BaseSignal & { dialogAlias: string, }; -// An element that appeared since the previous action, asserted before this action runs. +// An element appeared as a result of an action. export type ExpectSignal = BaseSignal & { name: 'expect', selector: string, @@ -166,6 +165,7 @@ export type Signal = NavigationSignal | PopupSignal | DownloadSignal | DialogSig export type ActionInContext = { pageGuid: string; action: Action; + signals: Signal[]; }; export type SignalInContext = { diff --git a/packages/isomorphic/codegen/csharp.ts b/packages/isomorphic/codegen/csharp.ts index a3a57ba93d10c..183d4fde09080 100644 --- a/packages/isomorphic/codegen/csharp.ts +++ b/packages/isomorphic/codegen/csharp.ts @@ -91,7 +91,7 @@ export class CSharpLanguageGenerator implements LanguageGenerator { } const subject = pageAlias; - const signals = toSignalMap(action); + const signals = toSignalMap(actionInContext.signals); if (signals.dialog) { formatter.add(` void ${pageAlias}_Dialog${signals.dialog.dialogAlias}_EventHandler(object sender, IDialog dialog) diff --git a/packages/isomorphic/codegen/java.ts b/packages/isomorphic/codegen/java.ts index 3e318114e2681..9e2ab7eb3dd16 100644 --- a/packages/isomorphic/codegen/java.ts +++ b/packages/isomorphic/codegen/java.ts @@ -79,7 +79,7 @@ export class JavaLanguageGenerator implements LanguageGenerator { } const subject = pageAlias; - const signals = toSignalMap(action); + const signals = toSignalMap(actionInContext.signals); if (signals.dialog) { formatter.add(` ${pageAlias}.onceDialog(dialog -> { diff --git a/packages/isomorphic/codegen/javascript.ts b/packages/isomorphic/codegen/javascript.ts index 01002d3f5901a..3e424b2b6caff 100644 --- a/packages/isomorphic/codegen/javascript.ts +++ b/packages/isomorphic/codegen/javascript.ts @@ -67,7 +67,7 @@ export class JavaScriptLanguageGenerator implements LanguageGenerator { } const subject = pageAlias; - const signals = toSignalMap(action); + const signals = toSignalMap(actionInContext.signals); if (signals.dialog) { formatter.add(` ${pageAlias}.once('dialog', dialog => { diff --git a/packages/isomorphic/codegen/jsonl.ts b/packages/isomorphic/codegen/jsonl.ts index d31858594e5ed..87d509fb30c39 100644 --- a/packages/isomorphic/codegen/jsonl.ts +++ b/packages/isomorphic/codegen/jsonl.ts @@ -34,11 +34,12 @@ export class JsonlLanguageGenerator implements LanguageGenerator { const entry = { ...actionInContext.action, pageGuid: actionInContext.pageGuid, + signals: actionInContext.signals, locator, ariaSnapshot: undefined, }; const lines = [JSON.stringify(entry)]; - const expect = toSignalMap(actionInContext.action).expect; + const expect = toSignalMap(actionInContext.signals).expect; if (options.generateExpectSignal && expect) lines.push(this.generateAction(expectSignalAction(actionInContext, expect), options)); return lines.join('\n'); diff --git a/packages/isomorphic/codegen/language.ts b/packages/isomorphic/codegen/language.ts index ee71f98ceb3cf..f8bb131a367c3 100644 --- a/packages/isomorphic/codegen/language.ts +++ b/packages/isomorphic/codegen/language.ts @@ -34,8 +34,8 @@ export function expectSignalAction(actionInContext: actions.ActionInContext, sig action: { name: 'assertVisible', selector: signal.selector, - signals: [], }, + signals: [], }; } @@ -49,12 +49,12 @@ export function sanitizeDeviceOptions(device: any, options: BrowserContextOption return cleanedOptions; } -export function toSignalMap(action: actions.Action) { +export function toSignalMap(signals: actions.Signal[]) { let popup: actions.PopupSignal | undefined; let download: actions.DownloadSignal | undefined; let dialog: actions.DialogSignal | undefined; let expect: actions.ExpectSignal | undefined; - for (const signal of action.signals) { + for (const signal of signals) { if (signal.name === 'popup') popup = signal; else if (signal.name === 'download') diff --git a/packages/isomorphic/codegen/python.ts b/packages/isomorphic/codegen/python.ts index 2640b52c17e3a..9950d118a92c4 100644 --- a/packages/isomorphic/codegen/python.ts +++ b/packages/isomorphic/codegen/python.ts @@ -74,7 +74,7 @@ export class PythonLanguageGenerator implements LanguageGenerator { } const subject = pageAlias; - const signals = toSignalMap(action); + const signals = toSignalMap(actionInContext.signals); if (signals.dialog) formatter.add(` ${pageAlias}.once("dialog", lambda dialog: dialog.dismiss())`); diff --git a/packages/playwright-core/src/client/browserContext.ts b/packages/playwright-core/src/client/browserContext.ts index 70ada2e326876..37c2e4702bf0b 100644 --- a/packages/playwright-core/src/client/browserContext.ts +++ b/packages/playwright-core/src/client/browserContext.ts @@ -54,9 +54,8 @@ import type * as channels from './channels'; import type * as actions from '@isomorphic/codegen/actions'; interface RecorderEventSink { - actionAdded?(page: Page, actionInContext: actions.ActionInContext, code: string): void; - actionUpdated?(page: Page, actionInContext: actions.ActionInContext, code: string): void; - signalAdded?(page: Page, signal: actions.SignalInContext): void; + actionAdded?(page: Page, action: actions.Action, code: string): void; + signalAdded?(page: Page, signal: actions.Signal, code: string): void; } export class BrowserContext extends ChannelOwner implements api.BrowserContext { @@ -160,11 +159,9 @@ export class BrowserContext extends ChannelOwner this._channel.on('response', ({ response, page }) => this._onResponse(network.Response.from(response), Page.fromNullable(page))); this._channel.on('recorderEvent', ({ event, data, page, code }) => { if (event === 'actionAdded') - this._onRecorderEventSink?.actionAdded?.(Page.from(page), data as actions.ActionInContext, code); - else if (event === 'actionUpdated') - this._onRecorderEventSink?.actionUpdated?.(Page.from(page), data as actions.ActionInContext, code); + this._onRecorderEventSink?.actionAdded?.(Page.from(page), data as actions.Action, code); else if (event === 'signalAdded') - this._onRecorderEventSink?.signalAdded?.(Page.from(page), data as actions.SignalInContext); + this._onRecorderEventSink?.signalAdded?.(Page.from(page), data as actions.Signal, code); }); this._closedPromise = new Promise(f => this.once(Events.BrowserContext.Close, f)); diff --git a/packages/playwright-core/src/client/channels.d.ts b/packages/playwright-core/src/client/channels.d.ts index 6a2902e0f5982..16bc671e1b72a 100644 --- a/packages/playwright-core/src/client/channels.d.ts +++ b/packages/playwright-core/src/client/channels.d.ts @@ -1369,7 +1369,7 @@ export type BrowserContextResponseEvent = { page?: PageChannel, }; export type BrowserContextRecorderEventEvent = { - event: 'actionAdded' | 'actionUpdated' | 'signalAdded', + event: 'actionAdded' | 'signalAdded', data: any, page: PageChannel, code: string, diff --git a/packages/playwright-core/src/server/browserContext.ts b/packages/playwright-core/src/server/browserContext.ts index 96d5c2154c3d0..7ae3122835d52 100644 --- a/packages/playwright-core/src/server/browserContext.ts +++ b/packages/playwright-core/src/server/browserContext.ts @@ -83,7 +83,7 @@ export type BrowserContextEventMap = { [BrowserContextEvent.RequestFulfilled]: [request: network.Request]; [BrowserContextEvent.RequestContinued]: [request: network.Request]; [BrowserContextEvent.BeforeClose]: []; - [BrowserContextEvent.RecorderEvent]: [event: { event: 'actionAdded' | 'actionUpdated' | 'signalAdded', data: any, page: Page, code: string }]; + [BrowserContextEvent.RecorderEvent]: [event: { event: 'actionAdded' | 'signalAdded', data: any, page: Page, code: string }]; [BrowserContextEvent.PageClosed]: [page: Page]; [BrowserContextEvent.InternalFrameNavigatedToNewDocument]: [frame: frames.Frame]; [BrowserContextEvent.FrameAttached]: [frame: frames.Frame]; diff --git a/packages/playwright-core/src/server/channels.d.ts b/packages/playwright-core/src/server/channels.d.ts index 9d55ce6535a64..b7dfb6b1ef11a 100644 --- a/packages/playwright-core/src/server/channels.d.ts +++ b/packages/playwright-core/src/server/channels.d.ts @@ -1372,7 +1372,7 @@ export type BrowserContextResponseEvent = { page?: PageChannel, }; export type BrowserContextRecorderEventEvent = { - event: 'actionAdded' | 'actionUpdated' | 'signalAdded', + event: 'actionAdded' | 'signalAdded', data: any, page: PageChannel, code: string, diff --git a/packages/playwright-core/src/server/debugController.ts b/packages/playwright-core/src/server/debugController.ts index d09d972131923..7b519e552453d 100644 --- a/packages/playwright-core/src/server/debugController.ts +++ b/packages/playwright-core/src/server/debugController.ts @@ -23,7 +23,6 @@ import { generateCode } from '@isomorphic/codegen/language'; import { JavaScriptLanguageGenerator } from '@isomorphic/codegen/javascript'; import { SdkObject, createInstrumentation } from './instrumentation'; import { Recorder, RecorderEvent } from './recorder'; -import { collapseActions } from './recorder/recorderUtils'; import type { Language } from '@isomorphic/locatorGenerators'; import type { BrowserContext } from './browserContext'; @@ -188,8 +187,7 @@ function wireListeners(recorder: Recorder, debugController: DebugController) { const languageGenerator = new JavaScriptLanguageGenerator(/* isPlaywrightTest */true); const actionsChanged = () => { - const aa = collapseActions(actions); - const { header, footer, text, actionTexts } = generateCode(aa, languageGenerator, { + const { header, footer, text, actionTexts } = generateCode(actions, languageGenerator, { browserName: 'chromium', launchOptions: {}, contextOptions: {}, @@ -209,13 +207,20 @@ function wireListeners(recorder: Recorder, debugController: DebugController) { debugController.emit(DebugController.Events.SetModeRequested, { mode }); }); recorder.on(RecorderEvent.ActionAdded, (action: actions.ActionInContext) => { - actions.push(action); + const last = actions[actions.length - 1]; + const shouldReplace = !!last && last.pageGuid === action.pageGuid && ( + (action.action.name === 'navigate' && last.action.name === 'navigate') || + (action.action.name === 'fill' && last.action.name === 'fill' && action.action.selector === last.action.selector)); + if (shouldReplace) + actions[actions.length - 1] = action; + else + actions.push(action); actionsChanged(); }); recorder.on(RecorderEvent.SignalAdded, (signal: actions.SignalInContext) => { const lastAction = actions.findLast(a => a.pageGuid === signal.pageGuid); if (lastAction) - lastAction.action.signals.push(signal.signal); + lastAction.signals.push(signal.signal); actionsChanged(); }); } diff --git a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts index e8da143bfbf18..3b15c92129c3a 100644 --- a/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts @@ -193,7 +193,7 @@ export class BrowserContextDispatcher extends Dispatcher { + this.addObjectListener(BrowserContext.Events.RecorderEvent, ({ event, data, page, code }: { event: 'actionAdded' | 'signalAdded', data: any, page: Page, code: string }) => { this._dispatchEvent('recorderEvent', { event, data, code, page: PageDispatcher.from(this, page) }); }); } diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts index 63044779ba239..ecb586e4b9e36 100644 --- a/packages/playwright-core/src/server/recorder.ts +++ b/packages/playwright-core/src/server/recorder.ts @@ -498,8 +498,8 @@ export class Recorder extends EventEmitter implements Instrume pageGuid: page.guid, action: { name: 'closePage', - signals: [], }, + signals: [], }); this._filePrimaryURLChanged(); }); @@ -519,8 +519,8 @@ export class Recorder extends EventEmitter implements Instrume action: { name: 'openPage', url: page.mainFrame().url(), - signals: [], }, + signals: [], }); } this._filePrimaryURLChanged(); @@ -548,6 +548,7 @@ export class Recorder extends EventEmitter implements Instrume const actionInContext: actions.ActionInContext = { pageGuid: frame._page.guid, action, + signals: [], }; return actionInContext; } diff --git a/packages/playwright-core/src/server/recorder/recorderApp.ts b/packages/playwright-core/src/server/recorder/recorderApp.ts index e61603a964c64..1291d61f95a69 100644 --- a/packages/playwright-core/src/server/recorder/recorderApp.ts +++ b/packages/playwright-core/src/server/recorder/recorderApp.ts @@ -26,7 +26,6 @@ import { syncLocalStorageWithSettings } from '../launchApp'; import { launchApp } from '../launchApp'; import { nullProgress, ProgressController } from '../progress'; import { ThrottledFile } from './throttledFile'; -import { collapseActions, shouldMergeAction } from './recorderUtils'; import { Recorder, RecorderEvent } from '../recorder'; import { BrowserContext } from '../browserContext'; @@ -283,14 +282,21 @@ export class RecorderApp { } private _onActionAdded(action: actions.ActionInContext) { - this._actions.push(action); + const last = this._actions[this._actions.length - 1]; + const shouldReplace = !!last && last.pageGuid === action.pageGuid && ( + (action.action.name === 'navigate' && last.action.name === 'navigate') || + (action.action.name === 'fill' && last.action.name === 'fill' && action.action.selector === last.action.selector)); + if (shouldReplace) + this._actions[this._actions.length - 1] = action; + else + this._actions.push(action); this._updateActions('reveal'); } private _onSignalAdded(signal: actions.SignalInContext) { const lastAction = this._actions.findLast(a => a.pageGuid === signal.pageGuid); if (lastAction) - lastAction.action.signals.push(signal.signal); + lastAction.signals.push(signal.signal); this._updateActions(); } @@ -315,11 +321,10 @@ export class RecorderApp { private _updateActions(reveal?: 'reveal') { const recorderSources = []; - const actions = collapseActions(this._actions); let revealSourceId: string | undefined; for (const languageGenerator of languageSet()) { - const { header, footer, actionTexts, text } = generateCode(actions, languageGenerator, this._languageGeneratorOptions); + const { header, footer, actionTexts, text } = generateCode(this._actions, languageGenerator, this._languageGeneratorOptions); const source: Source = { isRecorded: true, label: languageGenerator.name, @@ -373,18 +378,19 @@ export class ProgrammaticRecorderApp { const page = findPageByGuid(inspectedContext, action.pageGuid); if (!page) return; - const code = languageGenerator.generateAction(action, languageGeneratorOptions); - if (!lastAction || !shouldMergeAction(action, lastAction)) - inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'actionAdded', data: action, page, code }); - else - inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'actionUpdated', data: action, page, code }); lastAction = action; + const code = languageGenerator.generateAction(action, languageGeneratorOptions); + inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'actionAdded', data: action.action, page, code }); }); recorder.on(RecorderEvent.SignalAdded, signal => { const page = findPageByGuid(inspectedContext, signal.pageGuid); if (!page) return; - inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'signalAdded', data: signal, page, code: '' }); + if (lastAction) + lastAction.signals.push(signal.signal); + // Regenerate the last action's code, now including this signal. + const code = lastAction ? languageGenerator.generateAction(lastAction, languageGeneratorOptions) : ''; + inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'signalAdded', data: signal.signal, page, code }); }); } } diff --git a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts index 542538a9a1b12..b06a00994141a 100644 --- a/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts +++ b/packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts @@ -60,8 +60,8 @@ export class RecorderSignalProcessor { action: { name: 'navigate', url: frame.url(), - signals: [], }, + signals: [], }); } return; diff --git a/packages/playwright-core/src/server/recorder/recorderUtils.ts b/packages/playwright-core/src/server/recorder/recorderUtils.ts index 531e2939504a4..f08e02394699a 100644 --- a/packages/playwright-core/src/server/recorder/recorderUtils.ts +++ b/packages/playwright-core/src/server/recorder/recorderUtils.ts @@ -21,7 +21,6 @@ import { quoteCSSAttributeValue } from '@isomorphic/stringUtils'; import { Frame } from '../frames'; import type { CallMetadata } from '../instrumentation'; -import type * as actions from '@isomorphic/codegen/actions'; import type { CallLog, CallLogStatus } from '@recorder/recorderTypes'; import type { Progress } from '../progress'; @@ -54,38 +53,6 @@ export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus) return callLog; } -function isSameAction(a: actions.ActionInContext, b: actions.ActionInContext): boolean { - return a.action.name === b.action.name && a.pageGuid === b.pageGuid; -} - -function isSameSelector(action: actions.ActionInContext, lastAction: actions.ActionInContext): boolean { - return 'selector' in action.action && 'selector' in lastAction.action && action.action.selector === lastAction.action.selector; -} - -export function shouldMergeAction(action: actions.ActionInContext, lastAction: actions.ActionInContext | undefined): boolean { - if (!lastAction) - return false; - switch (action.action.name) { - case 'fill': - return isSameAction(action, lastAction) && isSameSelector(action, lastAction); - case 'navigate': - return isSameAction(action, lastAction); - } - return false; -} - -export function collapseActions(actions: actions.ActionInContext[]): actions.ActionInContext[] { - const result: actions.ActionInContext[] = []; - for (const action of actions) { - const lastAction = result[result.length - 1]; - if (shouldMergeAction(action, lastAction)) - result[result.length - 1] = action; - else - result.push(action); - } - return result; -} - export async function generateFrameSelector(progress: Progress, frame: Frame): Promise { const selectorPromises: Promise[] = []; progress.setAllowConcurrentOrNestedRaces(true); diff --git a/packages/protocol/spec/browserContext.yml b/packages/protocol/spec/browserContext.yml index 753e9d6a2c225..0285c0d3c4051 100644 --- a/packages/protocol/spec/browserContext.yml +++ b/packages/protocol/spec/browserContext.yml @@ -448,7 +448,6 @@ BrowserContext: type: enum literals: - actionAdded - - actionUpdated - signalAdded data: json page: Page diff --git a/packages/protocol/src/validator.ts b/packages/protocol/src/validator.ts index 6e102e9cfb6fa..f185273b2429f 100644 --- a/packages/protocol/src/validator.ts +++ b/packages/protocol/src/validator.ts @@ -741,7 +741,7 @@ scheme.BrowserContextResponseEvent = tObject({ page: tOptional(tChannel(['Page'])), }); scheme.BrowserContextRecorderEventEvent = tObject({ - event: tEnum(['actionAdded', 'actionUpdated', 'signalAdded']), + event: tEnum(['actionAdded', 'signalAdded']), data: tAny, page: tChannel(['Page']), code: tString, diff --git a/tests/library/inspector/recorder-api.spec.ts b/tests/library/inspector/recorder-api.spec.ts index d71998b52c666..f0f3bee63ee54 100644 --- a/tests/library/inspector/recorder-api.spec.ts +++ b/tests/library/inspector/recorder-api.spec.ts @@ -19,15 +19,28 @@ import { test, expect } from './inspectorTest'; import type { Page } from '@playwright/test'; import type * as actions from '@isomorphic/codegen/actions'; -class RecorderLog { - actions: (actions.ActionInContext & { code: string })[] = []; +type LoggedAction = { page: Page, action: actions.Action, signals: actions.Signal[], code: string }; - actionAdded(page: Page, actionInContext: actions.ActionInContext, code: string): void { - this.actions.push({ ...actionInContext, code }); +class RecorderLog { + actions: LoggedAction[] = []; + + actionAdded(page: Page, action: actions.Action, code: string): void { + const last = this.actions[this.actions.length - 1]; + const shouldReplace = !!last && last.page === page && ( + (action.name === 'navigate' && last.action.name === 'navigate') || + (action.name === 'fill' && last.action.name === 'fill' && action.selector === last.action.selector)); + if (shouldReplace) + this.actions[this.actions.length - 1] = { page, action, signals: [], code }; + else + this.actions.push({ page, action, signals: [], code }); } - actionUpdated(page: Page, actionInContext: actions.ActionInContext, code: string): void { - this.actions[this.actions.length - 1] = { ...actionInContext, code }; + signalAdded(page: Page, signal: actions.Signal, code: string): void { + const last = this.actions[this.actions.length - 1]; + if (last) { + last.signals.push(signal); + last.code = code; + } } }