phonegap-build / PushPlugin

This repository is deprecated head to phonegap/phonegap-push-plugin
https://github.com/phonegap/phonegap-plugin-push
MIT License
1.31k stars 996 forks source link

iOS: onNotificationAPN never fires #221

Open kaansoral opened 10 years ago

kaansoral commented 10 years ago

I'm testing the plugin with cordova 3.4.1-0.1.0 (Other versions cause a "device undefined" issue, this is the only version I can use)

I can verify the notification from the "Notification Received" / "Msg:" logs at XCODE when I click the notification, however onNotificationAPN never fires

Any ideas? (The android version works, the gcmregid/iosapns token routines work on both platforms, the push notifications are received, the only issue is onNotificationAPN)

kaansoral commented 10 years ago

I've narrowed down the issue to the custom notification data, the notifications does work if they are message-only (both on cold start + when the app is active)

I will try to simplify the custom data / inspect what causes the issue / research further and update this issue if I find the cause

kaansoral commented 10 years ago

It turns out, if the custom data has an array, the onNotificationAPN isn't fired, however there are no problems with basic data - such as integers/strings So the solution is to serialize the data into a string I will leave the issue open for others to learn, however feel free to close it

artemave commented 10 years ago

+1

mrsubtle commented 10 years ago

+1 Apple's own Push Notification Guide specifically details on how to use the alert.loc-args property (an array of strings) to define localization strings for the client based on the user's OS locale. This is a defect that may cause us to abandon usage of this plugin unfortunately. See https://developer.apple.com/library/mac/documentation/NetworkingInternet/Conceptual/RemoteNotificationsPG/Chapters/ApplePushService.html#//apple_ref/doc/uid/TP40008194-CH100-SW20 for more information.

If it helps to troubleshoot this issue, the PushPlugin seems to return a malformed array when parsing an array of strings.

It returns the following JSON (as an example):

    "loc-key":"PUSH_TEAM_SELECTION",
    "loc-args":"(
        "Richard Bennett",
        "Footy Lions",
        "Reserves"
    )",
mrsubtle commented 10 years ago

So, because I'm a pain in the butt, I fixed this issue locally.

Replace the notificationReceived Method in "PushPlugin.m" with this one:

- (void)notificationReceived {
    NSLog(@"PushPlugin_NOTIFICATION_RECEIVED");
    //NSLog(@"PushPlugin_NOTIFICATION: %@", notificationMessage);
    if (notificationMessage && self.callback)
    {
        NSError *error;
        NSData *jsonData = [NSJSONSerialization dataWithJSONObject:notificationMessage
                                                           options:NSJSONWritingPrettyPrinted // Pass 0 if you don't care about the readability of the generated string
                                                             error:&error];
        if (! jsonData) {
            NSLog(@"PushPlugin_ERROR: %@", error);
        } else {
            NSString *jsonStr = [[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding];

            NSLog(@"PushPlugin_JSON: %@",jsonStr);

            NSString * jsCallBack = [NSString stringWithFormat:@"%@(%@);", self.callback, jsonStr];
            [self.webView stringByEvaluatingJavaScriptFromString:jsCallBack];
        }
        self.notificationMessage = nil;
    }
}

This will transform your callback data to be the UNTOUCHED push notification data. It will facilitate JSON responses like this one:

{
  "aps": {
    "alert": {
      "loc-args": [
        "Mayer Hawthorne"
      ],
      "body": "Mayer Hawthorne liked your post",
      "loc-key": "PUSH_FEED_LIKE_USERPOST"
    },
    "sound": "alert.aifc",
    "badge": 15
  },
  "data": {
    "key": "FEED_ITEM",
    "value": "/profile/RichardBennett/activities/yq9se/"
  }
}

The above APNS payload will allow you to follow the localization guidelines set forth by Apple to make your Phonegap Push Notifications Localizable. And you will also be able to use custom data payloads containing Arrays.

Yay!

kaansoral commented 10 years ago

Thanks a lot for sharing your custom fix, It's good to know an easily applicable solution exists Manually modifying the plugins create a lot of overhead, so let's hope the solution gets adopted into the plugin

sayleeatdure commented 10 years ago

@mrsubtle thanks for the quick fix, I tried it and still no luck. Is there any other suggestion?

mrsubtle commented 10 years ago

It was definitely the malformed array that was causing my onNotificationAPN call back to fail.  I would suggest running your code surrounding the push execution through JSLint or some other validation tool.

Richard Bennett rich@n2d.co   |    http://n2d.co/   |    @mr_subtle   |    Behance   |    Zerp.ly    |    Scoutzie

From: sayleeatdure notifications@github.com Reply: phonegap-build/PushPlugin reply@reply.github.com Date: June 2, 2014 at 5:13:07 AM To: phonegap-build/PushPlugin pushplugin@noreply.github.com Cc: Richard Bennett rich@n2d.co Subject:  Re: [PushPlugin] iOS: onNotificationAPN never fires (#221)

@mrsubtle thanks for the quick fix, I tried it and still no luck. Is there any other suggestion?

— Reply to this email directly or view it on GitHub.

sayleeatdure commented 10 years ago

nope. That did not help too.

mrsubtle commented 10 years ago

After a lot of toying with this, here are my final changes to _PushPlugin.m_:

  1. Update the _notificationReceived_ method:

    - (void)notificationReceived {
    NSLog(@"PushPlugin_NOTIFICATION_RECEIVED");
    //NSLog(@"PushPlugin_NOTIFICATION: %@", notificationMessage);
    if (notificationMessage && self.callback)
    {
       NSMutableDictionary *editableNotification = [notificationMessage mutableCopy];
       NSError *error;
    
       if (isInline) {
           [editableNotification setObject:@"1" forKey:@"foreground"];
           isInline = NO;
       }
               else
           [editableNotification setObject:@"0" forKey:@"foreground"];
    
       NSData *jsonData = [NSJSONSerialization dataWithJSONObject:editableNotification
                                                          options:NSJSONWritingPrettyPrinted // Pass 0 if you don't care about the readability of the generated string
                                                            error:&error];
           if (! jsonData) {
               NSLog(@"PushPlugin_ERROR: %@", error);
           } else {
               NSString *jsonStr = [[NSString alloc] initWithData:jsonData encoding:NSUTF8StringEncoding];
    
               NSLog(@"PushPlugin_JSON: %@",jsonStr);
    
               NSString * jsCallBack = [NSString stringWithFormat:@"%@(%@);", self.callback, jsonStr];
               [self.webView stringByEvaluatingJavaScriptFromString:jsCallBack];
           }
    
       self.notificationMessage = nil;
    }
    }

    This update keeps the payload intact, and also adds a name/value pair "foreground" to the dictionary that is returned to your JavaScript. Like the original plugin, this will tell your app whether or not the notification was received when the app is running in the foreground, or as a background process. Not sure why I kept the value as a string. Maybe I was lazy.

  2. Comment out the _parseDictionary_ method entirely. We don't want the plugin to modify the payload at all anymore.
  3. Enjoy your custom payload.

Cheers all.

EddyVerbruggen commented 10 years ago

Hi @sayleeatdure, can you tell us whether or not the last change by @mrsubtle solves your issue? I'd like to apply this fix any day now, but (besides my own tests), I'd like to know your findings as well.

aaronyo commented 10 years ago

Thanks @mrsubtle.

@EddyVerbruggen, it works for me.

I made a pull request here: #290. It is based on @mrsubtle's code.

mrameezraja commented 9 years ago

Try making your ecb callbacks global for ios and android. i.e window.onNotificationAPN = function(){ ... } window.onNotification = function(){ ... }