Skip to content

fix(server): dispose temporary iframe element handles after hit-target checks - #42655

Merged
Pavel Feldman (pavelfeldman) merged 2 commits into
microsoft:mainfrom
ashrafiucse:fix-42653
Sep 10, 2026
Merged

Pavel Feldman (pavelfeldman) merged 2 commits into
microsoft:mainfrom
ashrafiucse:fix-42653

Conversation

@ashrafiucse

Copy link
Copy Markdown
Contributor
  • Dispose temporary iframe element handles in ElementHandle._checkFrameIsHitTarget (including early-return and exception paths) and in FrameSession._framePosition, so Chromium does not retain detached iframes after clicks inside frames
  • Add a regression test that removes an iframe after clicking inside it and asserts collection via CDP-forced GC

Diagnosis and reproduction in the issue by Jan Doležel (@dolezel).

Fixes #42653

Comment thread tests/page/page-click.spec.ts Outdated
await page.evaluate(() => document.querySelector('iframe')!.remove());
// Move the mouse away to release Chromium's own last-hovered-node retention.
await page.mouse.move(500, 500);
const session = await page.context().newCDPSession(page);

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.

You can do page.requestGC everywhere.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done — switched to page.requestGC() and dropped the chromium-only skip, so the test runs on all browsers now. Verified locally on chromium and firefox; webkit via CI.

@pavelfeldman

Copy link
Copy Markdown
Member

Ashraf Ali (@ashrafiucse) I enabled using notation for convenience and removed one of the disposes from your patch to avoid conflict. I'm not sure it'll rebase it though and make the test pass. You might need to push a rebase. Sorry for trouble.

…t checks

ElementHandle._checkFrameIsHitTarget and FrameSession._framePosition
create temporary iframe element handles via DOM.resolveNode and never
dispose them, so Chromium retains detached iframes after clicks inside
frames. Dispose them in finally, covering early returns and exceptions.

Fixes: microsoft#42653
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "tests 1"

5 flaky ⚠️ [chromium-library] › library/popup.spec.ts:260 › should not throw when click closes popup `@chromium-ubuntu-22.04-node20`
⚠️ [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`
⚠️ [webkit-page] › page/workers.spec.ts:165 › should clear upon navigation `@webkit-ubuntu-22.04-node20`
⚠️ [playwright-test] › ui-mode-test-watch.spec.ts:145 › should watch all `@ubuntu-latest-node26`

51551 passed, 1247 skipped


Merge workflow run.

@github-actions

Copy link
Copy Markdown
Contributor

Test results for "MCP"

2 failed
❌ [chromium] › mcp/annotate.spec.ts:446 › should switch screencast to -s session on show --annotate @mcp-windows-latest-chromium
❌ [chromium] › mcp/http.spec.ts:105 › http transport browser lifecycle (isolated) @mcp-ubuntu-latest-chromium

8346 passed, 1376 skipped


Merge workflow run.

@pavelfeldman
Pavel Feldman (pavelfeldman) merged commit b99b75d into microsoft:main Sep 10, 2026
43 of 45 checks passed
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.

[Bug]: Temporary iframe handles retained after frameLocator clicks

2 participants