Conductor: startIntentSenderForResult is marked @TargetApi(Build.VERSION_CODES.N)

Created on 23 Nov 2017 · 19Comments · Source: bluelinelabs/Conductor

startIntentSenderForResult is marked @TargetApi(Build.VERSION_CODES.N) but it was added in API 16. This is a bug.
When I try to use startIntentSenderForResult on API 21 I get a crash:

  Caused by: java.lang.NoSuchMethodError: No virtual method startIntentSenderForResult(Landroid/content/IntentSender;ILandroid/content/Intent;IIILandroid/os/Bundle;)V in class Lcom/bluelinelabs/conductor/internal/LifecycleHandler; or its super classes (declaration of 'com.bluelinelabs.conductor.internal.LifecycleHandler' appears in /data/app/com.folx.driver.debug-2/base.apk:classes21.dex)
                                                                           at com.bluelinelabs.conductor.internal.LifecycleHandler.startIntentSenderForResult(LifecycleHandler.java:306)
                                                                           at com.bluelinelabs.conductor.ActivityHostedRouter.startIntentSenderForResult(ActivityHostedRouter.java:95)
                                                                           at com.bluelinelabs.conductor.Controller.startIntentSenderForResult(Controller.java:533)

Maybe @PaulWoitaschek will know as he's the one who added this method.

Source

mzgreen

👍1

All 19 comments

Some multidex or build error? Or maybe you have multiple versions of conductor?
If I understand correctly the error in the signature
startIntentSenderForResult(Landroid/content/IntentSender;ILandroid/content/Intent;IIILandroid/os/Bundle;)

was never there.

And no, I did not add this method.

PaulWoitaschek on 23 Nov 2017

Sorry I mentioned you because of this https://github.com/PaulWoitaschek/Conductor/commit/9ea8dcafb44a3f6d24c49b7ef7efaa479b01dd9f

I believe it can't find the method because it's marked as N and above. Using in activity instead of Controller works ok.

mzgreen on 23 Nov 2017

Seems that Activity has 2 overloads of this method. One available from API5 and second from API16. I guess I tried to use the one that doesn't exist in Conductor. Nonetheless that annotation for Nougat is wrong.

mzgreen on 23 Nov 2017

added in API level 16
void startIntentSenderForResult (IntentSender intent, 
                int requestCode, 
                Intent fillInIntent, 
                int flagsMask, 
                int flagsValues, 
                int extraFlags, 
                Bundle options)

and

added in API level 5
void startIntentSenderForResult (IntentSender intent, 
                int requestCode, 
                Intent fillInIntent, 
                int flagsMask, 
                int flagsValues, 
                int extraFlags)

Indeed it is API 5 and API 16

Zhuinden on 25 Nov 2017

Actually what you are posting @Zhuinden is part of the Activity. The framework fragment has that requirement.

Can someone review my PR?

PaulWoitaschek on 16 Jun 2018

The annotation for Android N is correct. It was added in SDK 24.

The only way I know of to add support for earlier versions would be to depend on support Fragments instead of native. While I'm sure pretty much everyone using Conductor already includes AppCompat in their projects, this isn't a dependency I'm comfortable adding unless it's absolutely necessary. Perhaps there's a clean way to have a version of Conductor that's based on support Fragments without copy and pasting a whole bunch of code?

EricKuck on 16 Jun 2018

Yeah wouldn't be a fan of depending on appcompat now with the androidx renaming on the horizon :D

Zhuinden on 17 Jun 2018

@EricKuck platform fragments are deprecated anyways so moving to AppCompat would be a good move in my opinion.

mzgreen on 17 Jun 2018

Platform fragments are marked as deprecated so that people who don't know what they're doing can know that "oh whoops I imported the wrong thing"

There's nothing wrong with using a retained platform fragment for surviving config changes, lifecycle events, and intercepting startActivity* calls.

Zhuinden on 17 Jun 2018

Platform fragments aren't going anywhere, as support fragments depend on them. Conductor depending on them is, and likely always will be, totally fine.

EricKuck on 17 Jun 2018

as support fragments depend on them.

They do? I don't think they do. They live in the AppCompat's support FragmentManager which lives as non-config instance.

Platform fragments aren't going anywhere,

This is true. They're just no longer developed. Which is good, because now they can't break (i'm looking at you, support lib 27.1.0 fragment fade animations)

Zhuinden on 17 Jun 2018

You're totally right. I was thinking of it depending on onRetainNonConfigurationInstance, which also caused lots of controversy when it was deprecated. Long day.

EricKuck on 17 Jun 2018

🎉1

They won't remove platform fragments. I'm just not sure why do you want to stick with them. App Compat Fragments are much more stable, they have all of the recent bug fixes and their behavior is the same across different platform levels.
By deprecating platform fragments Android team says that devs should not be using them.
You can read this discussion: https://twitter.com/ianhlake/status/971455064274485248
Or this reddit thread: https://www.reddit.com/r/androiddev/comments/7vn7ix/android_p_will_deprecate_native_fragments/dttwmdt/
If you haven't already.

mzgreen on 18 Jun 2018

@mzgreen because dependency on specific version of AppCompat as a library can be more costly (see AndroidX repackaging) than using a retained fragment internally which is proven to work

Zhuinden on 18 Jun 2018

I'm starting to wonder if we should add the ability to switch out which Fragment type Conductor is backed with. @dmarcato took a stab at this a while back that still holds up well here.

The one thing I'm hesitant about with this approach is the use of the AndroidManifest metadata to detect which backend should be used (which also forces a dependency on the conductor-support module).

An alternative to this approach would be to do a quick check at runtime to see if android.support.v4.app.Fragment (or androidx equivalent) is in the classpath. If it isn't, the platform fragment would continue to be used. If it is, it would switch to the support fragment.

Thoughts?

EricKuck on 18 Jun 2018

👍1

It can work, but it must be proguard-friendly (with rules if needed)

Zhuinden on 18 Jun 2018

The trick with classpath was used in Retrofit some time ago. If OkHttp was present then Retrofit was using it otherwise HttpUrlConnectiom was used. I like this approach because it doesn't force any additional work on developers.

mzgreen on 18 Jun 2018

An even easier method than classpath would be to see if the calling Activity extends FragmentActivity. Since Activities are always in proguard rules, I think this would be super reliable (someone correct me if I'm wrong!).

The other issue thing brings up is what to do with TargetApi annotations. If the backing fragment could change dynamically, we wouldn't have a way to declare minimum SDKs for things like startIntentSenderForResult. If the developer uses a FragmentActivity, the min would be 16. If not, it would be 24. This could cause lots of unhappy developers if the method is marked as @TargetApi(16), but crashes on sdk 16-23 due to not using a FragmentActivity.

EricKuck on 18 Jun 2018

👍1

I would even argue: does it make sense to keep supporting the platform's Fragment?
If I remember correctly, Google is going to deprecate them as part of Android P, and will only provide support/updates for the support library's one...
Maybe it would make more sense to just depend on the support library and switch over to the support version?