Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Fix issues #387

Closed
wants to merge 2 commits into from
Closed

Fix issues #387

wants to merge 2 commits into from

Conversation

KingAndroid
Copy link

fix firebase notification issue when background & killed

ZhengLu added 2 commits June 10, 2017 18:41
works well for background & killed
@spstratis
Copy link

spstratis commented Jun 11, 2017

Was this a known issue for background notifications not working on iOS? I've been having issues getting them to display, but no issues when app is in the foreground.

@KingAndroid

@KingAndroid
Copy link
Author

Hi @spstratis
Plugin works on foreground mode and android perfectly.
It does not work only on ios background & killed mode.
Thanks

@spstratis
Copy link

Thanks for the response @KingAndroid. Your merge request here is to fix the issue on iOS I'm assuming?

@KingAndroid
Copy link
Author

Yes, I've resolve two project issue with this PR

@tusharvikky
Copy link

tusharvikky commented Jun 12, 2017 via email

@EddyVerbruggen
Copy link
Owner

Has anyone besides @KingAndroid tested this potential fix?

@EddyVerbruggen
Copy link
Owner

@KingAndroid The only reason why your fix would help is if you've set FirebaseAppDelegateProxyEnabled to NO in your app's plist file - is that the case for your app?

If so, remove it and retest your app.

If not then something else is preventing Firebase from automatically swizzling your appdelegate, most likely because of the order of requiring the plugin and calling init. Can you share your the app that you used to test this PR?

@KingAndroid
Copy link
Author

Test case

  1. FirebaseAppDelegateProxyEnabled does not effort for FCM.
    I've tried with both case -> Yes / No.

  2. You can try sample that I've sent you.
    You can test it with your own sample code.
    Does it work on your sample code ?

Thank you,

@tonyhogben
Copy link

@KingAndroid, @EddyVerbruggen, I am using 4.0.3 of the plugin and have a working Android implementation. I have working foreground messages in iOS but no background notifications or foreground messages once the app has been killed.

I have tried version 3.12.0 and have tried initializing the plugin in my app.ts and the first main page.

I have tried adding @KingAndroid's fix into the firebase.ios.js file...

// Set APNS Token to Firebase server
    appDelegate.prototype.applicationDidRegisterForRemoteNotificationsWithDeviceToken = (app, apnsToken) => {
      FIRInstanceID.instanceID().setAPNSTokenType(apnsToken, FIRInstanceIDAPNSTokenTypeUnknown);
    }

I still have no background or killed notifications.

@EddyVerbruggen
Copy link
Owner

@tonyhogben Care to share your repo so I can reproduce?

@tonyhogben
Copy link

@EddyVerbruggen https://github.com/tonyhogben/whole-school-meals

@EddyVerbruggen
Copy link
Owner

Thanks! Any special commands I need to run, or in-app buttons I need press to reproduce it?

@tonyhogben
Copy link

That version initializes the plugin in the app.ts.

Selecting a school runs subscribeToTopic but this does not appear to work in iOS or Android.

You must select a school to progress, you can skip the allergens on the next screen. Once on the main (menu) page you can use the cog to return to the schools list (which has the push token at the top if you need it).

Is that enough to get you going?

@tonyhogben
Copy link

Hi @EddyVerbruggen, did you have any joy with this?

@EddyVerbruggen
Copy link
Owner

Not yet

@tonyhogben
Copy link

Hey @EddyVerbruggen, sorry to chase you up, but do you have an opinion on the nature of the problem yet? I am being pressured into releasing the app and need to at least know what direction I am going to take where notifications are concerned.

@InvSqr
Copy link

InvSqr commented Jul 20, 2017

Having the same issue of not having the "onMessageReceived" callback fire on iOS when the app is in the background and opened from the notification. Rolling back to 3.12.0 fixed things. Performing an upgrade to 4.+ and the issue reappeared. Just my $0.02

@EddyVerbruggen
Copy link
Owner

@tonyhogben I'm on vacation so things are s-l-ooooo-w atm, sorry for that. I just fixed another issue I'd like to release 4.0.5 for tomorrow and I want to investigate your issue before releasing that. So I will be looking at your repo tomorrow (I already cloned it before going camping so the poor wifi doesn't hamper my tests).

I'll keep u posted!

🏕

@tonyhogben
Copy link

Thanks @InvSqr, I've not been able to get that working though :(

@EddyVerbruggen, awesome, thank you. Fingers crossed you find the issue!

@EddyVerbruggen
Copy link
Owner

Hey @tonyhogben I'm testing your app and can't get a notification show up in the notification center. That never happened to me before and luckily I found the cause: my dev push cert was expired:

screen shot 2017-07-21 at 19 08 48

I'll fix that later today and try again.

@tonyhogben
Copy link

Good catch. I've been using the new 'key' approach rather than a certificate (though I did test using a cert in case it had an effect).

I've never managed to get anything to show up in the iOS notification center and subscribeToTopic does not appear to work in iOS or Android either.

@EddyVerbruggen
Copy link
Owner

AFAIK uploading the APNs cert is required regardless. I will update you if renewing the cert has an effect for me.

@tonyhogben
Copy link

I don't know if you have noticed (I'm sure you've had no reason to even look), but the Google documentation that your plugin documentation links to has changed and now guides you though setting up an APN key rather than a certificate. I believe they are taking advantage of a change at Apple that makes it much easier than the certificate approach (no keychain required) and keys don't expire! ;)

@EddyVerbruggen
Copy link
Owner

Ah, that's useful info, thanks. It's not related to this problem (since sending notifications wasn't the problem anyway - it's showing them in notification center).

I haven't got a clue yet. Will look again tomorrow.

@EddyVerbruggen EddyVerbruggen added this to the 4.0.4 milestone Jul 24, 2017
@EddyVerbruggen
Copy link
Owner

EddyVerbruggen commented Jul 24, 2017

I seem to have found the issue. Not that it makes sense. The call to FIRApp.configure() was too early.

@tonyhogben You can install plugin version 4.0.5 (I'll push it to npm in the next hour or so). But please also change a bit of your code. This bit needs to change in app.ts (so the plugin is "required" before app start, but init runs afterwards):

  app.on("launch", () => {
    if (app.android) {
        initialize();
    }

    // Initialize firebase (Notifications)
    firebase.init({
        onPushTokenReceivedCallback: token => {
          console.log("Firebase push token: " + token);
        },
        onMessageReceivedCallback: message => {
          console.dir(message);
          alert("body: " + message.body);
        }
    }).then(
        (instance) => {
          console.log("firebase.init done");
        },
        (error) => {
          console.log("firebase.init error: " + error);
        }
    );
});

@tonyhogben
Copy link

tonyhogben commented Jul 27, 2017

@EddyVerbruggen, thank you. Just a quick confirmation that this is indeed all working. One caveat though, it only works with an Apple APNs certificate and does not work with an APNs key.

This is important as currently the plugin's Cloud Messaging documentation tells you to follow Google documentation that no longer covers creating a certificate, but instead guides you through creating a key.

Thanks again, great work Eddy.

Edit: Ignore the caveat, Key's work fine

@Marcisbee
Copy link

Marcisbee commented Jul 27, 2017

Yes, it worked for me too. But as for Android, notifications for it broke when I updated to 4.0.5. I never receive anything now from onPushTokenReceivedCallback on Android. On iOS now everything is fine. I use Angular and I wrapped it in "launchEvent" instead of "launch", that's the only difference.

@tonyhogben
Copy link

Sigh. @Marcisbee you appear to be correct. Using the firebase console I can successfully send a message targeting all Android devices using the app, but I cannot send any topic based notifications. @EddyVerbruggen, any ideas?

@EddyVerbruggen
Copy link
Owner

I actually used a key myself now instead of the cert. And since that's serverside it doesn't matter for this plugin anyway.

I didn't change Android stuff in the latest version. Which older Android version (as close to the current one) does work for you?

@tonyhogben
Copy link

@EddyVerbruggen, it appears to be linked to the code change you made in my project regarding the point at which firebase is initialised. When I attempted to subscribe to a topic on Android, I encountered an error...

Error subscribing to topic: Can be run only after init

Based on this I re-wrote my app.ts code as follows...

app.on("launch", () => {
  if (app.ios) {
      initFirebase();
  }
});

if (app.android) {
      initFirebase();
}

function initFirebase() {
  // Initialize firebase (Notifications)
  firebase.init({
      onPushTokenReceivedCallback: token => {
        console.log("Firebase push token: " + token);
      },
      onMessageReceivedCallback: message => {
        //console.dir(message);
        let title: string = "Important Update";
        if (message.title) {
          title = message.title;
        }
        alert({
          title: title,
          message: message.body,
          okButtonText: "Okay"
        }).then(function () {
          //console.log("Dialog closed!");
        });
      }
  }).then(
      (instance) => {
        console.log("firebase.init done");
      },
      (error) => {
        console.log("firebase.init error: " + error);
      }
  );
}

This works, though I am not entirely sure this is the cleanest approach, any suggestions?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants