fix(codegen): do not commit last action on mouse move (#6252)

On a slow page that does a lot of things before navigating upon click,
it is common to move mouse away from the click point. Previously,
we would commit the click action and record a `page.goto()` for the
navigation. Now we attribute any signals, even after accidental mouse move,
to the previous action, in the 5-seconds time window.

src/server/supplements/injected/recorder.ts

@@ -24,7 +24,6 @@ declare global {
   interface Window {
     _playwrightRecorderPerformAction: (action: actions.Action) => Promise<void>;
     _playwrightRecorderRecordAction: (action: actions.Action) => Promise<void>;
-    _playwrightRecorderCommitAction: () => Promise<void>;
     _playwrightRecorderState: () => Promise<UIState>;
     _playwrightRecorderSetSelector: (selector: string) => Promise<void>;
     _playwrightRefreshOverlay: () => void;
@@ -335,15 +334,14 @@ export class Recorder {
     if (this._hoveredElement === target)
       return;
     this._hoveredElement = target;
-    // Mouse moved -> mark last action as committed via committing a commit action.
-    this._commitActionAndUpdateModelForHoveredElement();
+    this._updateModelForHoveredElement();
   }
 
   private _onMouseLeave(event: MouseEvent) {
     // Leaving iframe.
     if (this._deepEventTarget(event).nodeType === Node.DOCUMENT_NODE) {
       this._hoveredElement = null;
-      this._commitActionAndUpdateModelForHoveredElement();
+      this._updateModelForHoveredElement();
     }
   }
 
@@ -355,7 +353,7 @@ export class Recorder {
       (window as any)._highlightUpdatedForTest(result ? result.selector : null);
   }
 
-  private _commitActionAndUpdateModelForHoveredElement() {
+  private _updateModelForHoveredElement() {
     if (!this._hoveredElement) {
       this._hoveredModel = null;
       this._updateHighlight();
@@ -365,7 +363,6 @@ export class Recorder {
     const { selector, elements } = generateSelector(this._injectedScript, hoveredElement);
     if ((this._hoveredModel && this._hoveredModel.selector === selector) || this._hoveredElement !== hoveredElement)
       return;
-    window._playwrightRecorderCommitAction();
     this._hoveredModel = selector ? { selector, elements } : null;
     this._updateHighlight();
     if ((window as any)._highlightUpdatedForTest)
@@ -571,7 +568,7 @@ export class Recorder {
     this._performingAction = false;
 
     // Action could have changed DOM, update hovered model selectors.
-    this._commitActionAndUpdateModelForHoveredElement();
+    this._updateModelForHoveredElement();
     // If that was a keyboard action, it similarly requires new selectors for active model.
     this._onFocus();
 

src/server/supplements/recorder/codeGenerator.ts

@@ -50,6 +50,7 @@ export class CodeGenerator extends EventEmitter {
     this._currentAction = null;
     this._lastAction = null;
     this._actions = [];
+    this.emit('change');
   }
 
   setEnabled(enabled: boolean) {
@@ -99,12 +100,10 @@ export class CodeGenerator extends EventEmitter {
           return;
         }
       }
-      for (const name of ['check', 'uncheck']) {
-        // Check and uncheck erase click.
-        if (lastAction && action.name === name && lastAction.name === 'click') {
-          if ((action as any).selector === (lastAction as any).selector)
-            eraseLastAction = true;
-        }
+      // Check and uncheck erase click.
+      if (lastAction && (action.name === 'check' || action.name === 'uncheck') && lastAction.name === 'click') {
+        if (action.selector === lastAction.selector)
+          eraseLastAction = true;
       }
     }
 

src/server/supplements/recorderSupplement.ts

@@ -199,10 +199,6 @@ export class RecorderSupplement {
     await this._context.exposeBinding('_playwrightRecorderRecordAction', false,
         (source: BindingSource, action: actions.Action) => this._recordAction(source.frame, action));
 
-    // Commits last action so that no further signals are added to it.
-    await this._context.exposeBinding('_playwrightRecorderCommitAction', false,
-        (source: BindingSource, action: actions.Action) => this._generator.commitLastAction());
-
     await this._context.exposeBinding('_playwrightRecorderState', false, source => {
       let snapshotUrl: string | undefined;
       let actionSelector = this._highlightedSelector;
@@ -334,6 +330,9 @@ export class RecorderSupplement {
   }
 
   private async _performAction(frame: Frame, action: actions.Action) {
+    // Commit last action so that no further signals are added to it.
+    this._generator.commitLastAction();
+
     const page = frame._page;
     const actionInContext: ActionInContext = {
       pageAlias: this._pageAliases.get(page)!,
@@ -364,6 +363,7 @@ export class RecorderSupplement {
       return;
     }
     const timer = setTimeout(() => {
+      // Commit the action after 5 seconds so that no further signals are added to it.
       actionInContext.committed = true;
       this._timers.delete(timer);
     }, 5000);
@@ -372,6 +372,9 @@ export class RecorderSupplement {
   }
 
   private async _recordAction(frame: Frame, action: actions.Action) {
+    // Commit last action so that no further signals are added to it.
+    this._generator.commitLastAction();
+
     this._generator.addAction({
       pageAlias: this._pageAliases.get(frame._page)!,
       ...describeFrame(frame),

tests/cli/cli-codegen-2.spec.ts

@@ -593,4 +593,29 @@ await page.GetFrame(url: \"http://localhost:${server.PORT}/frames/frame.html\").
     await page.goto(httpServer.PREFIX + '/page2.html');
     await recorder.waitForOutput('<javascript>', `await page.goto('${httpServer.PREFIX}/page2.html');`);
   });
+
+  test('should record slow navigation signal after mouse move', async ({ page, openRecorder, server }) => {
+    const recorder = await openRecorder();
+    await recorder.setContentAndWait(`
+    <script>
+      async function onClick() {
+        await new Promise(f => setTimeout(f, 100));
+        await window.letTheMouseMove();
+        window.location = ${JSON.stringify(server.EMPTY_PAGE)};
+      }
+    </script>
+    <button onclick="onClick()">Click me</button>
+    `);
+    await page.exposeBinding('letTheMouseMove', async () => {
+      await page.mouse.move(200, 200);
+    });
+
+    const [, sources] = await Promise.all([
+      // This will click, finish the click, then mouse move, then navigate.
+      page.click('button'),
+      recorder.waitForOutput('<javascript>', 'waitForNavigation'),
+    ]);
+
+    expect(sources.get('<javascript>').text).toContain(`page.waitForNavigation(/*{ url: '${server.EMPTY_PAGE}' }*/)`);
+  });
 });