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(ios): fire resume/pause events if no cordova plugins installed #4467

Merged
merged 3 commits into from
Apr 22, 2021

Conversation

jcesarmobile
Copy link
Member

Cordova fires pause and resume events, but if there are no cordova plugins installed, cordova doesn't get initialized, so the events are not fired.
This PR fires pause/resume when there are no cordova plugins installed for compatibility

closes #4456

@ikeith
Copy link
Contributor

ikeith commented Apr 22, 2021

If we always want to send the events, would it make sense to just remove the code from CDVPluginManager and just have one place where the notifications are handled rather than duplicating it?

And it's a minor concern but the block-based notification API can easily lead to a (small) memory leak. The returned opaque token is supposed to be saved so it can be passed to removeObserver later. There is a swift-lint rule to catch that, although it is disabled by default.

@jcesarmobile
Copy link
Member Author

I try to avoid making changes in cordova classes because they are mostly copies of the original cordova ones, so we can reintroduce it in future updates without noticing and cause duplicate events

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@jcesarmobile jcesarmobile changed the title fix(ios): Fire resume/pause events if no cordova plugins installed fix(ios): fire resume/pause events if no cordova plugins installed Apr 22, 2021
@jcesarmobile jcesarmobile merged commit 0105f7a into main Apr 22, 2021
@jcesarmobile jcesarmobile deleted the fire-cord-events branch April 22, 2021 18:04
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: pause/resume document events only fired if cordova plugins are installed
3 participants