chore(recorder): move fill coalescing back to collapseActions

packages/playwright-core/src/client/browserContext.ts

@@ -58,6 +58,7 @@ import type * as actions from '@isomorphic/codegen/actions';
 
 interface RecorderEventSink {
   actionAdded?(page: Page, action: actions.Action, code: string): void;
+  actionUpdated?(page: Page, action: actions.Action, code: string): void;
   signalAdded?(page: Page, signal: actions.Signal, code: string): void;
 }
 
@@ -170,6 +171,8 @@ export class BrowserContext extends ChannelOwner<channels.BrowserContextChannel>
     this._channel.on('recorderEvent', ({ event, data, page, code }) => {
       if (event === 'actionAdded')
         this._onRecorderEventSink?.actionAdded?.(Page.from(page), data as actions.Action, code);
+      else if (event === 'actionUpdated')
+        this._onRecorderEventSink?.actionUpdated?.(Page.from(page), data as actions.Action, code);
       else if (event === 'signalAdded')
         this._onRecorderEventSink?.signalAdded?.(Page.from(page), data as actions.Signal, code);
     });

packages/playwright-core/src/client/channels.d.ts

@@ -1327,7 +1327,7 @@ export type BrowserContextResponseEvent = {
   page?: PageChannel,
 };
 export type BrowserContextRecorderEventEvent = {
-  event: 'actionAdded' | 'signalAdded',
+  event: 'actionAdded' | 'actionUpdated' | 'signalAdded',
   data: any,
   page: PageChannel,
   code: string,

packages/playwright-core/src/server/browserContext.ts

@@ -82,7 +82,7 @@ export type BrowserContextEventMap = {
   [BrowserContextEvent.RequestFulfilled]: [request: network.Request];
   [BrowserContextEvent.RequestContinued]: [request: network.Request];
   [BrowserContextEvent.BeforeClose]: [];
-  [BrowserContextEvent.RecorderEvent]: [event: { event: 'actionAdded' | 'signalAdded', data: any, page: Page, code: string }];
+  [BrowserContextEvent.RecorderEvent]: [event: { event: 'actionAdded' | 'actionUpdated' | 'signalAdded', data: any, page: Page, code: string }];
   [BrowserContextEvent.PageClosed]: [page: Page];
   [BrowserContextEvent.InternalFrameNavigatedToNewDocument]: [frame: frames.Frame];
   [BrowserContextEvent.FrameAttached]: [frame: frames.Frame];

packages/playwright-core/src/server/channels.d.ts

@@ -1328,7 +1328,7 @@ export type BrowserContextResponseEvent = {
   page?: PageChannel,
 };
 export type BrowserContextRecorderEventEvent = {
-  event: 'actionAdded' | 'signalAdded',
+  event: 'actionAdded' | 'actionUpdated' | 'signalAdded',
   data: any,
   page: PageChannel,
   code: string,

packages/playwright-core/src/server/debugController.ts

@@ -23,6 +23,7 @@ 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';
@@ -187,7 +188,7 @@ function wireListeners(recorder: Recorder, debugController: DebugController) {
   const languageGenerator = new JavaScriptLanguageGenerator(/* isPlaywrightTest */true);
 
   const actionsChanged = () => {
-    const { header, footer, text, actionTexts } = generateCode(actions, languageGenerator, {
+    const { header, footer, text, actionTexts } = generateCode(collapseActions(actions), languageGenerator, {
       browserName: 'chromium',
       launchOptions: {},
       contextOptions: {},

packages/playwright-core/src/server/dispatchers/browserContextDispatcher.ts

@@ -197,7 +197,7 @@ export class BrowserContextDispatcher extends Dispatcher<BrowserContext, channel
         page: PageDispatcher.fromNullable(this, request.frame()?._page.initializedOrUndefined()),
       });
     });
-    this.addObjectListener(BrowserContext.Events.RecorderEvent, ({ event, data, page, code }: { event: 'actionAdded' | 'signalAdded', data: any, page: Page, code: string }) => {
+    this.addObjectListener(BrowserContext.Events.RecorderEvent, ({ event, data, page, code }: { event: 'actionAdded' | 'actionUpdated' | 'signalAdded', data: any, page: Page, code: string }) => {
       this._dispatchEvent('recorderEvent', { event, data, code, page: PageDispatcher.from(this, page) });
     });
   }

packages/playwright-core/src/server/recorder/recorderApp.ts

@@ -27,6 +27,7 @@ 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';
 
@@ -319,9 +320,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(this._actions, languageGenerator, this._languageGeneratorOptions);
+      const { header, footer, actionTexts, text } = generateCode(actions, languageGenerator, this._languageGeneratorOptions);
       const source: Source = {
         isRecorded: true,
         label: languageGenerator.name,
@@ -379,10 +381,16 @@ export class ProgrammaticRecorderApp {
         const page = findPageByGuid(inspectedContext, actionInContext.pageGuid);
         if (!page)
           return;
+        let event: 'actionAdded' | 'actionUpdated' = 'actionAdded';
+        if (shouldMergeAction(actionInContext, lastAction)) {
+          event = 'actionUpdated';
+          // Signals already reported for the superseded action still belong to this one.
+          actionInContext.signals.unshift(...lastAction!.signals);
+        }
         lastAction = actionInContext;
         lastActionPage = page;
         const code = languageGenerator.generateAction(actionInContext, languageGeneratorOptions);
-        inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event: 'actionAdded', data: actionInContext.action, page, code });
+        inspectedContext.emit(BrowserContext.Events.RecorderEvent, { event, data: actionInContext.action, page, code });
       }),
       eventsHelper.addEventListener(recorder, RecorderEvent.SignalAdded, signalInContext => {
         const page = findPageByGuid(inspectedContext, signalInContext.pageGuid);

packages/playwright-core/src/server/recorder/recorderSignalProcessor.ts

@@ -27,7 +27,7 @@ export interface ProcessorDelegate {
 }
 
 // How long an action is held back, waiting for a superseding action to merge with it:
-// a double click after a click, another keystroke after a fill, another navigation.
+// a double click after a click, another navigation after a navigation.
 const kActionBufferTimeout = 500;
 
 type BufferedSignal = { frame: Frame, signal: Signal, timestamp: number };
@@ -83,7 +83,7 @@ export class RecorderSignalProcessor {
 
   private _shouldBuffer(actionInContext: actions.ActionInContext): boolean {
     const action = actionInContext.action;
-    return (action.name === 'click' && action.button === 'left') || action.name === 'fill' || action.name === 'navigate';
+    return (action.name === 'click' && action.button === 'left') || action.name === 'navigate';
   }
 
   private _supersedes(actionInContext: actions.ActionInContext, pending: actions.ActionInContext): boolean {
@@ -94,9 +94,6 @@ export class RecorderSignalProcessor {
     // A higher click count on the same target is a double (or triple) click.
     if (action.name === 'click' && pendingAction.name === 'click')
       return action.selector === pendingAction.selector && action.clickCount > pendingAction.clickCount;
-    // Another keystroke into the same field supersedes the previous value.
-    if (action.name === 'fill' && pendingAction.name === 'fill')
-      return action.selector === pendingAction.selector;
     // Another navigation on the same page supersedes the previous url.
     if (action.name === 'navigate' && pendingAction.name === 'navigate')
       return true;

packages/playwright-core/src/server/recorder/recorderUtils.ts

@@ -26,6 +26,7 @@ import type { CallMetadata } from '../instrumentation';
 import type { CallLog, CallLogStatus } from '@recorder/recorderTypes';
 import type { Progress } from '../progress';
 import type { Language } from '@isomorphic/locatorGenerators';
+import type * as actions from '@isomorphic/codegen/actions';
 
 function buildFullSelector(framePath: string[], selector: string) {
   return [...framePath, selector].join(' >> internal:control=enter-frame >> ');
@@ -84,6 +85,27 @@ export function metadataToCallLog(metadata: CallMetadata, status: CallLogStatus,
   return callLog;
 }
 
+export function shouldMergeAction(actionInContext: actions.ActionInContext, lastAction: actions.ActionInContext | undefined): boolean {
+  if (!lastAction)
+    return false;
+  const action = actionInContext.action;
+  const last = lastAction.action;
+  return action.name === 'fill' && last.name === 'fill'
+    && actionInContext.pageGuid === lastAction.pageGuid
+    && action.selector === last.selector;
+}
+
+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, signals: [...lastAction.signals, ...action.signals] };
+    else
+      result.push(action);
+  }
+  return result;
+}
 
 async function generateFrameSelector(progress: Progress, frame: Frame, timeout: number): Promise<string[]> {
   const selectorPromises: Promise<string>[] = [];

packages/playwright-core/src/tools/backend/browserContextEx.ts

@@ -19,6 +19,7 @@ import type * as playwrightTypes from '../../..';
 
 export type RecorderEventSink = {
   actionAdded?(page: playwrightTypes.Page, action: actions.Action, code: string): void;
+  actionUpdated?(page: playwrightTypes.Page, action: actions.Action, code: string): void;
   signalAdded?(page: playwrightTypes.Page, signal: actions.Signal, code: string): void;
 };
 

packages/playwright-core/src/tools/backend/context.ts

@@ -253,6 +253,12 @@ export class Context {
       actionAdded: (page, action, code) => {
         recordedActions.push(code);
       },
+      actionUpdated: (page, action, code) => {
+        if (recordedActions.length)
+          recordedActions[recordedActions.length - 1] = code;
+        else
+          recordedActions.push(code);
+      },
       signalAdded: (page, signal, code) => {
         if (recordedActions.length && code)
           recordedActions[recordedActions.length - 1] = code;

packages/protocol/spec/browserContext.yml

@@ -433,6 +433,7 @@ BrowserContext:
           type: enum
           literals:
           - actionAdded
+          - actionUpdated
           - signalAdded
         data: json
         page: Page

packages/protocol/src/validator.ts

@@ -715,7 +715,7 @@ scheme.BrowserContextResponseEvent = tObject({
   page: tOptional(tChannel(['Page'])),
 });
 scheme.BrowserContextRecorderEventEvent = tObject({
-  event: tEnum(['actionAdded', 'signalAdded']),
+  event: tEnum(['actionAdded', 'actionUpdated', 'signalAdded']),
   data: tAny,
   page: tChannel(['Page']),
   code: tString,

tests/library/inspector/recorder-api.spec.ts

@@ -28,6 +28,10 @@ class RecorderLog {
     this.actions.push({ action, code });
   }
 
+  actionUpdated(page: Page, action: actions.Action, code: string): void {
+    this.actions[this.actions.length - 1] = { action, code };
+  }
+
   signalAdded(page: Page, signal: actions.Signal, code: string): void {
     this.signals.push({ signal, code });
   }