telenordigital / connect-android-sdk

Android SDK for CONNECT ID
https://telenordigital.github.io/id-docs.telenordigital.com/
Other
16 stars 14 forks source link

Chrome custom tab no longer works (most of the time) #133

Closed rasmusohrstig closed 5 years ago

rasmusohrstig commented 6 years ago

Recently we seem to be falling back on the SDK:s built in web solution (ConnectActivity, WebView) on most devices almost all the time. We have seen this problem on Samsung Galaxy S7, S8 with Android 8 and Google Pixel 1 with Android 9.

I have tried to warm up the Chrome custom tab service and waiting before attaching the button to the layout, but it does not seem to help.

The if clause on line 80 in CustomTabsHelper seems to always be false. So in other words the PackageManager is not able to resolve the Intent.

rasmusohrstig commented 6 years ago

By they way, it would be a good idea to move the Intent resolution from View.onAttachedToWindow() to when the user presses the button. That would make it possible to warm up Chrome custom tabs before the Intent resolution code runs.

simonnorberg commented 6 years ago

We're using custom tabs from support library 28 with the new androidx package. https://developer.android.com/jetpack/androidx/

implementation "androidx.browser:browser:1.0.0"

I think in our app Gradle forces Connect SDK to use this version as well.

jorunfa commented 6 years ago

Seems I'm able to recreate the problem.

Androidx looks good - didn't know about it before now. One thing currently stopping migration is that it requires gradle:3.2.0, and upgrading to this causes a break with the bintray plugin we are using. Luckily there seems to be a fix for that coming along: https://github.com/novoda/bintray-release/issues/213#issuecomment-426579155

In the meantime I will see if I can find another fix for the problem.

jorunfa commented 6 years ago

There is one known problem I've seen before: image

The resolved activity for the intent is the ResolverActivity which, if I'm not mistaken, is the activity for choosing which app you want to open the intent in. This happens if you have multiple apps capable of opening the same uri. Do you have multiple apps using the same uri scheme installed on the device?

Can you debug this line and check? https://github.com/telenordigital/connect-android-sdk/blob/master/connect/src/com/telenor/connect/ui/ConnectLoginButton.java#L78

jorunfa commented 6 years ago

I see we're also using support library 27.something in the SDK. Can bump it to 28, but would be nice to know if that resolves the issue.

jorunfa commented 6 years ago

Can you try

$ git clone git@github.com:telenordigital/connect-android-sdk.git
$ cd connect-android-sdk/
$ git checkout custom-tabs
$ open -a /Applications/Android\ Studio.app .
(run id-example)

And see if that works? And if doing $ git checkout master or git checkout v1.8.0 and running the example app isn't working?

rasmusohrstig commented 6 years ago

I tested your branch and it actually works. I also tried migrating to AndroidX in the same branch and it still works after that. So that's good.

I'm beginning to think that we have done something else in our app that has caused it to not work for us, becuase I tried checking out a version of our own app from a couple of weeks ago and that also worked.

I will keep looking for what it might be. I suspect it could be something to do with our Proguard setup.

rasmusohrstig commented 6 years ago

I have not yet tried using AndroidX in id-example while still using support in the library code.

That is basically what we are doing currently in our app.

soldierinwhite commented 5 years ago

@jorunfa I think I have identified the issue now, looking at the same issue elsewhere. (e.g. https://github.com/openid/AppAuth-Android/issues/395). Jetifier seems to be the culprit.

The CustomTabsHelper class in the sdk has a hardcoded string constant private static final String ACTION_CUSTOM_TABS_CONNECTION = "android.support.customtabs.action.CustomTabsService"

The package manager checks an intent with this action to find an app that can handle it.

The issue is that jetifier, which refactors 3rd party libraries in a project from support to androidx, does this even for hardcoded strings in the libraries. This means that the action string is refactored to "androidx.browser.customtabs.action.CustomTabsService", which is the corresponding library in androidx, but is not included in the Chrome manifest as an action. This means our client app always gets an empty list of apps to handle the custom tabs intents and reverts to opening the web view.

You can't replicate this when cloning the id-example app, since jetifier does not seem to behave the same with modules as aar libraries.

The other repos that have had this problem suggest using a String builder when declaring the action string constant instead of just a hardcoded string to protect it from Jetifier. Like this. I tested it with our app and it works.

jorunfa commented 5 years ago

Seems like a reasonable workaround. Thanks for doing the research! Question: Do we want to change this if we move from "normal" support lib -> androidx?

soldierinwhite commented 5 years ago

Yes, you would still need to change this. Since we would still enable jetifier in the client app to refactor other gradle libraries to androidx, the hardcoded string will still be changed. Of course, the workaround is unnecessary if Chrome and other browsers would update the action strings in their manifests, or jetifier would remove this bug/feature (it even refactors constant strings in the Android framework).

Until then, the workaround seems like the quickest fix and would continue to work even if these fixes were to happen in the near future.

jorunfa commented 5 years ago

Merged and released. Does it work now/does this close the issue?

simonnorberg commented 5 years ago

Yes, 1.9.2 custom tabs works on all my devices.