EddyVerbruggen / nativescript-local-notifications

:mailbox: NativeScript plugin to easily schedule local notifications
MIT License
162 stars 57 forks source link

Conflict between nativescript-local-notifications and nativescript-plugin-firebase on iOS #107

Closed Danziger closed 6 years ago

Danziger commented 6 years ago

I was using version 3.0.0 of nativescript-local-notifications with nativescript-push-notifications and, as far as the limitations of that other plugin allow, everything was working fine on iOS.

However, after replacing it with nativescript-plugin-firebase, it's not possible to schedule and display a local notification anymore:

Step by step, this is what's happening:

If the App is in background, onMessageReceivedCallback won't be called until the App is opened, but then the same whole thing happens.

Tested on an iPhone 7 with iOS 10.3.1.

EddyVerbruggen commented 6 years ago

The Firebase plugin allows showing notifications in the foreground as well. Does that eliminate the use for this plugin perhaps?

Danziger commented 6 years ago

@EddyVerbruggen As far as I understood, nativescript-plugin-firebase is intended to only receive the notifications and then if I want to display them and customise how they look, I should use nativescript-local-notifications, is this right?

Also, note this same setup is working fine on Android.

EddyVerbruggen commented 6 years ago

@Danziger Nope, the Firebase plugin shows notifications as well, see this for instance: https://github.com/EddyVerbruggen/nativescript-plugin-firebase/blob/f399d03ec9749ecee10ec9c38faf1f8190073c5b/docs/MESSAGING.md#interactive-notifications-ios-only-for-now

However, until recently the Firebase plugin didn't support showing notifications while the app was in the foreground.

Danziger commented 6 years ago

@EddyVerbruggen Well, we use Adobe Mobile Services to send the notifications, so the format is always:

{
    "foreground": true,
    "from": "...",
    "data": {
        "adb_deeplink": "...",
        "attachment-url": "...",
        "message": "...",
        ... custom fields ...
    }
}

So we can't send a payload like the one in that example as we can only change the fields inside data.

Danziger commented 6 years ago

@EddyVerbruggen Any clue about this one? I guess the issue is in nativescript-plugin-firebase rather than here. I can try to fix it myself, but any help on where to start to look is very much appreciated.

EddyVerbruggen commented 6 years ago

I think adding a “show notifications when app is in foreground” property could be added to init. Unless the property in an individual notification is false it should show.

Would that work for you?

Danziger commented 6 years ago

I will check that out on nativescript-plugin-firebase and see if setting it there fixes the issue, although right now I'm already using forceShowWhenInForeground: true in nativescript-local-notifications and that's not working:

LocalNotifications.schedule([{
    id,
    title,
    icon: 'res://notification',
    silhouetteIcon: 'res://notification_silhouette',
    color: MAIN_COLOR,
    image,
    thumbnail: true,
    forceShowWhenInForeground: true,
    channel: this._translateService.instant('...'),
}]);
EddyVerbruggen commented 6 years ago

To eaily test it, you could change https://github.com/EddyVerbruggen/nativescript-plugin-firebase/blob/d0f3f3d478b3e3a121d36be4c61cee7e738df7a0/src/messaging/messaging.ios.ts#L529 to if (1 === 1) in your node_modules folder. If that helps I can add the change I proposed in my previous comment.

Danziger commented 6 years ago

@EddyVerbruggen Ok, that partially fixes the issue. Now the notifications are showing up but when I tap them, the userNotificationCenterDidReceiveNotificationResponseWithCompletionHandler that is called is the one in messaging.ios.ts, not the one in nativescript-local-notifications.

Danziger commented 6 years ago

@EddyVerbruggen can you please take a look at this PR to see if these changes are fine for you? https://github.com/Danziger/nativescript-plugin-firebase/pull/1

If they are, I will merge that into my fork and then open a PR in the official repo.

The problem was that both nativescript-local-notificationsand nativescript-plugin-firebase are setting a Notification Center delegate, so only one of them (the last one to do it) will work as expected.

I have added an option in nativescript-plugin-firebase to avoid that and only subscribe to Firebase and get notified through the callback, which is more appropriate for those who want to decide what to do or how/when to display a notification every time a notification is received.

EddyVerbruggen commented 6 years ago

Man, you're kicking a$$, looks good to me! Perhaps remove the package.json version bump from your PR (I can ignore it, no biggie). I will release 7.0.0 soon and I'd love to include this.

edusperoni commented 6 years ago

Hey,

does this mean I have to set displayNotifications to false if I want ot use nativescript-local-notifications? Wouldn't this prevent my app from showing the notification while in the background?

Danziger commented 6 years ago

Hi @edusperoni. I need to do a bit more of testing on that, but unfortunately, I'm currently a bit stuck trying to integrate these 2 plugins with Adobe Mobile Services and make that work on iOS particularly.

As long as I manage to get that to work I will test this properly.

The problem is that both this plugin and nativescript-local-notifications are setting the .delegate on UNUserNotificationCenter, which is a singleton. That means that only the second one to do so will have its delegate in place, while the first one will be overwritten.

In my case, that meant that nativescript-plugin-firebase would receive a push notification and invoke the onMessageReceivedCallback callback, where I would use nativescript-local-notifications to create/display an actual notification on the device, but instead of that, the same onMessageReceivedCallback would be triggered.

Danziger commented 6 years ago

@EddyVerbruggen I just thought about this. Maybe these changes need to be implemented on Android as well to have these 2 new options working consistently on both platforms? What do you think?

edusperoni commented 6 years ago

I also use both plugins, but we're not in production yet and this was going to be our last test going into production. Ideally, I'd like to be able to receive push and also create local notifications. I don't really mind having to use the same callback, as long as I'm able to tell the two notification types apart.

nativescript-local-notifications doesn't allow a payload, for example, whereas FCM does. A a result, we have a sqlite table in which we associate a payload to a notification id.

Since we're using angular services for this, I'm not even sure we can hook something up to generate a notification from FCM when the app is closed.

edusperoni commented 5 years ago

I've been looking at the code, and it seems that if we use nativescript-local-notifications with showNotifications = false on firebase, I'm no longer able to get firebase notification on foreground, but with showNotifications = true, I can receive both, but showNotificationsWhenInForeground doesn't work, obviously, and I lose information like notification id.

I'm not even able to get firebase notification and then schedule them via local-notifications because the firebase callback is never called. I don't know how we should approach this, any ideas?

EddyVerbruggen commented 5 years ago

@edusperoni Can you tell me why you're using the local notifications plugin at all (since you use push notifications as well)?

edusperoni commented 5 years ago

We have a screen that shows the bus times coming to/from our institution. The user can schedule a notification (local-notifications) for when their bus leaves (or a few minutes before/after). We have push notifications to warn the user of events outside of their control (coming from our software).

EddyVerbruggen commented 5 years ago

Ah, interesting, thx!

bniehuser commented 5 years ago

any potential workaround / solution for this issue? our application also makes use of both plugins; firebase for remote notifications from server and local-notifications for user-generated reminders and such. the conflation of the UNUserNotificationCenter.delegate means that the firebase handler is handling messages when the application is open, and the received message is missing all data (exactly as described above). i have not tested sending firebase messages to the application while it is in the background, however these seem to appear correctly in the notification tray from local-notifications. we are using simple text content, so i'm not sure which handler is handling these background notifications. can this issue be reopened?

if whatever functionality is being assigned to the UNUserNotificationCenter.delegate is exposed from each plugin, conceivably a developer could wrap both plugins' behavior and assign it explicitly.

I will test with capturing this delegate after init of each plugin and see if i can manually wrap it, although i'm not sure if this will lead to double-processing both firebase and local notifications -- will report back.

followup: i tried naively to create a new delegate object after firebase initialization using its value before and after init, e.g.:

// before firebase init
const olddelegate = UNUserNotificationCenter.currentNotificationCenter().delegate;

// firebase init...

//on init callback:
const newdelegate = UNUserNotificationCenter.currentNotificationCenter().delegate;
UNUserNotificationCenter.currentNotificationCenter().delegate = {
    ...newdelegate,
    userNotificationCenterDidReceiveNotificationResponseWithCompletionHandler: (center: UNUserNotificationCenter, response: UNNotificationResponse, completionHandler: () => void): void => {
        olddelegate.userNotificationCenterDidReceiveNotificationResponseWithCompletionHandler(center, response, completionHandler);
        newdelegate.userNotificationCenterDidReceiveNotificationResponseWithCompletionHandler(center, response, completionHandler);
    },
    userNotificationCenterWillPresentNotificationWithCompletionHandler: (center: UNUserNotificationCenter, notification: UNNotification, completionHandler: (p1: UNNotificationPresentationOptions) => void): void => {
        olddelegate.userNotificationCenterWillPresentNotificationWithCompletionHandler(center, notification, completionHandler);
        newdelegate.userNotificationCenterWillPresentNotificationWithCompletionHandler(center, notification, completionHandler);
    },
};

which was a shot in the dark in the first place, since i know nothing about how ios manages this stuff under the hood. essentially it did nothing. for all i know i'm modifying things that are totally unused or overwritten later.

is this a technique that could possibly work, and/or is there a clean way to identify which message comes from which plugin and handle them accordingly? thanks for your time and awesome plugins.

jannomeister commented 5 years ago

@EddyVerbruggen @Danziger In my case I used both nativescript-local-notifications and nativescript-firebase-plugin because I need to make a local notifications for useful tips for the user of the app. And I used nativescirpt-firebase-plugin for remote notifications. Hope there's a workaround for it :(

edusperoni commented 5 years ago

I had an ideia a few days ago but haven't got the time to actually test it:

A plugin (nativescript-ios-notification-delegate maybe) that binds it's delegate and provides a way for other layers to intercept the message. If your plugin is willing to intercept the message, it would intercept and return true, for example, making it not send to other "listeners".

So, for example: firebase and local-notifications registers it's methods. In the message, firebase verifies if it's a firebase message, handles it and pass it or not to local-notifications, which does the same.

@EddyVerbruggen would you support this plugin in your projects I were to create it?

EddyVerbruggen commented 5 years ago

@edusperoni Yes, I was thinking of something similar because it's one of the most painful cross-plugin issues we currently have. I'd actually prefer having something like that in NativeScript. Perhaps during development of such a plugin, we can find a way to incorporate it in NativeScript's core. Anyway, I would most certainly support this effort.

ghost commented 5 years ago

@edusperoni Hopefully you can sort this thing out. We'd be happy to do testing afterwards!

edusperoni commented 5 years ago

@doombringer45 if you want to test it, the PR is out!

I'll do some proper testing on my organization's app on Monday. (we were using both plugins by ignoring the callbacks up until now)

PeterStaev commented 4 years ago

@EddyVerbruggen , can we reopen this and have a proper fix. Right now it is a tedious job to make both plugins work - both override the shared delegate. Not only that but seems Firebase one is really persistent on doing so even if I put my own delegate, it continues to override it on every call to registerForPushNotifications 😁

I was able to make it work, with a wrapping delegate with which I override the one that is set up AND custom code in the app.ts for iOS to not use the default set up of the Firebase plugin (good that I dont use anything except messaging 😄 !)