OneSignal / react-native-onesignal

React Native Library for OneSignal Push Notifications Service
Other
1.57k stars 374 forks source link

[Android] Opening app with deep linking enabled via push notification containing Launch URL causes app to restart #1194

Closed tingzhouu closed 2 years ago

tingzhouu commented 3 years ago

Description:

I have an app with deep linking enabled, following react-navigation's instructions here by adding an additional intent-filter in AndroidManifest.xml.

The app is not running in the background. Opening the app via a push notification containing Launch URL will cause the app to restart when I go to the home screen and open the app again, meaning that the app goes from running in the background to running in the foreground.

Emphasizing on how to recreate this:

  1. App is opened via push notification, must not be running in the background before opening the push notification.
  2. Push notification contains Launch URL.
  3. Launch URL's scheme must be the same as the scheme set in AndroidManifest.xml

Environment

OneSignal React-Native SDK version: 4.0.6 React-Native version: 0.64.0 SDK added using: yarn Devices used: Pixel 4a, Samsung Galaxy S10+

Steps to Reproduce Issue:

  1. Start a new React Native project - npx react-native init abcdefg
  2. Add OneSignal SDK using yarn - yarn add react-native-onesignal and follow standard installation instructions here
  3. Modify android/app/src/main/AndroidManifest.xml with below snippet to enable deep linking
<intent-filter>
      <action android:name="android.intent.action.VIEW" />
      <category android:name="android.intent.category.DEFAULT" />
      <category android:name="android.intent.category.BROWSABLE" />
      <data android:scheme="abcdefg" />
</intent-filter>
  1. Install app on device
  2. Ensure that app is not running in the background, meaning app is killed.
  3. Send push notification containing Launch URL, where the scheme of Launch URL is the same as the scheme set in Step 3. In this example, the Launch URL could be abcdefg:// or abcdefg://anythinggoes.
  4. Open push notification
  5. Get app to run in the background by going to the home screen of the device or pressing the 'App Overview' button.
  6. Open app again

Repeat steps 8 and 9 to reproduce the behavior.

Expected behavior: App to open and nothing else happens Actual behavior: App opens and restarts

Anything else:

Example Project: example_project.zip

Screen Recording: video file

rgomezp commented 3 years ago

Howdy, Can you please include your full manifest?

Make sure you have android:launchMode="singleTask"> as well.

Furthermore, please include your JS code for actually handling the opened event.

Cheers

tingzhouu commented 3 years ago

Hello! I have included in my issue a zip file of the project, link here if it was missed out.

There is no JS code handling the event.

Full manifest below, android:launchMode="singleTask" is present.

<manifest xmlns:android="http://schemas.android.com/apk/res/android"
  package="com.awesometest">

    <uses-permission android:name="android.permission.INTERNET" />

    <application
      android:name=".MainApplication"
      android:label="@string/app_name"
      android:icon="@mipmap/ic_launcher"
      android:roundIcon="@mipmap/ic_launcher_round"
      android:allowBackup="false"
      android:theme="@style/AppTheme">
      <activity
        android:name=".MainActivity"
        android:label="@string/app_name"
        android:configChanges="keyboard|keyboardHidden|orientation|screenSize|uiMode"
        android:launchMode="singleTask"
        android:windowSoftInputMode="adjustResize">
        <intent-filter>
            <action android:name="android.intent.action.MAIN" />
            <category android:name="android.intent.category.LAUNCHER" />
        </intent-filter>

        <intent-filter>
          <action android:name="android.intent.action.VIEW" />
          <category android:name="android.intent.category.DEFAULT" />
          <category android:name="android.intent.category.BROWSABLE" />
          <data android:scheme="awesometest" />
        </intent-filter>
      </activity>
    </application>
</manifest>
rgomezp commented 3 years ago

Howdy, We will try reproducing as soon as possible. Thank you for your patience.

rgomezp commented 3 years ago

Howdy, So after looking into this further, I don't exactly see the problem. When you open the app from cold start, it will go through the normal initial launch process - this includes the splash screen or anything you might have set up during loading.

When you background it and bring it back, that has nothing to do with the deep link from the notification -- so if it is reloading, you're going to have to figure out why. But I don't see how that's OneSignal related.

In terms of how to properly deep link and route your app to the correct screen, we recommend using additional data and routing to the correct screen using something like React Router, in this case.

See the following documentation for information on how to parse the additional data payload and routing to the correct page/screen using that value: https://documentation.onesignal.com/docs/links#deep-linking-with-additional-data

Cheers

ghost commented 3 years ago

Hello @rgomezp, we are struggling with the same problem here

We have Android DeepLinks configured and they are working properly in our app. We use de "Launch URL" param to send the DeepLink with the push and open the app with it. When opening the app with the push with a Launch URL set it opens the app and navigates properly, but, when we go to the background and come back, the app seems to be fully reloaded. This doesn't happen when opening the app only with a DeepLink. It seems the same scenario as @tingzhouu has related above.

I would like to understand why we have a "Launch URL" but we cannot use it to launch the app. What are the pros of using additional data as you said above? For me, the docs are not clear about it.

tingzhouu commented 3 years ago

Hello @emilioheinz, I discovered a workaround for this issue.

Under step 6 of my Steps to Reproduce Issue:, use a different prefix from what you have specified in your AndroidManifest.xml.

If your AndroidManifest.xml uses <data android:scheme="awesometest" />, use awesometest-notif instead. Example Launch URLs could be: awesometest-notif:// or aweometest-notif://anythinggoes.

You will then have to handle for the prefix used in your Launch URL, which in this example is awesometest-notif.

tingzhouu commented 3 years ago

Hello @rgomezp, the re-launch happens only when the app is opened via a push notification containing a Launch URL. I don't know if this is React Native specific, or can also be reproduced on Android natively.

A suggestion I would make is to include this in the documentation to warn developers of this. I'm okay to close this issue as there is a workaround.

ghost commented 3 years ago

I don't think we can close it, since there is a issue and it still happening.

ghost commented 3 years ago

@tingzhouu About your workaround, I think it is almost the same as using aditional data instead of launch url.

ghost commented 3 years ago

I've figured something out. When we use additional data instead of Launch URL param and do this:

OneSignal.setNotificationOpenedHandler(openedEvent => {
    const { notification } = openedEvent

    Linking.openURL(notification.additionalData.deepLink)
})

The same bug happens, I think it can be related to Linking.openURL being called inside the app, the strange thing is that if I call Linking.openURL(url) on a onPress of a button, this doesn't behave like this.

rgomezp commented 3 years ago

@emilioheinz , I'm curious as to what happens if you wrap the Linking line in a 5 sec setTimeout.

Furthermore, I would try to use something more like React Router for routing the user to the correct page after the app opens rather than the Linking API which is most likely the cause of this reloading problem in my opinion.

ghost commented 3 years ago

@rgomezp Yes, we did it with the additional data param, and passed the link to our NavigationService. Basically, we reused almost every code that we were running when the app was opened with a Link. It is working fine.

I'm curious as to what happens if you wrap the Linking line in a 5 sec setTimeout.

I'll try to test it and post here the results!

rgomezp commented 3 years ago

Any update @emilioheinz ?

ghost commented 3 years ago

I'll try to take a look at this before next Sunday 😄

ghost commented 3 years ago

@rgomezp Sorry for the delay. I've tested it in my application. Wrapping Linking line in a 5 sec setTimeout solves the problem, but for end-users this behavior is not the best, as they need to wait 5 sec every time they open a notification with DeepLink.

If there is anything else that I can help you to find a better solution, let me know!

rgomezp commented 3 years ago

@emilioheinz , Thanks for the reply.

That's certainly interesting but it's just what I suspected. It looks like this is a React Native issue.

Unfortunately this won't be fixed until RN solves the problem. However, you could always try the following workaround options:

  1. try reducing the setTimeout to see the lowest time it will still work understanding that it is probably not the optimal solution
  2. try using a different routing mechanism such as via React Router

Hope this helps.

Closing the issue given the core problem is in the issue mentioned above.

Cheers

ghost commented 3 years ago

@rgomezp we are using the second option and it works fine. You said that it looks like it is a React Native issue. Could you please detail your thoughts? It may help us to open an issue on React Native repository.

rgomezp commented 3 years ago

See the link in my previous comment for more details

ghost commented 3 years ago

Nice, thank you!

GiorgioBertolotti commented 3 years ago

Hello @rgomezp, I'm still running into this issue and I'm starting to think it is related to how the Intent is started from the OneSignal's Android SDK.

I think it's related to OneSignal because, as shows the GIF in the original post, the app is completely restarted when it comes in foreground, so this must not be related to React Native's Linking, it feels way more like an Android native thing.

This is the Android logcat related to the app being bring in foreground: logcat.

Also previously I had a workaround based on a native module that launched the app with a custom Intent, but lately it's starting to show some problems. This is the custom Intent I used (which solved the restart problem):

OneSignal.setNotificationOpenedHandler(
  new OneSignal.OSNotificationOpenedHandler() {
    @Override
    public void notificationOpened(OSNotificationOpenedResult result) {
      try {
        Intent intent = new Intent(Intent.ACTION_VIEW);
        intent.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT | Intent.FLAG_ACTIVITY_NEW_TASK);
        intent.setData(Uri.parse(result.getNotification().getLaunchURL()));
        intent.putExtra("openURL", result.getNotification().getLaunchURL());
        intent.putExtra("data", result.getNotification().getAdditionalData().toString());
        Context context = mReactApplicationContext.getCurrentActivity();
        context.startActivity(intent);

        // Emit event
        mContext
          .getJSModule(DeviceEventManagerModule.RCTDeviceEventEmitter.class)
          .emit("OneSignal-remoteNotificationOpened", RNUtils.jsonToWritableMap(result.toJSONObject()));
      } catch(Exception e) {
      }
    }
});

PS: As a temporary workaround I'm following @emilioheinz example by launching the URL from additionalData, but it feels dirty not to use the notification's launchUrl. Also it involves using a setTimeout which just feels wrong.

jbernhardt23 commented 2 years ago

I'm having this exact same issue and I'm not using Linking.openURL to handle the launchURL. I took a look at the React Native Issue mentioned above and applied the manual fixed suggested in that thread but the app keeps re-starting, so it does not seem that the Linking API is causing it (I'm not even using it anywhere in my code). Using a different URI_SCHEME from the one declared in the AndroidManifest as prefix in the URL or not using launchURL at all seems to work, so I'm wondering if it is indeed something related to the native sdk or it is a requirement not to use the same URI_SCHEME as part of the launchURL, which wouldn't make too much sense.

EDIT

I also have app links enabled in my app, if I set launchURL to be a URL from my website, it also causes the app to re-start.

jkasten2 commented 2 years ago

In the react-native-onesignal 4.3.2 Release we changed the Android Activity launch implementation to use Notification Reverse Activity Trampolining. This may effect how the app is started and resumed. It possible this could have fixed this issue but it hasn't been tested to confirm.

rgomezp commented 2 years ago

For those still seeing this issue, please upgrade to at least 4.3.2 as mentioned in the above comment.

Cheers

jbernhardt23 commented 2 years ago

I have upgraded to 4.3.5 but the issue still happens. So far the only way to avoid the app from re-starting is to not use the Launch URL

jbernhardt23 commented 2 years ago

Worth mentioning that if you still need to use the Launch URL for backward compatibility, you can add <meta-data android:name="com.onesignal.suppressLaunchURLs" android:value="true"/> to the AndroidManifest and this also avoids the app to re-start. This way, on the new version of your app you can use the Additional fields and also the Launch URL for users that do not upgrade right away.

https://documentation.onesignal.com/docs/links#example-suppress-launch-urls

rgomezp commented 2 years ago

@jbernhardt23 , Thanks for the info. I'll leave this open for now even though it sounds like you may have found a good temporary workaround.

jkasten2 commented 2 years ago

This may be resolved after the following PR https://github.com/OneSignal/OneSignal-Android-SDK/pull/1598 lands into a new release of react-native-onesignal

kesheshyan commented 2 years ago

Closing for now. Feel free to reopen if the issue persists.

siddharth-kt commented 2 years ago

This issue is still happening any workaround please ? @rgomezp