Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update the flag when querying the Activities which can handle VIEW intent #452

Closed
wants to merge 5 commits into from

Conversation

rayworks
Copy link

@rayworks rayworks commented Nov 9, 2023

Update the query flag to cover more installed browser apps (besides Chrome) which may support custom tabs.

@PEConn
Copy link
Contributor

PEConn commented Nov 9, 2023

Sorry, do you mind explaining a bit more what the change does?

@rayworks
Copy link
Author

Hi @PEConn , Because only types of apps that are visible automatically since Android 11, by using the flag PackageManager.MATCH_ALL, we can get all candidates which are capable of handling intent with action Intent.ACTION_VIEW. After further check for the CustomTabsService, we won't miss other target browsers(e.g. org.mozilla.firefox) even if Chrome is not installed on the device.

@PEConn
Copy link
Contributor

PEConn commented Nov 10, 2023

The manifest for that demo app has:

    <queries>
        <intent>
            <action android:name="android.support.customtabs.action.CustomTabsService" />
        </intent>
    </queries>

So all browsers that support Custom Tabs should be visible.

@rayworks
Copy link
Author

rayworks commented Nov 10, 2023

Hi @PEConn ,
On my side, adding <action android:name="android.support.customtabs.action.CustomTabsService" /> is not enough.
As I can tell from the testing result on OnePlus 7Pro (Android 12), the query result list only contains browser Chrome.

     // Get all apps that can handle VIEW intents.
        List<ResolveInfo> resolvedActivityList = pm.queryIntentActivities(activityIntent, 0);
        List<String> packagesSupportingCustomTabs = new ArrayList<>();
        for (ResolveInfo info : resolvedActivityList) {
            Intent serviceIntent = new Intent();
            serviceIntent.setAction(ACTION_CUSTOM_TABS_CONNECTION);
            serviceIntent.setPackage(info.activityInfo.packageName);
            if (pm.resolveService(serviceIntent, 0) != null) {
                packagesSupportingCustomTabs.add(info.activityInfo.packageName);
            }
        }

If you look into the code, the first step is to query all the Activities which can handle the intent.
After that, it filters out the the packages by resolving the specific Service with action android.support.customtabs.action.CustomTabsService.
So the issue may occur on the first step where we call the method queryIntentActivities(activityIntent, 0).

@rayworks
Copy link
Author

Query results changed after I updated the flag from 0 to PackageManager.MATCH_ALL at this line of code.

BEFORE:
Screenshot 2023-11-13 at 11 27 52

AFTER:
Screenshot 2023-11-13 at 11 30 01

@PEConn
Copy link
Contributor

PEConn commented Nov 13, 2023

That's really weird - ultimately though I'm not against making this change, as it is in demo app code. Do you mind adding a comment saying why you're doing this and that in practice (name the device and Android version you're using) you've found the behaviour to be different from what's documented.

@rayworks
Copy link
Author

rayworks commented Nov 13, 2023

It turns out the method queryIntentActivities(intent, flag) has been deprecated on Android 13+, the new one is queryIntentActivities (Intent intent, PackageManager.ResolveInfoFlags flags).
As you can see from the values for ResolveInfoFlags, flag 0 is no longer the option any more, so I'd rather use the MATCH_ALL in the code.

@PEConn
Copy link
Contributor

PEConn commented Nov 14, 2023

As you can see from the values for ResolveInfoFlags, flag 0 is no longer the option any more, so I'd rather use the MATCH_ALL in the code.

Maybe I'm missing something, but the doc you linked says "long: Value is either 0 or a combination of ...".

Looked at your code, I'm fine with bumping the compileSdkVersion as long as you've run the tests. I'd still like you to add a the comment I mentioned here.

@rayworks
Copy link
Author

@PEConn
Please have another round of review.

@PEConn
Copy link
Contributor

PEConn commented Nov 17, 2023

Hey,

Thanks for the update. Sorry for being so picky, but could you please either:

  • Undo the compileSdkVersion change (keeping it at 31).
  • Or, update other usages of queryIntentActivities (such as in ManageDataLauncherActivity and TwaProviderPicker).

As it is, it's a little inconsistent that we've updated the code in one place we use queryIntentActivities, but not in others.

I'd prefer it if you took the first option and landed this PR with just a change to CustomTabsHelper, and then (if you're still interested) upload a separate PR that updates the targetSdkVersion. In general, small PRs that do single logical things are preferred.

Thanks,
Peter

@rayworks rayworks closed this Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants