Skip to content

Update MessagingService.java - #631

Open
BlueBeetle13 wants to merge 1 commit into
evollu:masterfrom
BlueBeetle13:patch-1
Open

Update MessagingService.java#631
BlueBeetle13 wants to merge 1 commit into
evollu:masterfrom
BlueBeetle13:patch-1

Conversation

@BlueBeetle13

Copy link
Copy Markdown

Allow the app to receive notifications from the FCMEvent.Notification event in background mode even if the notification contains the 'notification' object. Normally this is intercepted by Firebase and the react native app will not receive notifications while the app is in background mode.

Allow the app to receive notifications from the `FCMEvent.Notification` event in background mode even if the notification contains the 'notification' object. Normally this is intercepted by Firebase and the react native app will not receive notifications while the app is in background mode.
@evollu

evollu commented Oct 31, 2017

Copy link
Copy Markdown
Owner

great. any docs when will this method be called?

@BlueBeetle13

Copy link
Copy Markdown
Author

This overrides the Firebase handleIntent method. Normally Firebase will consume the notification if the notification object is present. If not then is passes the notification through. I am bypassing this and passing everything through.

So this will get called when every notification arrives.

My situation was that I was providing large and small icons for the notification. And the back end (Firebase) is passing the notification object on the notification so it works with iOS and Android without having to send 2 versions of the message. So notifications in the notification tray on Android looked different depending on if the notification is received while the app is in foreground or background. This normalizes the process so they look the same. When you call the presentLocalNotification method you need to remember to pass any payload items (send from Firebase in data) so they will get passed through when the user clicks the notification from the tray.

@evollu

evollu commented Oct 31, 2017

Copy link
Copy Markdown
Owner

this seems to be a breaking behavior which may cause FCM not work properly. For example this code

if(var2.equals("com.google.firebase.messaging.NOTIFICATION_DISMISS")) {
                var4 = 1;
            }

may get skipped?

The question is when will intent.getExtras() to be not null?

@BlueBeetle13

Copy link
Copy Markdown
Author

Do you propose moving the conditional from line 65 to wrap lines 69-72?

@BlueBeetle13

Copy link
Copy Markdown
Author

In any case, any failure calls the super class function, which would then be the default behaviour (if this code was never added).

@huynhtansi

Copy link
Copy Markdown

I have same idea but in my test case, intent.getExtras() contains "gcm.notification.badge" instead of "badge". So your code might show notification but didn't parallel with handleBadge method.

I agree with @evollu it might make FCM not work properly in some case. So i think we should only override handleIntent for updating badge number.

This is my code in test case

@Override
public void handleIntent(Intent intent) {
        super.handleIntent(intent);
        if (intent.getExtras() != null && intent.getExtras().containsKey("gcm.notification.badge")) {
            Object badgeNumber = intent.getExtras().get("gcm.notification.badge");
            if (badgeNumber != null) {
                new BadgeHelper(this).setBadgeCount(Integer.parseInt(badgeNumber.toString()));
            }
        }
    }

@evollu

evollu commented Nov 3, 2017

Copy link
Copy Markdown
Owner

how about

@Override
public void handleIntent(Intent intent) {
        super.handleIntent(intent);
        // move both broadcasting and badge logic here
        // concern:  there are other events that removes a notification etc. don't know if it contains extra data. but if developer filters intent based on customized key it shouldn't trigger any bug.
}

@derit

derit commented Nov 21, 2018

Copy link
Copy Markdown

this patch not work with firebase latest version

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.

4 participants