Skip to content

Support intent: and android-app: url schemes in templates#4229

Merged
timrae merged 2 commits intoankidroid:developfrom
marcardar:support-intent-url-schemes
Apr 25, 2016
Merged

Support intent: and android-app: url schemes in templates#4229
timrae merged 2 commits intoankidroid:developfrom
marcardar:support-intent-url-schemes

Conversation

@marcardar
Copy link

Following on from this discussion.

@marcardar marcardar force-pushed the support-intent-url-schemes branch 3 times, most recently from 1b3ea15 to 66c1fa0 Compare April 24, 2016 05:07
@marcardar
Copy link
Author

Tested against pre-22 (as well as 22+). It is safe to use Intent.parseUri(url, Intent.URI_ANDROID_APP_SCHEME) pre-22.

@timrae
Copy link
Member

timrae commented Apr 24, 2016

Is there any documentation on that android-app scheme? It'd be nice if there was a cleaner way which was better documented. The docs seem to just recommend using a custom scheme.

@marcardar
Copy link
Author

marcardar commented Apr 24, 2016

The only documentation is here unfortunately.

Definitely the Android team pushes devs away from custom schemes. It looks like Chrome supports custom schemes (less and less) rather than encourages them. That link you included shows the dev how to use an intent: scheme instead of a custom one.

This PR is working really well for me and tested on various API levels. Once AnkiDroid supports intent: and android-app: schemes, I can remove those intent filters from my manifest.

@timrae
Copy link
Member

timrae commented Apr 24, 2016

That link you included shows the dev how to use an intent: scheme instead of a custom one.

Ooops, sorry I was skim reading. Have you tried using the fallback URL mechanism mentioned in the docs I linked you to?

Also, you may choose to specify fallback URL by adding the following string extra:

S.browser_fallback_url=[encoded_full_url]

If you give it the URL to your google play listing, it should automatically get redirected to the play app on almost all versions of Android I think.

@marcardar
Copy link
Author

If you give it the URL to your google play listing, it should automatically get redirected to the play app on almost all versions of Android I think.

That works if I click on the link within Google Chrome, but not if the link is in the template. No idea why that should be the case.

When AnkiDroid finds that the resolved intent is not available, it should probably call the fallback URL (if one is set), otherwise use the market:// How does that sound?

@marcardar
Copy link
Author

Looks like it's already discussed here.

@marcardar marcardar force-pushed the support-intent-url-schemes branch from 66c1fa0 to 4da8016 Compare April 24, 2016 08:15
@marcardar
Copy link
Author

I've just added support for browser_fallback_url. Please check.

@timrae
Copy link
Member

timrae commented Apr 24, 2016

Did you see the comment about adding BROWSABLE? As I'm sure you know, since Kitkat the Webview is using Chromium so it's supposed to behave the same as Chrome.

@timrae
Copy link
Member

timrae commented Apr 24, 2016

I've just added support for browser_fallback_url. Please check.

My angle was more to let the Android system deal with the intent in a standard way rather than adding lots of rules to AnkiDroid.

@marcardar
Copy link
Author

marcardar commented Apr 24, 2016

My angle was more to let the Android system deal with the intent in a standard way rather than adding lots of rules to AnkiDroid.

Yeah I agree AnkiDroid shouldn't be doing this unless it has to (as a workaround). Nice spot regarding the BROWSABLE category. I'll try it out now. Although IMO, it seems a little silly to insist that the intent uri MUST include that category (why else would a template include the intent uri?).

Also worth noting that my intent-filter always had both the DEFAULT and BROWSABLE categories included.

@marcardar
Copy link
Author

marcardar commented Apr 24, 2016

BTW, I also tried disabling the custom WebViewClient (by not setting it), to see if the webview would handle the intent uri differently. However, that also does not work. EDIT: this is true only regarding the fallback url.

@marcardar
Copy link
Author

Oh, actually if you don't set the WebViewClient then the intent: uri is automatically resolved and used. However, it looks like the fallback url is not resolved (Web page note available). I tried this using the zxing examples given in that Chrome link. Note that those intent uris do not include the BROWSABLE category.

So regardless, it looks like we need to resolve these links manually.

@marcardar
Copy link
Author

marcardar commented Apr 24, 2016

I tried adding the BROWSABLE category to the intent uri and that doesn't seem to make any difference compared to not adding it. This is true for when setting the WebViewClient and also when not setting it.

Also worth noting that my intent-filter always had both the DEFAULT and BROWSABLE categories included.

@marcardar marcardar force-pushed the support-intent-url-schemes branch 4 times, most recently from fa9b606 to 375a6ae Compare April 24, 2016 09:49
@marcardar
Copy link
Author

Made small change - remove fallback url from resolved intent (as mentioned here: https://developer.chrome.com/multidevice/android/intents)

@marcardar marcardar force-pushed the support-intent-url-schemes branch from 375a6ae to 427b542 Compare April 25, 2016 09:28
@marcardar marcardar force-pushed the support-intent-url-schemes branch from 427b542 to e3de2d1 Compare April 25, 2016 09:32
}
} else {
// https://developer.chrome.com/multidevice/android/intents says that we should remove this
intent.removeExtra(EXTRA_BROWSER_FALLBACK_URL);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we add the Intent.CATEGORY_BROWSABLE here?

@timrae
Copy link
Member

timrae commented Apr 25, 2016

Can you please remove all the fallback URL stuff? I only suggested adding it as a way to remove the explicit market redirect; if that doesn't work then I prefer to ignore that attribute entirely.

if (url.startsWith("intent:")) {
intent = Intent.parseUri(url, Intent.URI_INTENT_SCHEME);
} else if (url.startsWith("android-app:")) {
intent = Intent.parseUri(url, Intent.URI_ANDROID_APP_SCHEME); // note - also works pre-22
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why would this work pre-22? Even if it somehow did work in your tests, the doc says added in 22 so I don't want to use it in production code.

Copy link
Author

@marcardar marcardar Apr 25, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Presumably the constant is inlined at compile time and then parseUri is able to deal with the flag, at least to some extent. Shall I just add a Build.VERSION.SDK_INT if statement?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Weird... Yeah maybe just squeeze it in with url.startsWith("android-app:")

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should still parse it, otherwise templates will behave differently on different devices. You can see from the format there is very little difference between intent: and android-app: uris...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tbh I don't see much point in including support for android-app:

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. I think we should just pass those URIs through.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency, it makes sense to treat intent: and android-app: the same. Just like Chrome does.

@marcardar
Copy link
Author

I've removed the fallback url stuff and improved the android-app: support (including testing on API16).

}
} else {
// https://developer.chrome.com/multidevice/android/intents says that we should remove this
intent.addCategory(Intent.CATEGORY_BROWSABLE);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts about this?

@timrae timrae merged commit 18a51db into ankidroid:develop Apr 25, 2016
@marcardar
Copy link
Author

Thanks!

@marcardar marcardar deleted the support-intent-url-schemes branch April 26, 2016 07:21
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.

3 participants