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

[flutter_local_notifications] fix for nil value in notification.request.content.userInfo #1114

Closed
wants to merge 2 commits into from

Conversation

badver
Copy link
Contributor

@badver badver commented Apr 7, 2021

Sometimes there is nil value in notification.request.content.userInfo[...]
Just add checking for nil in if statement.

@MaikuB
Copy link
Owner

MaikuB commented Apr 11, 2021

Do you have examples on how the scenario could happen? Trying to understand the root cause as I haven't seen any reports on this and I would've thought all notifications created from this plugin when run on macOS 10.14+ would have these values due to the code at

content.userInfo = [MethodCallArguments.payload: arguments[MethodCallArguments.payload] as Any, MethodCallArguments.presentSound: presentSound, MethodCallArguments.presentBadge: presentBadge, MethodCallArguments.presentAlert: presentAlert]

@badver
Copy link
Contributor Author

badver commented Apr 12, 2021

It happens often when a desktop app is minimized on macOS 10.15.7 and a user has clicked on a notification from FCM.

@MaikuB
Copy link
Owner

MaikuB commented Apr 15, 2021

That sounds odd as I thought userinfo is populated and the code for the fcm plugin would would indicate that it is too https://github.com/FirebaseExtended/flutterfire/blob/44c410c4cb0f01d173c348e835a6a497416d174b/packages/firebase_messaging/firebase_messaging/ios/Classes/FLTFirebaseMessagingPlugin.m#L279

If it isn't then the bug should exist in the fcm plugin too. Based on this, I suspect the issue lies elsewhere. It would be great if you're able to provide a link to a repo with a minimal app that can reproduce this.

The other thing is the changes in this PR would end up having the plugin process notifications coming from other plugins. This should be avoided as it would prevent other plugins from doing so. Some of them, like fcm, rely on this to trigger callbacks that would be received in the Flutter/Dart side. The iOS implementation of the plugin does have some code to do this but I hadn't brought it over to the macOS implementation as last I saw, the Flutter engine needed changes on macOS to allow for multiple notification plugins to work together like it does on iOS.

@badver
Copy link
Contributor Author

badver commented Jun 1, 2021

Sorry for the delay. I cannot reproduce this problem anymore.
I think it's related to firebase_messaging, not your package, so let's close this as unreproducible.

@badver badver closed this Jun 1, 2021
@badver badver deleted the fix_nil branch June 1, 2021 07:17
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