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: move offline/cache/interception switches to BrowserContext #748

Merged
merged 3 commits into from Jan 29, 2020

Conversation

yury-s
Copy link
Member

@yury-s yury-s commented Jan 29, 2020

No description provided.

docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
docs/api.md Outdated Show resolved Hide resolved
@@ -91,17 +97,19 @@ export class CRNetworkManager {
}

async setCacheEnabled(enabled: boolean) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we have a parameter which is now ignored?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is how it is declared in the delegate interface. 2 other browsers use that param. We can update that in a separate change.

@@ -90,6 +90,8 @@ export class FFPage implements PageDelegate {
promises.push(this._session.send('Page.setJavascriptEnabled', { enabled: false }));
if (options.userAgent)
promises.push(this._session.send('Page.setUserAgent', { userAgent: options.userAgent }));
if (options.cacheEnabled === false)
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like this is missing request interception changes in firefox.

test/interception.spec.js Show resolved Hide resolved
return !!this._page.browserContext()._options.interceptNetwork;
}

private async _updateProtocolRequestInterception() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This stuff is too complicated 😞

Copy link
Member Author

Choose a reason for hiding this comment

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

it is, let me change that in a follow-up.

@yury-s yury-s merged commit 6faf74b into microsoft:master Jan 29, 2020
@yury-s yury-s deleted the context-methods branch January 29, 2020 20:51
pavelfeldman added a commit to pavelfeldman/playwright that referenced this pull request Feb 1, 2020
pavelfeldman added a commit that referenced this pull request Feb 1, 2020
sand4rt pushed a commit to sand4rt/playwright that referenced this pull request Dec 21, 2022
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.

None yet

2 participants