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

Added PAGE_LINK_PATHPREFIX preference for specifying Android pathPrefix #82

Merged
merged 6 commits into from
Oct 21, 2020

Conversation

mcfarljw
Copy link
Contributor

@mcfarljw mcfarljw commented Oct 4, 2020

We have multiple apps using the same domain and need to be able to specify the pathPrefix so that the correct app is opened. This is shown in the Android docs here: https://developer.android.com/training/app-links/deep-linking

Variables when adding the plugin look something like this:

--variable PAGE_LINK_DOMAIN="app.example.com" --variable PAGE_LINK_PATHPREFIX="link/app1"
--variable PAGE_LINK_DOMAIN="app.example.com" --variable PAGE_LINK_PATHPREFIX="link/app2"

The default for the PAGE_LINK_PATHPREFIX preference is set to "" which means android:pathPrefix="/" should still be good for those not using it.

This isn't an issue for iOS as app paths are set in the apple-app-site-association file which is hosted on the server.

@chemerisuk
Copy link
Owner

@mcfarljw hm, my expectation for variable PAGE_LINK_DOMAIN is be used for *.page.link domains owned by google. Where you can't use the same domain for several apps. Can you explain what type of domain specified in your case?

@mcfarljw
Copy link
Contributor Author

mcfarljw commented Oct 5, 2020

I made a typo in my example above. It should have been app.example.com being referenced for both PAGE_LINK_DOMAIN examples which I have edited for clarity.

I think you can actually can use the same *.page.link for several apps, but in our case we're using a single custom domain. The pathPrefix kind of serves like a filter if using multiple apps on the same domain. In our specific case we have a language company (https://skritter.com) that has an app for learning Chinese and Japanese that are separate apps. They both share the same Firebase project resources as they use the same datastore and user accounts. We have dynamic links configured like this:

If we don't specify the pathPrefix then trying to open https://app.skritter.com/link/zh with both apps installed will open the Japanese app since it's first alphabetically and it doesn't know which app to open. If you look at the the assetlinks.json file for domain verification Google doesn't let you specify which path goes to which app so you have to do it using the pathPrefix. Here is our Google file for reference: https://app.skritter.com/.well-known/assetlinks.json

The reason Apple makes this a bit easier is because you specify the paths in their verification file so don't need to do anything in the actual app code. Here is our Apple file for reference: https://app.skritter.com/.well-known/apple-app-site-association

@chemerisuk
Copy link
Owner

@mcfarljw thanks for the clarification. I was thinking a little about all this information and suggest above:

  1. Change variable name PAGE_LINK_DOMAIN to APP_DOMAIN_NAME.
  2. Add a note into README that APP_DOMAIN_NAME value can be *.page.link owned by Google or any other custom domain.
  3. PAGE_LINK_PATHPREFIX rename to APP_DOMAIN_PATH with default value "/". I'd like to avoid empty string for a better clarity of what the value means.
  4. Add a note into README about APP_DOMAIN_PATH, when it should be used.

What do you think?

@mcfarljw
Copy link
Contributor Author

mcfarljw commented Oct 7, 2020

I think renaming things from PAGE_LINK_* TO APP_DOMAIN_* would be better as it's more universal and more clear for those using custom domains. I'm also ok with making the APP_DOMAIN_PATH having whichever default value you feel is best as long as in the README it mentioned the required formatting of the value if used.

Something else of note (somewhat related to these custom domain changes) when using custom domains is in my config.xml I had to add the following for things to work on iOS:

<config-file target="*-Info.plist" parent="FirebaseDynamicLinksCustomDomains">
  <array>
    <string>https://app.skritter.com/link</string>
  </array>
</config-file>

This might be also mentioned in #28 and there could be two ways of handling it. The first would be just mentioning that this needs to be added to the your projects config.xml if using custom domains. The second would be adding support directly for it in plugin.xml like so:

<config-file target="*-Info.plist" parent="FirebaseDynamicLinksCustomDomains">
  <array>
    <string>https://$APP_DOMAIN_NAME$APP_DOMAIN_PATH</string>
  </array>
</config-file>

It's mentioned in the docs here https://firebase.google.com/docs/dynamic-links/custom-domains#set_up_a_custom_domain_in_the. I'm guessing adding this to the plugin would also work just fine with the page link stuff so might be best to just include in the plugin for simplicity?

@chemerisuk
Copy link
Owner

@mcfarljw I’d like to include the snippet with FirebaseDynamicLinksCustomDomains. It’s always better to make things just work, not everybody reads README.

  1. please add http: protocol urls as well as for Android
  2. do you have chance to verify if FirebaseDynamicLinksCustomDomains does not produce issues for *.page.link domains?

@chemerisuk
Copy link
Owner

Decided to push a new version without this PR, because renaming variables could introduce breaking changes.

@mcfarljw
Copy link
Contributor Author

mcfarljw commented Oct 8, 2020

Fair enough, I won't be able to test the page link stuff with FirebaseDynamicLinksCustomDomains until later today or tomorrow as things are a bit hectic at work at the moment.

@mcfarljw
Copy link
Contributor Author

I was able to test this out and using a plain APP_DOMAIN_NAME with the path prefix still works even when declaring the FirebaseDynamicLinksCustomDomains property. I've added those changes to this PR for further review. The two options when installing now are:

cordova plugin add cordova-plugin-firebase-dynamiclinks --variable APP_DOMAIN_NAME="mydomain.page.link"

cordova plugin add cordova-plugin-firebase-dynamiclinks --variable APP_DOMAIN_NAME="mydomain.com" --variable APP_DOMAIN_PATH="/app1"

@chemerisuk
Copy link
Owner

chemerisuk commented Oct 13, 2020

@mcfarljw looks good for me. I've noticed one more thing: preference DOMAIN_URI_PREFIX in plugin.xml used to set default value for dynamic links builder and has description below:

Sets the domain uri prefix (of the form "//xyz.app.goo.gl", "//custom.com/xyz") to use for this Dynamic Link.

I guess we should add $APP_DOMAIN_PATH there as well. And also we need to remove protocol prefix because now both http and https are supported. Unfortunately I don't have a setup with multiple Firebase projects targeted on a single domain. Can you test this use case? Just call createDynamicLink and make sure it triggers only the right app.

@mcfarljw
Copy link
Contributor Author

I appended the DOMAIN_URI_PREFIX to the APP_DOMAIN_PATH and it generates links as expected with and without the APP_DOMAIN_PATH domain being passed. The generated links open the expected apps as well. So given the following using a path of /app1:

cordova.plugins.firebase.dynamiclinks.createDynamicLink({
    link: "https://google.com"
}).then(function(url) {
    console.log("Dynamic link was created:", url);
});

It would log https://mydomain.com/app1?link=https%3A%2F%2Fgoogle.com as the link output.

@chemerisuk
Copy link
Owner

chemerisuk commented Oct 20, 2020

@mcfarljw great! Have you checked if we really need a protocol for DOMAIN_URI_PREFIX? Eg

<preference name="DOMAIN_URI_PREFIX" value="//$APP_DOMAIN_NAME$APP_DOMAIN_PATH"/>

@mcfarljw
Copy link
Contributor Author

Removing the protocol appears to break things, at least on Android. The returned link is formatted like //mydomain.com/app1 which won't go anywhere if used directly. I think minimal effort should be made to support HTTP beyond maybe declaring them in the intent filter if a choice has to be made between the two. Apple has been restrictive of using that protocol requiring apps to adhere to stricter link protocols:

https://developer.apple.com/documentation/security/preventing_insecure_network_connections

I think it might be best to just make the plugin only officially support HTTPS as the *.page.link domains already have that and it could just be made clear that custom domains need to be secure as well.

@chemerisuk chemerisuk merged commit fd194bc into chemerisuk:master Oct 21, 2020
@chemerisuk
Copy link
Owner

@mcfarljw thanks for your work! Changes released in v4.8.0

@mcfarljw mcfarljw deleted the path-prefix branch October 21, 2020 13:04
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