Skip to content

fix(tracing): do not stall on dialogs#7059

Merged
pavelfeldman merged 3 commits into
microsoft:masterfrom
pavelfeldman:fix_Dialog
Jun 11, 2021
Merged

fix(tracing): do not stall on dialogs#7059
pavelfeldman merged 3 commits into
microsoft:masterfrom
pavelfeldman:fix_Dialog

Conversation

@pavelfeldman
Copy link
Copy Markdown
Member

Fixes #7048

Comment thread tests/tracing.spec.ts Outdated
Comment thread tests/tracing.spec.ts
await page.goto(server.EMPTY_PAGE);

page.on('dialog', async dialog => {
await dialog.accept();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we have to catch this promise and await it? I am worried that dialog.accept will race with context.close and throw an unhandled promise rejection.

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.

I believe we are ok since the following call to confirm is blocking and is serialized with the accept ack.

Co-authored-by: Joel Einbinder <joel.einbinder@gmail.com>
@pavelfeldman pavelfeldman merged commit cbce7cb into microsoft:master Jun 11, 2021
@aslushnikov aslushnikov added this to the v1.12.2 milestone Jun 11, 2021
aslushnikov pushed a commit to aslushnikov/playwright that referenced this pull request Jun 11, 2021
aslushnikov added a commit that referenced this pull request Jun 11, 2021
Cherry-Pick #7059 SHA cbce7cb

Fixes #7048

Co-authored-by: Pavel Feldman <pavel.feldman@gmail.com>
@aslushnikov aslushnikov removed this from the v1.12.2 milestone Jun 11, 2021
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] Dialogs cannot be dismissed if tracing is on in Chromium or Webkit

3 participants