Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(downloads): fix acceptDownloads complaint (#1777) #1923

Merged
merged 4 commits into from
Apr 23, 2020

Conversation

rwoll
Copy link
Member

@rwoll rwoll commented Apr 22, 2020

This patch fixes download.path() throwing an error that acceptDownloads is not set when using a download within a page.on handler even when acceptDownloads had been set to true.

See this comment for an explanation of the issue.

If you checkout fbc52e7, you can run the newly added tests with the previous code and observe the failure.

Fixes #1777

The fail with:

```
NON-TEST ERROR #1: UNHANDLED PROMISE REJECTION
  Error: Pass { acceptDownloads: true } when you are creating your browser context.
```

which shows the issue we were running into.
This patch fixes `download.path()` throwing an error that
`acceptDownloads` is not set when using a download within a `page.on`
handler even when `acceptDownloads` had been set to `true`;

Fixes microsoft#1777
@msftclas
Copy link

msftclas commented Apr 22, 2020

CLA assistant check
All CLA requirements met.

Copy link
Member

@pavelfeldman pavelfeldman left a comment

Choose a reason for hiding this comment

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

It looks great, thank for the fix! One small test issue and it can be landed.

test/download.spec.js Outdated Show resolved Hide resolved
Per-review feedback, just let this timeout if it's not working.
@pavelfeldman pavelfeldman merged commit 53c78a8 into microsoft:master Apr 23, 2020
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] Chrome downloads fail despite acceptDownloads set
3 participants