Pushwoosh / pushwoosh-phonegap-plugin

Pushwoosh PhoneGap Build Plugin
Other
109 stars 139 forks source link

iOS: Listeners conflict with @capacitor/local-notifications #392

Closed zarko-tg closed 6 months ago

zarko-tg commented 10 months ago

Basically, looking at the surface of things, having the Pushwoosh plugin running next to the @capacitor/local-notifications makes the listeners of the latter stop working on Capacitor iOS. @capacitor/local-notifications "localNotificationReceived" & "localNotificationActionPerformed" are kinda important from a standpoint of a user/UI flow so I'm hoping this can get fixed or worked around in either of the plugins.

As wfhm put it nicely in https://github.com/Pushwoosh/pushwoosh-ios-sdk/issues/94#issuecomment-1706602867 :

It looks like both @capacitor/local-notifications and pushwoosh-cordova-plugin plugins try to use methods of UNUserNotificationCenterDelegate, and the Capacitor plugin doesn't expect other plugins to swizzle these methods (like our plugin does). So the swizzled versions of these methods do not trigger callbacks of the local-notifications plugin at all. With Ionic Capacitor you should still have access to the native iOS project, so theoretically you can manually update your AppDelegate to let both plugins access UNUserNotificationCenterDelegate methods, however it might depend on the local-notifications implementation of these methods in their plugin.

A reproduction code/project is available with the comment here: https://github.com/Pushwoosh/pushwoosh-ios-sdk/issues/94#issuecomment-1693204785

wfhm commented 9 months ago

@zarko-tg I apologize for such a delayed response. I'll look into the @capacitor/local-notifications plugin and try to create an example project for you.

Stay tuned!

zarko-tg commented 8 months ago

@wfhm That'd be fantastic.

wfhm commented 8 months ago

@zarko-tg Okay, so there is a workaround but it is quite complicated, and our standard approach to resolving delegate conflicts is not applicable.

Right now, if you disable Pushwoosh notification delegate directly in the plugin, you are still left with Capacitor delegate overwriting your app's delegate during CapacitorBridge initialization, and it happens when Capacitor core is initialized, not just the plugin. So we cannot just create a custom router delegate that will distribute notifications between Pushwoosh and local-notifications plugin without messing with Capacitor initialization.

The solution I ended up with is creating a helper class inside the local-notifications plugin that will receive pushes and forward it to Pushwoosh handler methods.

Screenshot 2023-10-09 at 19 56 45 Screenshot 2023-10-09 at 19 45 50

That should be enough to make Capacitor's NotificationRouter class serve as a single delegate in your app that can also receive Pushwoosh notifications. However, for some reason, received push notifications generate a local notification with the same content, so, to work around it, you can also modify the the code of Capacitor/NotificationRouter class by adding the following check to the willPresent:withCompletionHandler method:

public func userNotificationCenter(_ center: UNUserNotificationCenter,
                                       willPresent notification: UNNotification,
                                       withCompletionHandler completionHandler: @escaping (UNNotificationPresentationOptions) -> Void) {
        let presentationOptions: UNNotificationPresentationOptions?

        if notification.request.trigger?.isKind(of: UNPushNotificationTrigger.self) == true {
            presentationOptions = pushNotificationHandler?.willPresent(notification: notification)
        } else {
            if let userInfo = notification.request.content.userInfo["pw_push"] {
                return
            }
            presentationOptions = localNotificationHandler?.willPresent(notification: notification)
        }

        completionHandler(presentationOptions ?? [])
    }

You can also find its code in the archive below.

Archive.zip

wfhm commented 8 months ago

We are planning to rework delegate swizzling in our plugins to make resolving such conflicts much easier, so I will keep this issue open until there is any news.

enginseer-dev commented 7 months ago

@zarko-tg The conflict was fixed in the new version of the plugin: https://github.com/Pushwoosh/pushwoosh-phonegap-plugin/releases/tag/8.3.17

Please feel free to message us, if you have any follow-up questions.

zarko-tg commented 7 months ago

Hi @wfhm, Firstly, apologies for not being able to check the workaround due to time constraints. I'll be checking out the new version for iOS but we now get an (unrelated?) Android failure:

> Task :app:processReleaseMainManifest FAILED
/Users/foo/bar/android/app/src/main/AndroidManifest.xml Error:
        uses-sdk:minSdkVersion 23 cannot be smaller than version 26 declared in library [com.pushwoosh:pushwoosh-inbox-ui:6.7.0] /Users/foo/.gradle/caches/transforms-3/0f75b005e76e163215622787ea7bc654/transformed/pushwoosh-inbox-ui-6.7.0/AndroidManifest.xml as the library might be using APIs not available in 23
        Suggestion: use a compatible library with a minSdk of at most 23,
                or increase this project's minSdk version to at least 26,
                or use tools:overrideLibrary="com.pushwoosh.inbox.ui" to force usage (may lead to runtime failures)

See https://developer.android.com/r/studio-ui/build/manifest-merger for more information about the manifest merger.

FAILURE: Build failed with an exception.

I don't think we can move (so high) from 23 for the time being. And we don't even use pushwoosh-inbox-ui. What'd be the right thing to do about it?

wfhm commented 6 months ago

@zarko-tg please let me check if we can downgrade minSdkVersion and I'll get back to you shortly.

wfhm commented 6 months ago

@zarko-tg Please try the 8.3.18 release - minSdkVersion was reverted back to 23.

akidisdev commented 6 months ago

@zarko-tg Small update. Please, try the 8.3.19 release.

zarko-tg commented 6 months ago

Thanks for that, it seems to have solved the problem!