search

GoogleChromeLabs / svgomg-twa

A sample that project Trusted Web Activities technology to wrap SVGOMG in an Android Application
Apache License 2.0
519 stars 132 forks source link

Replace useBrowserOnChromeOS with cros_web_alternative. #68

Closed PEConn closed 4 years ago

andreban commented 4 years ago

So, if the developer adds a meta tag with the cros_web_alternative key, but an empty value, it means that Chrome OS should use the app. If that value has content, it will open the link in the URL?

I wonder if it would make more sense to simply not have the cros_web_alternative meta tag when the developer wants to open the app.

Not sure how to solve this in SVGOMG, but would be feasible in llama-pack.

PEConn commented 4 years ago

I think cros_web_alternative will have other restrictions on it as well, for example a working Digital Asset Link (and of course that it is a valid URL). So the difference isn't between cros_web_alternative present or not, it's between it being valid or not.

andreban commented 4 years ago

I'm looking at this from a developer intent perspective. Adding the meta-tag will mean their intended behaviour is opening the provided link on Chrome OS. Not adding the meta-tag means their intent is opening the Android App on Chrome OS.

If we want it even more clear, we could keep both meta-tags, and expressing the URL to be opened is required if opening in Chrome OS is set to true.

So, we get a clear difference between the behaviour the developer intended and an implementation error, as providing an invalid URL or failing Digital Asset Links would be.

PEConn commented 4 years ago

I would prefer to not have two tags, just the one.

Admittedly the only reason I had cros_web_alternative being empty as an option was because I couldn't figure out how to conditionally include a meta-data element. Any ideas?

andreban commented 4 years ago

So, from an svgomg-twa perspective, we might be ok with with just leaving the meta-tag there and document that it should be removed from the AndroidManifest if the developers want to open the app on Chrome OS. Disabling this only makes sense when developers have authored their own code anyway, meaning they are probably more comfortable with Android Studio and AndroidManifest.xml.

Having said that, I wonder if manifest-merge could be a way to do this using build.gradle.

PEConn commented 4 years ago

There we go Andre - got rid of useBrowserOnChromeOS, that should do for now. If people want an easier way to turn this off we can look into manifest-merge later.

As a further point, I was wondering if we should offer users the ability to specify a crosLaunchUrl. That's not included in this CL but should be easy to add in the future.