Open browser site settings when the user presses "Manage Data"

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

Open browser site settings when the user presses "Manage Data" #61

Closed FluorescentHallucinogen closed 4 years ago

andreban commented 4 years ago

Interesting - Trying to clear data from an application that hasn't been properly verified by Digital Asset Links causes the app to crash:

2019-11-07 13:38:10.455 7653-7653/org.chromium.twa.svgomg E/AndroidRuntime: FATAL EXCEPTION: main
    Process: org.chromium.twa.svgomg, PID: 7653
    java.lang.RuntimeException: Failed to validate origin https://svgomg.firebaseapp.com
        at android.support.customtabs.trusted.ManageDataLauncherActivity$1$1.onRelationshipValidationResult(ManageDataLauncherActivity.java:136)
        at android.support.customtabs.CustomTabsClient$2$5.run(CustomTabsClient.java:323)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at android.os.Looper.loop(Looper.java:214)
        at android.app.ActivityThread.main(ActivityThread.java:7356)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:492)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:930)

It does work as intended for apps that pass the Digital Asset Links validation.

PEConn commented 4 years ago

Yeah, this is behaviour from the library (raising an error if verification fails). You can override this by creating a subclass of ManageDataLauncherActivity and overriding onError(RuntimeException e) to no-op.

I thought it would be weird to just drop the error if the origin wasn't verified because then the user has pressed the "Manage Data" button and then nothing happens. It could display a toast I guess, but that's still not amazing UX.

FluorescentHallucinogen commented 4 years ago

If the user just installs the TWA and have never run it (even if previously have opened the corresponding PWA in the browser) and press the "Manage Data", visually nothing happens (I have not checked logs).
If the user sets a different browser than Chrome or Chromium (e.g. Samsung Internet or Microsoft Edge) as the default browser in Android settings and press the "Manage Data", the TWA crashes.

FluorescentHallucinogen commented 4 years ago

@andreban @PEConn In order to merge this pull request, it required first to make any other changes to this project and/or custom-tabs-client?

PEConn commented 4 years ago

I'd like to address the two error cases you mentioned two comments above - and this would involve modifications to android-browser-helper (the new version of custom-tabs-client). We should create a bug in that repo to track them.

I don't want to land this change as is, because yeah, we'd crash in those two instances. If you wanted to get this code merged yourself, could you create a subclass of the ManageDataLauncherActivity that overrides onError to provide some user feedback (maybe just a Toast saying "Unable to manage data."). I think that would be acceptable until the deeper issues are fixed.

FluorescentHallucinogen commented 4 years ago

@NotWoods Since Firefox Preview on Android now supports TWA, it would be really nice to open Firefox activity for site settings of corresponding URL when the user presses "Manage Data" in TWA app settings. Is it possible? If yes, what the name of this activity?

NotWoods commented 4 years ago

We use the navigation graph component so we could expose a deep link or intent filter. I don't know if we have that right now off the top of my head.

FluorescentHallucinogen commented 4 years ago

@torgo Since Samsung Internet is based on Chromium and TWA support coming to Samsung Internet (BTW, any ETA?), it would also be really nice to open Samsing Internet activity for site settings of corresponding URL when the user presses "Manage Data" in TWA app settings. Is it possible? If yes, what the name of this activity?

FluorescentHallucinogen commented 4 years ago

Having this info from @NotWoods and @torgo, I believe it's possible to make changes into custom-tabs-client (android-browser-helper) to add support of Firefox and Samsung Internet instead of just showing "Unable to manage data" / "This browser is not supported" toast.

Generally speaking, it would be nice to have a single standard mechanism to open site settings (with permissions) of corresponding URL for TWA for all browsers that support Trusted Web Activity to avoid bloating (continuous updating) of custom-tabs-client (android-browser-helper) to support more and more browsers.

@PEConn WDYT?

PEConn commented 4 years ago

I agree that it would be good to have a standard mechanism - and once we (Chrome, Firefox and Samsung) have agreed on it we can move the code from android-browser-helper to androidx.browser.

We won't be able to get rid of having to deal with non-supporting browsers though so there'll still be that complexity.

FluorescentHallucinogen commented 4 years ago

Any progress?

FluorescentHallucinogen commented 4 years ago

I see #76 was merged.

FluorescentHallucinogen commented 4 years ago

@PEConn @andreban

I'd like to address the two error cases you mentioned two comments above - and this would involve modifications to android-browser-helper (the new version of custom-tabs-client). We should create a bug in that repo to track them.

Is https://github.com/GoogleChrome/android-browser-helper/issues/12 this issue or is it another separate issue?

FluorescentHallucinogen commented 4 years ago

@PEConn @andreban Please reply.

FluorescentHallucinogen commented 4 years ago

Closed in favor of https://github.com/GoogleChromeLabs/bubblewrap/pull/279.