fechanique / cordova-plugin-fcm

Google FCM Push Notifications Cordova Plugin
624 stars 990 forks source link

Heads up notification not working in foreground #365

Open balajikarthiq opened 6 years ago

balajikarthiq commented 6 years ago

Heads up notification is not working, even setting the priority as "High" and checked the file MyFirebaseMessagingService.java.

The sendNotification method is commented in onMessageReceived method.

CowboyCode commented 6 years ago

It is working in foreground but it doesn't make a sound. The message is received, like you would click on it, receiving it in background.

In your code, onNotification, you can display the message in a modal for example. I save all notifications in sqlite and have a screen that shows all messages. In the onNotification, I redirect to the notification screen.

FYI

munyanyika24 commented 6 years ago

I am getting notification on Android but on iOS the onNotification method is not firing at all.

I am running iOS 10.3, in console, I see messages that everything is initialized and token is registered but when a send a notification via the firebase console, I receive it on Android but not on iOS. The plugin version i am running is 2.1.2 (latest one)

Please assist

CowboyCode commented 6 years ago

Did you set up your ios certificates in firebase?

1N50MN14 commented 6 years ago

@CowboyCode I'm using your fork and all works for me on iOS excepting when sending data notifications, the xcode log shows:

FIRMessaging received data-message, but FIRMessagingDelegate's-messaging:didReceiveMessage: not implemented

There's an open PR on this issue but it unfortunately conflicts with your modifications I was just wondering if you somehow got this working for you.. thank you (and for spending time trying to assist others on this repo)

CowboyCode commented 6 years ago

I will test it on Monday morning and let you know

1N50MN14 commented 6 years ago

@CowboyCode Thank you

blumanski commented 6 years ago

@1N50MN14 It works for me, see my Xcode log below. I have it running on an iphone 6 and have my app targeted to 9.0 May remove the plugin and add it back helps?

2017-08-28 11:01:17.950265+1000 Projectname[347:20457] Message ID 1: 
0:1503882076935879%7df1ea3a7df1ea3a
2017-08-28 11:01:17.951057+1000 Projectname[347:20457] {
aps =     {
    alert =         {
            body = dsfdsfds;
            title = sdfdsfdsf;
        };
        category = "FCM_PLUGIN_ACTIVITY";
        sound = default;
    };
    channels = "[\"Test\"]";
    created = 1503882075000;
    doc = "";
    "gcm.message_id" = "0:1503882076935879%7df1ea3a7df1ea3a";
    header = sdfdsfdsf;
    message = dsfdsfds;
    page = "";
    type = page;
}

2017-08-28 11:01:17.951407+1000 Projectname[347:20457] 
stringByEvaluatingJavaScriptFromString 
FCMPlugin.onNotificationReceived({"doc":"","header":"sdfdsfdsf","channels":"[\"Test\"]","aps":
{"alert":{"title":"sdfdsfdsf","body":"dsfdsfds"},"sound":"default","category":"FCM_PLUGIN_ACTIVITY"},"message":"dsfdsfds","created":"1503882075000","type":"page","page":"","gcm.message_id":"0:1503882076935879%7df1ea3a7df1ea3a"});

I receive the messages as below

if (typeof(FCMPlugin) != 'undefined') {
    FCMPlugin.onNotification(function(data){
    console.log(data)
    ….

Then I pop the message up using

$cordovaToast
    .show(
        data.header + "\n" + data.message,
        'long',
        'center'
  )

After that I redirect to a notification page which shows all received notifications, I save those in sqlite.

Without the implemented code, you would not know that you got a message, there is noise when the app is in foreground.

1N50MN14 commented 6 years ago

@blumanski Thank you for the feedback; this works for you because you're sending it as notification message and not as a data message, the above works for me as well. I'm sending a data message in accordance to the FCM data message specs (https://firebase.google.com/docs/cloud-messaging/concept-options) whereby you don't specify the titleand body attributes, instead a data object which is passed silently to the app without triggering a notification alert.

blumanski commented 6 years ago

@1N50MN14 lol, I have to many github accounts...

Ah, ok, I just add my data to the notification, FCM does not display the data, just the header and body when the app is in the background. The data has still to be done in the code.

Can't you send it as notification and send the data with it? I add heaps of data to it

1N50MN14 commented 6 years ago

@blumanski :)

Yes I can send a notification with data in it but then it would be displayed to the user by the OS whereas the intention of sending a data message is to let the app deal with the processing / handling of the data.

In my case, I'm using push messages to notify users with things such as "someone liked your post" and data messages to instruct the app to perform certain tasks that do not involve user interaction.

To test this, remove the title and body attributes from your message; instead only include a data object, you'll see the error thrown in xcode

blumanski commented 6 years ago

I will check that out

blumanski commented 6 years ago

@1N50MN14 I think it needs another upgrade of the firebase ios sdk. I give it a shot

https://github.com/firebase/quickstart-cpp/issues/9

1N50MN14 commented 6 years ago

Yes this is it! Thank you for looking into it despite not needing it yourself, very much appreciated @blumanski

CowboyCode commented 6 years ago

@1N50MN14 No worries :-) I did the upgrade to 4.1.0 firebase ios sdk, but it is still not working. You can test it here https://github.com/CowboyCode/cordova-plugin-fcm

Haven't done a pull request yet

1N50MN14 commented 6 years ago

Oh bummer, will test it right away one sec

1N50MN14 commented 6 years ago

@CowboyCode Yup, same error: <Warning> [Firebase/Messaging][I-FCM002019] FIRMessaging received data-message, but FIRMessagingDelegate's-messaging:didReceiveMessage: not implemented

hmmmm this is strange.. did you download the release from https://firebase.google.com/docs/cpp/setup?

blumanski commented 6 years ago

Na, the ios version, 4.1.0. https://firebase.google.com/download/ios

The error must come from the plugin code, I am not a object-c developer :-)

1N50MN14 commented 6 years ago

@blumanski ahh bummer.. neither am I, this language is just too weird.. ;)

blumanski commented 6 years ago

@1N50MN14 it looks like everything is in the code, I guess it would take somebody 5 minutes to add this :-) It would take me probably 10 hours lol, if it even would work :-)

CowboyCode commented 6 years ago

@1N50MN14 There is actually a pull request for the same issue #349, I didn't see that. I had only ten minutes and just pushed it up, may works right away, otherwise I will take more time to do it. If you want to try it, it's on https://github.com/CowboyCode/cordova-plugin-fcm

Cheers

1N50MN14 commented 6 years ago

Thank you @CowboyCode!
So I've been testing the updated fork, the message is unfortunately not arriving to the app BUT the good news is that XCode is not throwing the previous error anymore, the internal implementation seems to be correct it must be a delegation issue to the app itself... what is interesting though is that I'm not seeing the merged changes in your repo!! :)

CowboyCode commented 6 years ago

True, the commit must have failed, I will recommit it tomorrow

1N50MN14 commented 6 years ago

@CowboyCode That would be great thank you!

chrisjpalmer commented 6 years ago

Hi guys. I have forked ostownsville's repo as well just like @CowboyCode. I found a bug completely unrelated to this issue to do with copy google-services.json. I made a PR on ostownsville's repo BECAUSE it seems to be the new central repo for this plugin.

BUT the reason why I am commenting is because if @CowboyCode is going to put that commit up...

True, the commit must have failed, I will recommit it tomorrow

...will you merge with ostownsville?

CowboyCode commented 6 years ago

@chrisjpalmer Sorry for the confusion, I am ostownsville (work) and also blumanski and CowboyCode, too many computers and locations and ssh keys...

I have already added this pull request, regarding the copy google-services.json to the repo. https://github.com/fechanique/cordova-plugin-fcm/pull/370

And https://github.com/fechanique/cordova-plugin-fcm/pull/364 (untested)

You can double check it at the moment under https://github.com/CowboyCode/cordova-plugin-fcm

I will do some testing tomorrow, and then I will merge it into ostownsville and create probably a pull request here.

Cheers

1N50MN14 commented 6 years ago

@CowboyCode Just tested it, unfortunately the same error message still appears :(

Messaging][I-FCM002019] FIRMessaging received data-message, but FIRMessagingDelegate's-messaging:didReceiveMessage: not implemented

chrisjpalmer commented 6 years ago

@CowboyCode Great, thanks for the clarification! Also thank you for contributing to this plugin. We seriously need it!

I can see that you have merged #370 into ostownsville/cordova-plugin-fcm. I actually cloned ostownsville and found a small problem with the way google-services.json is copied. It was only a one liner. I could have it wrong but, the .json started copying properly for me after making the change.

NOW... BACK TO THE ORIGINAL THREAD I would really love to see the FIRMessaging error go away for iOS! I wish I could help you guys but my iPhone is in repair so I have nothing to test on. Best of luck and thank you!

CowboyCode commented 6 years ago

@1N50MN14 @chrisjpalmer I merged it now into ostownsville https://github.com/ostownsville/cordova-plugin-fcm

All is up to date but the error for data messages is still there. There were breaking changes in version 4.0.0 https://firebase.google.com/docs/reference/ios/firebasemessaging/api/reference/Protocols/FIRMessagingDelegate#-messagingdidreceivemessage

I create a new issue for that.

Cheers

chrisjpalmer commented 6 years ago

Hi @CowboyCode could you merge my latest PR?

Thanks

CowboyCode commented 6 years ago

Done :-)

1N50MN14 commented 6 years ago

@CowboyCode @chrisjpalmer Are you experiencing the plugin requesting push permissions on its own right when it first loads or is it just me?

1N50MN14 commented 6 years ago

Ok I see on both Android and iOS the plugin is initialized automatically whenever the webview is ready, then the permission request is triggered; so if there's a splash screen delay this shows up first

Edit: Ok I think replacing the automatic exec() of read with something like

FCMPlugin.prototype.init = function(success, error){
    exec(success, error, "FCMPlugin", 'ready', []); 
}

Should resolve it... what do you think?

CowboyCode commented 6 years ago

Regarding the splash screen delay, don't use a splash screen plugin, add it in android itself and you have no problems.

https://www.bignerdranch.com/blog/splash-screens-the-right-way/

1N50MN14 commented 6 years ago

@CowboyCode Oh I didn't realize that, thanks for the heads up!

By the way, the init() approach works, I think it's a good idea regardless the splash; in my case I didn't want to show the permission alert first thing when users open the app, only after they have signed up & logged in..

1N50MN14 commented 6 years ago

@CowboyCode @chrisjpalmer With the latest update I'm seeing push notifications are no longer showing when the app is closed, was just wondering if you're experiencing a similar issue?

CowboyCode commented 6 years ago

I can't test it before Thursday (AEST), I let you know after

chrisjpalmer commented 6 years ago

Hi CoyboyCode. Troublesome iOS.

I will take a look today.

Regards

Chris

On 1 Oct 2017, at 11:04 pm, Oliver Blum notifications@github.com wrote:

I can't test it before Thursday, I let you know then

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fechanique/cordova-plugin-fcm/issues/365#issuecomment-333372066, or mute the thread https://github.com/notifications/unsubscribe-auth/AcaUOxUxIpS4pFQ1aZr1hxSoxae98ClTks5sn3_ogaJpZM4Ouhg-.

1N50MN14 commented 6 years ago

@chrisjpalmer I've been banging my head against the wall with this one, along with the early permission popup which shows the moment the plugin loads (even though I wrapped the ready call in the plugin);

chrisjpalmer commented 6 years ago

Hi Ayman

The iOS plugin will always display the popup (the way its currently coded), on load. This is because the firebase initialisation routine is fired as soon as the app loads. We could incorporate what you are talking about however.

Chris

On 3 Oct 2017, at 8:05 am, Ayman Mackouly notifications@github.com wrote:

@chrisjpalmer https://github.com/chrisjpalmer I've been banging my head against the wall with this one, along with the early permission popup which shows the moment the plugin loads (even though I wrapped the ready call in the plugin);

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fechanique/cordova-plugin-fcm/issues/365#issuecomment-333665058, or mute the thread https://github.com/notifications/unsubscribe-auth/AcaUO407KIT2Oz1eoJ9jV8vocPZLsKs3ks5soVAvgaJpZM4Ouhg-.

1N50MN14 commented 6 years ago

@chrisjpalmer I think it would be a good idea imho to have a choice on when to initialize firebase, one can choose either on startup or wherever it makes sense.. in my case this has been problematic because the popup is blocking and it appears exactly where I don't want any blocking alerts ;) I'm also a bit concerned Apple may take notice on alerts showing up first thing..

The immediate concern now though is push notifications not showing up on iOS.. it's been stopping me the last few days from pushing the app to the app store :(

chrisjpalmer commented 6 years ago

Hi there. Very sorry. Our latest P.R. has broken the plugin I am guessing.

Please revert to a pervious commit by using: cordova plugin add https://github.com/ostownsville/cordova-plugin-fcm.git#COMMIT_HASH <https://github.com/ostownsville/cordova-plugin-fcm.git#COMMIT_HASH> where commit hash is the commit you would like to use of our plugin.

If using Ionic, prefix with ‘ionic’.

Chris

On 3 Oct 2017, at 9:38 am, Ayman Mackouly notifications@github.com wrote:

@chrisjpalmer https://github.com/chrisjpalmer I think it would be a good idea imho to have a choice on when to initialize firebase, one can choose either on startup or wherever it makes sense.. in my case this has been problematic because the popup is blocking and it appears exactly where I don't want any blocking alerts ;) I'm also a bit concerned Apple may take notice on alerts showing up first thing..

The immediate concern now though is push notifications not showing up on iOS.. it's been stopping me the last few days from pushing the app to the app store :(

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/fechanique/cordova-plugin-fcm/issues/365#issuecomment-333685203, or mute the thread https://github.com/notifications/unsubscribe-auth/AcaUO2Ht6Qi2QQ9LnjQg37r49jCZDVsJks5soWXxgaJpZM4Ouhg-.

1N50MN14 commented 6 years ago

@chrisjpalmer Thank you I'll try to revert 521e288

One question, the plugin's js automatically invokes https://github.com/ostownsville/cordova-plugin-fcm/blob/master/src/ios/FCMPlugin.m#L26 va https://github.com/ostownsville/cordova-plugin-fcm/blob/master/www/FCMPlugin.js#L40 - i wrapped that inside an init call instead, but firebase still somehow managed to run, do you think I missed on something?

chrisjpalmer commented 6 years ago

Hi there. Revert to a8e14ad7691cade14288d302b18e7a4ccb7b0c8a.

I don't think the ready method is linked to anything in code. It probably needs to be removed. If you check out https://github.com/ostownsville/cordova-plugin-fcm/blob/master/src/ios/AppDelegate%2BFCMPlugin.m you'll see that the initialisation routine - (BOOL)application:(UIApplication *)application customDidFinishLaunchingWithOptions:(NSDictionary *)launchOptions { sets up all the firebase stuff and will thus trigger the popup.

1N50MN14 commented 6 years ago

(thank you) right so I see a void method load here https://github.com/ostownsville/cordova-plugin-fcm/blob/master/src/ios/AppDelegate%2BFCMPlugin.m#L26 but I don't see where it's being called is it some sort of constructor?

chrisjpalmer commented 6 years ago

Yes so checkout this forum post on the Objective-C load method.

https://stackoverflow.com/questions/13326435/nsobject-load-and-initialize-what-do-they-do

load() is called by the runtime. load() performs method swizzling which allows us to hook into the app-delegate's applicationDidFinishLaunching method. Then we can init firebase.

1N50MN14 commented 6 years ago

Great, so from the code base point of view what could stop us from having custom initialization? of what I see this is the recommended approach for plugins https://cordova.apache.org/docs/en/latest/guide/platforms/ios/plugin.html

chrisjpalmer commented 6 years ago

Hi there. So I have tested the plugin this morning on iOS 10.3.3.

Notifications Not Working 1) I tested commit a8e14ad to see if the old version worked. At first only foreground notifications would propagate and only through the direct firebase channel (not APNS). This was because i recently changed my firebase app details over: APNS and Firebase had cached my device id for the old firebase app. Upon deletion of the application and restarting the phone, both foreground and background notifications started working. I was able to see the notification dialog pop up and hear the sounds.

2) I tested the most recent commit of the plugin for comparison. It also works => both foreground and background notifications. I am able to hear sounds and see the dialog.

I should stress that in between tests I...

  1. Deleted the app
  2. Rebooted the phone
  3. Recompiled the app with the plugin version I wanted to test
  4. Tested notifications using the Firebase Console notifications test feature
  5. Tested silent and non-silent notifications using my server backend.

Conclusion: Notifications appear to be working (at least from my perspective).

Custom Initialisation I personally think this is a really good idea. Thanks for the documentation from the cordova website. If you would like to have a crack at implementing it then go ahead. Otherwise I might have to postpone this until some future time as I am busy with work.

Hope this helps

Chris

1N50MN14 commented 6 years ago

@chrisjpalmer I finally figured out the issue! It was a bug in iOS 11!!! Upgraded to 11.1 and push notifications started to arrive, damn it Apple!!! Thank you so much for the help!!

Going to look into the init thing now

raghuyt commented 6 years ago

@1N50MN14 Have you got 11.1 or 11.0.1 in my device i have 11.0.2 but this plugin not working

1N50MN14 commented 6 years ago

@raghuyt hmm I just upgraded to 11.0.2, no incoming notifications anymore...

On 11.0.1 I was receiving notifications, but when the app was closed / in background mode, notification data was not being forwarded to the app..

Only @chrisjpalmer or @CowboyCode can rescue us on this one