Skip to content

chore(firefox): remove the WebSocket handshake Worker workaround - #42455

Open
Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:firefox-fix-missing-WebSocket-request-workaround
Open

chore(firefox): remove the WebSocket handshake Worker workaround#42455
Devin Rousso (dcrousso) wants to merge 1 commit into
microsoft:mainfrom
dcrousso:firefox-fix-missing-WebSocket-request-workaround

Conversation

@dcrousso

@dcrousso Devin Rousso (dcrousso) commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

See #42037 for the original tests.

D310690 preserves the original LoadInfo when Firefox creates the handshake channel, which keeps the worker browsing context associated with its WebSocket request

require requestData again in FrameManager.onWebSocketRequest and assert that _onWebSocketOpened receives it

remove the Firefox guards for the worker locale and setExtraHTTPHeaders coverage because the association now reaches both handshake paths

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

🔴 The failures are caused by this PR

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

All four failures are Firefox worker/handshake WebSocket tests that this PR directly rewrites, and they fail for the reason the PR description already calls out: this needs the Firefox 154 roll with D310690, which CI isn't running yet. Until that lands, the worker WebSocket handshake isn't associated with its browsing context, so the newly-required requestData never arrives and the removed guards no longer protect these tests.

Details

Overall: not flakes, not infra. The diff reaches every failing test — it removes the Firefox worker workaround in ffPage.ts::_onWebSocketOpened, makes requestData required in FrameManager.onWebSocketRequest (frames.ts), and deletes the it.fixme(browserName === 'firefox' && !isBidi, …) skips on three of these exact tests. All four fail only on Firefox, which is exactly where the change applies.

Caused by this PR

  • [firefox-library] › library/browsercontext-locale.spec.ts:226 › should send Accept-Language header on WebSocket handshake and :241 › … from a worker — the PR removes the firefox && !isBidi it.fixme on these, so they now run on Firefox. Without the D310690 roll, Firefox still doesn't send the emulated Accept-Language on the (worker) handshake, so the assertion fails.
  • [firefox-library] › library/web-socket.spec.ts:260 › should send extra HTTP headers on WebSocket handshake from a worker — same story: the firefox && !isBidi skip is removed, but the worker handshake still lacks the extra headers on the current Firefox build.
  • [firefox-library] › library/web-socket.spec.ts:200 › should not tear down the page when a WebSocket is opened inside a worker — the workaround that returned early when request/response were missing is gone and _onWebSocketOpened now does assert(request) / assert(response). On a Firefox without D310690, the worker WebSocket has no handshake metadata, so the assert throws instead of the old graceful path — hence the failure.

No separate flake check was needed: the change removes these tests' Firefox guards and rewrites the exact code path they exercise, so the causation is direct.

What unblocks it: land the Firefox 154 roll that includes D310690 first (or rebase onto it), then this PR should go green on Firefox.

Triaged by the Playwright bot - agent run

@dgozman

Copy link
Copy Markdown
Collaborator

This one fails on the bots - does it need a rebase?

@dcrousso

Copy link
Copy Markdown
Contributor Author

oh oops i opened this before we updated to firefox 154

`D310690` preserves the original `LoadInfo` when Firefox creates the handshake channel, which keeps the worker browsing context associated with its `WebSocket` request

require `requestData` again in `FrameManager.onWebSocketRequest` and assert that `_onWebSocketOpened` receives it

remove the Firefox guards for the worker `locale` and `setExtraHTTPHeaders` coverage because the association now reaches both handshake paths

this requires the Firefox `154` roll that first includes `D310690`
@dcrousso
Devin Rousso (dcrousso) force-pushed the firefox-fix-missing-WebSocket-request-workaround branch from d690403 to 981cc8e Compare August 31, 2026 15:43
@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

4 flaky ⚠️ [chromium-library] › library/beforeunload.spec.ts:130 › should support dismissing the dialog multiple times `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:736 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node22`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:257 › third party 'Partitioned;' cookies `@firefox-ubuntu-22.04-node20`
⚠️ [firefox-library] › library/browsercontext-cookies-third-party.spec.ts:470 › top level 'Partitioned;' cookie and same origin iframe `@firefox-ubuntu-22.04-node20`

51281 passed, 1238 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chrome] › mcp/files.spec.ts:106 › clicking on download link emits download @mcp-macos-latest-chrome
❌ [chromium] › mcp/request-blocking.spec.ts:147 › allowed blocks different hostname (wildcard port) @mcp-macos-latest-chromium

8303 passed, 1367 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

🟡 The PR's own failures are cleared; two unrelated MCP failures remain unproven

Hi, I'm the Playwright bot and I took another look at the CI now that Firefox 154 has landed.

The four Firefox WebSocket failures from the earlier run are gone — the latest tests 1 run is flaky-only, which matches the PR's premise: it needed the Firefox 154 roll with D310690, and that's now in. What's left are two MCP failures on macOS (mcp/files.spec.ts download, mcp/request-blocking.spec.ts wildcard port). This diff is Firefox-WebSocket-only, so it can't reach either — but I can't formally certify them as flakes (they've never failed before), so I'm calling this yellow rather than green.

Details

Overall: the change touches ffPage.ts::_onWebSocketOpened (Firefox-only), FrameManager.onWebSocketRequest in frames.ts (WebSocket-only), and removes Firefox it.fixme guards on three WebSocket tests. Nothing in it goes near Chromium downloads or request-blocking.

Cleared (was caused by this PR, now passing)

  • The earlier firefox-library failures — browsercontext-locale.spec.ts:226/:241, web-socket.spec.ts:200/:260 — no longer fail. They needed the Firefox 154 roll (D310690) that CI wasn't running when the PR opened; it's landed, and the latest tests 1 run reports 4 flaky, 0 failed.

Uncertain (not this PR, but not a proven flake either)

  • [chrome] › mcp/files.spec.ts:106 › clicking on download link emits download (@mcp-macos-latest-chrome) and [chromium] › mcp/request-blocking.spec.ts:147 › allowed blocks different hostname (wildcard port) (@mcp-macos-latest-chromium). The diff doesn't reach either code path, and in the aggregated results DB both tests are 100% green — 0 failures across 728 and 725 runs respectively, on every browser and across other SHAs/PRs. Two rock-solid, unrelated tests failing together on the same macOS runner reads like an infra hiccup, but I found no prior failure of either test to prove a flake, so I'm leaving them as uncertain rather than confirmed noise. A re-run should tell — I'd expect both to go green.

Triaged by the Playwright bot.

Triaged by the Playwright bot - agent run

Sign up for free to 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