Skip to content

chore(recorder): move fill coalescing back to collapseActions - #42442

Open
Dmitry Gozman (dgozman) wants to merge 1 commit into
microsoft:mainfrom
dgozman:recorder-fill-collapse-actions
Open

chore(recorder): move fill coalescing back to collapseActions#42442
Dmitry Gozman (dgozman) wants to merge 1 commit into
microsoft:mainfrom
dgozman:recorder-fill-collapse-actions

Conversation

@dgozman

@dgozmanDmitry Gozman (dgozman) commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

Partially reverts #41958:

  • RecorderSignalProcessor no longer buffers fill actions; click and navigate coalescing stays there.
  • Brings back shouldMergeAction()/collapseActions() (fill-only now) and the actionUpdated protocol event.

#41958 forced quick typing into the same input - a pause over 500ms would generate two separate fill() actions, which is a behavior change we don't necessarily need to make yet.

Revert the fill part of RecorderSignalProcessor buffering, and bring back
the "actionUpdated" event together with client-side collapseActions().
Click and navigate coalescing stays in the signal processor.
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

10 flaky⚠️ [chromium-library] › library/chromium/chromium.spec.ts:179 › serviceWorker(), and fromServiceWorker() work `@frozen-time-library-chromium-linux`
⚠️ [chromium-library] › library/browsertype-connect.spec.ts:714 › run-server › should record trace with sources `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/beforeunload.spec.ts:130 › should support dismissing the dialog multiple times `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node24`
⚠️ [chromium-library] › library/video.spec.ts:664 › screencast › should capture full viewport `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:699 › screencast › should capture full viewport on hidpi `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/heap.spec.ts:223 › should not leak workers `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-page] › page/page-event-request.spec.ts:181 › should return response body when Cross-Origin-Opener-Policy is set `@firefox-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-test-update.spec.ts:202 › should update test locations `@ubuntu-latest-node22`

51243 passed, 1240 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

3 failed
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-macos-latest-chromium
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-windows-latest-chromium
❌ [chromium] › mcp/clipboard.spec.ts:19 › clipboard write without permission dialog @mcp-ubuntu-latest-chromium

8256 passed, 1361 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Hi, I'm the Playwright bot and I took a look at the failing CI.

🟢 CI is clear — the one failure is pre-existing, not from this PR

The only real failures are mcp/clipboard.spec.ts:19 › clipboard write without permission dialog on Chromium (ubuntu/macos/windows). The same test fails identically on this PR's base commit 6748802c (a push to main) and on a string of unrelated PRs, so it's already broken on trunk. This PR only reworks recorder fill coalescing and doesn't touch the MCP clipboard/permissions path. The tests 1 report had no failures — just flakes.

Details

Overall: nothing in this diff explains the failures. It moves fill coalescing out of RecorderSignalProcessor back into collapseActions()/shouldMergeAction() and re-adds the actionUpdated recorder event — all recorder codegen. The failing test exercises browser_navigate + browser_evaluate clipboard permissions, which this PR never reaches.

Pre-existing flake / infra

No real failures elsewhere

  • The tests 1 report shows 10 flaky, 0 failed (51243 passed). Nothing to triage there.

Triaged by the Playwright bot - agent run

else
recordedActions.push(code);
},
signalAdded: (page, signal, code) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we remove this handler now that there is an explicit actionUpdated event?

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@dgozman@yury-s