From 73ffaf65d75b2378168ac5a11eb37cced03ff6ea Mon Sep 17 00:00:00 2001
From: Rui Figueira <rui.figueira@gmail.com>
Date: Mon, 4 Mar 2024 20:31:03 +0000
Subject: [PATCH] fix(codegen): fill action prevents omnibox navigation
 recording (#29790)

This PR is a fix proposal for a bug when trying to record a omnibox
navigation after a recorded action (e.g., `fill`).

The following test, included in this PR, reproduces the problem:

```ts
  test('should record omnibox navigations after recordAction', async ({ page, openRecorder, server }) => {
    const recorder = await openRecorder();
    await recorder.setContentAndWait(`<textarea></textarea>`);
    await Promise.all([
      recorder.waitForOutput('JavaScript', 'fill'),
      page.locator('textarea').fill('Hello world'),
    ]);
    // for performed actions, 5 seconds is the time needed to ensure they are committed
    await page.waitForTimeout(5000);
    await page.goto(server.PREFIX + `/empty.html`);
    await recorder.waitForOutput('JavaScript', `await page.goto('${server.PREFIX}/empty.html');`);
  });
```

After performed actions (e.g., `click`), it successfully records the
navigation as long as there's at least a 5 sec. gap between both
actions. That happens because after that 5 sec. interval the performed
action is automatically commited and therefore the navigation is not
stored as a signal of that action.

The proposed fix for recorded actions also forces that action to be
automatically commited after 5 sec (for testing, I'm using 500ms to
speed up the test execution).
---
 .../playwright-core/src/server/recorder.ts    | 17 ++++++++-----
 tests/library/inspector/cli-codegen-1.spec.ts | 24 +++++++++++++++++++
 2 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/packages/playwright-core/src/server/recorder.ts b/packages/playwright-core/src/server/recorder.ts
index 997e1244d2..d5f84ba625 100644
--- a/packages/playwright-core/src/server/recorder.ts
+++ b/packages/playwright-core/src/server/recorder.ts
@@ -626,13 +626,8 @@ class ContextRecorder extends EventEmitter {
       callMetadata.endTime = monotonicTime();
       await frame.instrumentation.onAfterCall(frame, callMetadata);
 
-      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);
+      this._setCommittedAfterTimeout(actionInContext);
       this._generator.didPerformAction(actionInContext);
-      this._timers.add(timer);
     };
 
     const kActionTimeout = 5000;
@@ -664,9 +659,19 @@ class ContextRecorder extends EventEmitter {
       frame: frameDescription,
       action
     };
+    this._setCommittedAfterTimeout(actionInContext);
     this._generator.addAction(actionInContext);
   }
 
+  private _setCommittedAfterTimeout(actionInContext: ActionInContext) {
+    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);
+    }, isUnderTest() ? 500 : 5000);
+    this._timers.add(timer);
+  }
+
   private _onFrameNavigated(frame: Frame, page: Page) {
     const pageAlias = this._pageAliases.get(page);
     this._generator.signal(pageAlias!, frame, { name: 'navigation', url: frame.url() });
diff --git a/tests/library/inspector/cli-codegen-1.spec.ts b/tests/library/inspector/cli-codegen-1.spec.ts
index a0c5596eea..a54ae4d421 100644
--- a/tests/library/inspector/cli-codegen-1.spec.ts
+++ b/tests/library/inspector/cli-codegen-1.spec.ts
@@ -817,4 +817,28 @@ await page.GetByRole(AriaRole.Slider).FillAsync("10");`);
     expect.soft(sources.get('C#')!.text).toContain(`
 await page.GetByRole(AriaRole.Button, new() { Name = "Submit" }).ClickAsync();`);
   });
+
+  test('should record omnibox navigations after performAction', async ({ page, openRecorder, server }) => {
+    const recorder = await openRecorder();
+    await recorder.setContentAndWait(`<button>Submit</button>`);
+    await Promise.all([
+      recorder.waitForOutput('JavaScript', 'click'),
+      page.locator('button').click(),
+    ]);
+    await page.waitForTimeout(500);
+    await page.goto(server.PREFIX + `/empty.html`);
+    await recorder.waitForOutput('JavaScript', `await page.goto('${server.PREFIX}/empty.html');`);
+  });
+
+  test('should record omnibox navigations after recordAction', async ({ page, openRecorder, server }) => {
+    const recorder = await openRecorder();
+    await recorder.setContentAndWait(`<textarea></textarea>`);
+    await Promise.all([
+      recorder.waitForOutput('JavaScript', 'fill'),
+      page.locator('textarea').fill('Hello world'),
+    ]);
+    await page.waitForTimeout(500);
+    await page.goto(server.PREFIX + `/empty.html`);
+    await recorder.waitForOutput('JavaScript', `await page.goto('${server.PREFIX}/empty.html');`);
+  });
 });