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

1.4.x not showing notification on Android #299

Closed
AdriVanHoudt opened this issue Nov 3, 2015 · 23 comments
Closed

1.4.x not showing notification on Android #299

AdriVanHoudt opened this issue Nov 3, 2015 · 23 comments
Labels

Comments

@AdriVanHoudt
Copy link
Contributor

Sending the exact same payload. 1.3 works fine, 1.4 just call the .on('notification, fn) with all keys in additionalData

@AdriVanHoudt
Copy link
Contributor Author

so after changing body to message I get

{
    "image": "urltopicture.jpg",
    "message": "test",
    "additionalData": {
        "summaryText": "There are %n% new notifications",
        "gcm.notification.notId": "121",
        "gcm.notification.title": "Title 1",
        "id": "121",
        "tag": "121",
        "style": "inbox",
        "coldstart": false,
        "collapse_key": "org.company.app",
        "foreground": false
    }
}

The message should be body see https://developers.google.com/cloud-messaging/concept-options#notifications_and_data_messages
Also not sure why title and notId are not being properly parsed?

@AdriVanHoudt
Copy link
Contributor Author

Shouldn't this break for more people? :O

@macdonst macdonst added the retest label Nov 3, 2015
@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt can you send me an example of data you are sending to GCM. That will really help me debugging this.

@smdvdsn
Copy link
Collaborator

smdvdsn commented Nov 3, 2015

Also @AdriVanHoudt do you see different results if the app is closed vs forground vs background

@macdonst I suspect this is caused by the GcmListenerService code. I think that it displays the notification if there is a gcm.notification.title or gcm.notification.body key in the message. I wonder if it is bypassing our normlize in those cases?

@AdriVanHoudt
Copy link
Contributor Author

@smdvdsn I see nothing, only difference is that when in foreground it immediately handles the message and otherwise it waits for the app to open

this is (roughly, I use a lib which accepts the registration ids as a different param) what I send to gcm:

{
    data: undefined,
    notification: {
        title: 'Title 1',
        body: 'test',
        image: 'http: //urltopicture.jpg',
        id: 121,
        tag: 121,
        style: 'inbox',
        summaryText: 'Thereare%n%newnotifications',
        notId: 121
    }
}

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt thanks, I can reproduce it now using node-gcm to send a push with your test data. Stay tuned.

@AdriVanHoudt
Copy link
Contributor Author

Cool, I will, also debugging this myself (at least trying :D)

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt it's something weird when we iterate through the key set of the extras coming in from GCM. For some reason we skip a few.

@AdriVanHoudt
Copy link
Contributor Author

jup managed to get this out of debugger:

D/PushPlugin_GCMIntentService(26109): onMessage - from: 128469695999
D/PushPlugin_GCMIntentService(26109): mormalize extras
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.image
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.image with image
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.style
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.style with style
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.summaryText
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.summaryText with summaryText
D/PushPlugin_GCMIntentService(26109): key = image
D/PushPlugin_GCMIntentService(26109): key = style
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.body
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.body with body
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.tag
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.tag with tag
D/PushPlugin_GCMIntentService(26109): key = gcm.notification.id
D/PushPlugin_GCMIntentService(26109): replace key gcm.notification.id with id
D/PushPlugin_GCMIntentService(26109): key = collapse_key

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt @smdvdsn I'm pretty sure it is a concurrent modification problem. When the body property is replaced with message it screws up the iterator we are using. Testing a fix now.

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt @smdvdsn Yup, that was it. Can you test branch issue299. I'm getting on a plane for Europe in a couple of hours and would like to push a fix if possible but it would be good if someone else took a look at this as well.

@AdriVanHoudt
Copy link
Contributor Author

on it

@AdriVanHoudt
Copy link
Contributor Author

it is working now, big thank you @macdonst for fixing this so quickly, you doing a patch release now?

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

Yes, I will do a patch and release to NPM in the next 10 minutes. It didn't surface in my testing as I wasn't mixing data and notification.

@macdonst
Copy link
Member

macdonst commented Nov 3, 2015

@AdriVanHoudt published version 1.4.2

@macdonst macdonst closed this as completed Nov 3, 2015
chaffeqa added a commit to sportstech/phonegap-plugin-push that referenced this issue Nov 3, 2015
* 'master' of github.com:phonegap/phonegap-plugin-push:
  Version 1.4.2
  Issue phonegap#299: 1.4.x not showing notification on Android
  Version 1.4.1
  Issue phonegap#295: data.registrationId is empty string "" on register event callback
  Issue phonegap#291: Reregister on Android
  [typo] fixing .finish() example
  Update CHANGELOG
  Version 1.4.0
  Issue phonegap#93: Receive Notifications in Background
@telemakhos
Copy link

This should be in the readme or in a visible place... I've spent several hours until I found this fix...

@AdriVanHoudt
Copy link
Contributor Author

How so? This fix is to make it working like it should/like in the readme

@telemakhos
Copy link

I mean, just adding a little section with notices for regressions and breaking changes in the readme (the first thing one reads when arrives to the repo) so newcomers don't spend 5 hours banging their heads against the wall before realising they have to install the plugin via git clone.

@AdriVanHoudt
Copy link
Contributor Author

This fix was published as 1.4.1 so need to git clone, are you allowing pathes in your config.xml? I have it set at 1.4.x. It is possible you need to reinstall the plugin to get the right version

@telemakhos
Copy link

I've been using this plugin only since yesterday. First I installed the plugin via

cordova plugin add phonegap-plugin-push (stable)

and I wasn't receiving the notifications, then after finding this issue I finally installed with

cordova plugin add https://github.com/phonegap/phonegap-plugin-push (unstable)

and started working. I was just just suggesting to include a section with references to major breaking changes (like this one) in the readme of the repo, maybe pointing out to the corresponding issues...

@AdriVanHoudt
Copy link
Contributor Author

hmm weird I have <plugin name="phonegap-plugin-push" spec="1.4.x" /> in my config.xml and that works fine. I personally see no point in listing issues in the readme, this is fixed if you still have a problem I would suggest opening another issue about the fix not working when installing via the cordova command line

@macdonst
Copy link
Member

macdonst commented Nov 9, 2015

@telemakhos Yeah, I don't think we need to add it to the README as it is already fixed. Whenever I do a minor release I do a blog post with all the changes/fixes.

It's really odd that cordova plugin add phonegap-plugin-push did you get you the latest 1.4.2 release which includes the fix. Can you try again and do cordova plugin ls to see what version it installs?

@fredgalvao fredgalvao mentioned this issue Nov 22, 2016
21 tasks
@lock
Copy link

lock bot commented Jun 5, 2018

This thread has been automatically locked.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

4 participants