Skip to content

feat(electron): emit dialog event for main-process dialog API#40911

Closed
yury-s wants to merge 2 commits into
microsoft:mainfrom
yury-s:fix-8278
Closed

feat(electron): emit dialog event for main-process dialog API#40911
yury-s wants to merge 2 commits into
microsoft:mainfrom
yury-s:fix-8278

Conversation

@yury-s
Copy link
Copy Markdown
Member

@yury-s yury-s commented May 19, 2026

Summary

Adds two events on ElectronApplication for intercepting Electron's main-process dialog API:

  • filechooser (ElectronFileChooser) for dialog.showOpenDialog / dialog.showSaveDialogsetFiles(paths) / cancel().
  • dialog (ElectronDialog) for dialog.showMessageBox / dialog.showCertificateTrustDialogaccept(result) / dismiss().

The hook is installed via CDP Runtime.evaluate against the Node session, so it works for both packaged and non-packaged Electron apps. Patched dialog methods stream call payloads back through a Runtime.addBinding binding and await results from the Playwright server.

Fixes #8278

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Adds two events on ElectronApplication for intercepting the Electron
main-process dialog API:

- `filechooser` (ElectronFileChooser) for `dialog.showOpenDialog` and
  `dialog.showSaveDialog`, with `setFiles(paths)` / `cancel()`.
- `dialog` (ElectronDialog) for `dialog.showMessageBox` and
  `dialog.showCertificateTrustDialog`, with `accept(result)` / `dismiss()`.

The hook is installed via CDP `Runtime.evaluate` against the Node
session, so it works for both packaged and non-packaged apps. The
patched dialog methods stream call payloads back via a `Runtime.addBinding`
binding and await results from the Playwright server.

Fixes microsoft#8278
@yury-s yury-s marked this pull request as ready for review May 21, 2026 00:08
@yury-s yury-s requested a review from dgozman May 21, 2026 00:08
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Collaborator

@dgozman dgozman left a comment

Choose a reason for hiding this comment

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

Looking at this in details, I don't think we provide more value than:

await app.evaluate(({ dialog }, paths) => {
  dialog.showOpenFileDialogSync = () => paths;
}, ['/path/to/file']);

Perhaps we just need some docs, and that's it?

Comment thread packages/playwright-core/src/server/dispatchers/electronDispatcher.ts Outdated
return this._initializer.method;
}

options(): any {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do we need to return options? Are there any particular options we care about? Perhaps explicit isMultiple() to match the browser API?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The list of options is pretty reach in electron, it might be useful for asserting behavior/filtering the dialogs, things like defaultPath, filters, openFiles etc. We can drop that completely and expose isMultiple() to match web api, or just wait for user reports.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "MCP"

7181 passed, 1113 skipped


Merge workflow run.

@github-actions
Copy link
Copy Markdown
Contributor

Test results for "tests 1"

4 flaky ⚠️ [chromium-library] › library/inspector/recorder-api.spec.ts:120 › should type `@ubuntu-22.04-chromium-tip-of-tree`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-arm-node20`
⚠️ [chromium-library] › library/video.spec.ts:719 › screencast › should work with video+trace `@chromium-ubuntu-22.04-node20`
⚠️ [chromium-library] › library/video.spec.ts:476 › screencast › should capture static page in persistent context @smoke `@chromium-ubuntu-22.04-node22`

42060 passed, 850 skipped


Merge workflow run.

@yury-s
Copy link
Copy Markdown
Member Author

yury-s commented May 21, 2026

Looking at this in details, I don't think we provide more value than:

await app.evaluate(({ dialog }, paths) => {
  dialog.showOpenFileDialogSync = () => paths;
}, ['/path/to/file']);

Perhaps we just need some docs, and that's it?

For simple cases yes, but for more complex where the users my want to have some logic in playwright we are currently missing out on ElectronApplication.exposeBinding, it only exists on the renderer contexts, maybe we should expose it for the node context too?

@yury-s
Copy link
Copy Markdown
Member Author

yury-s commented May 21, 2026

Docs-only approach: #40952.

@yury-s yury-s closed this May 21, 2026
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][Electron] filechooser event not emitted when dialog.show(Open|Save)Dialog is called

2 participants