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(android): init cookie manager on plugin methods #6684

Closed
wants to merge 2 commits into from

Conversation

ItsChaceD
Copy link
Contributor

@ItsChaceD ItsChaceD commented Jun 27, 2023

This PR fixes an issue where CapacitorCookies plugin methods are throwing NullPointerException when patching document.cookie is disabled.

Addresses: #6640

@jcesarmobile
Copy link
Member

I've sent #6685 so we can move the CapacitorCookieManager creation back to load() method and we can revert the fix that is causing #6640 without having to call initCookieManager on every plugin call

@ItsChaceD
Copy link
Contributor Author

I've sent #6685 so we can move the CapacitorCookieManager creation back to load() method and we can revert the fix that is causing #6640 without having to call initCookieManager on every plugin call

Perfect, that change makes more sense. I will review & open PRs to move the initialization back to the load() method. The logic on this PR is also faulty since CookieHandler.setDefault(this.cookieManager); should only be called when isEnabled() is true.

@markemer
Copy link
Contributor

I've sent #6685 so we can move the CapacitorCookieManager creation back to load() method and we can revert the fix that is causing #6640 without having to call initCookieManager on every plugin call

Perfect, that change makes more sense. I will review & open PRs to move the initialization back to the load() method. The logic on this PR is also faulty since CookieHandler.setDefault(this.cookieManager); should only be called when isEnabled() is true.

Yeah, thats a good idea. A bit cleaner.

@jcesarmobile jcesarmobile deleted the fix/cookies-npe branch June 28, 2023 08:15
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

3 participants