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

frontend/android: check if webview can go back or close the app #2701

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

thisconnect
Copy link
Collaborator

@thisconnect thisconnect commented May 6, 2024

This should replace native Android back behavior with either going
back in the webview history or closing the app.

In theory the react-router pushes history entries to the webview
navigation history, as soon as the user clicks a Link or navigate
is called. The navigate function can also be called with replace
option so that it does not add to the history.

This is using the native goBack method but it seems that the app
only goes back to account summary. In theory this should behave
the same as native browser back. It is not clear why it jumps to
the beginning.

So this could probably be improved, but seems already useful with
just going back to the account summary.

On the account summary it cannot go back any further so it will
trigger the old do-you-want-to-quit alert.

Replaced deprecated onBackPressed override.

@thisconnect thisconnect force-pushed the frontend-android-back branch 3 times, most recently from 20ca1f2 to 81f8401 Compare May 6, 2024 15:45
@thisconnect thisconnect marked this pull request as draft May 6, 2024 15:48
@thisconnect thisconnect force-pushed the frontend-android-back branch 2 times, most recently from 7702edf to 17f8920 Compare May 6, 2024 16:24
@thisconnect thisconnect marked this pull request as ready for review May 6, 2024 17:16
@thisconnect thisconnect force-pushed the frontend-android-back branch 4 times, most recently from 16a9db7 to 8d94eb2 Compare May 6, 2024 17:51
@thisconnect thisconnect changed the title Frontend android back frontend/android: check if webview can go back or close the app May 6, 2024
@benma
Copy link
Contributor

benma commented May 7, 2024

Would be good to describe the behavior more clearly. When/where does a navigation add to the navigation history? A way to unit test this stuff would be very nice, to document the navigation behavior clearly.

I also wonder how robust this is - can a change like #2524 change this behavior? Again some sort of automated tests would be very very good to have for this.

@thisconnect
Copy link
Collaborator Author

A way to unit test this stuff would be very nice, to document the navigation behavior clearly.

To be explicit, you want to test vw.canGoBack() and vw.goBack() and or the whole thing around it?

I guess that would be very nice. I have no idea what that takes, would it test with a real webview with our production bundled.js and simulating user clicking in react UI and then simulating webview.goBack() and testing if the URI is correct?

Sounds to me more like integration / e2e test, rather than unit test? As far as I see we don't have any java test or maybe I am blind.

Also I am afraid that I have no idea how to setup something like this.

I also wonder how robust this is - can a change like #2524 change this behavior? Again some sort of automated tests would be very very good to have for this.

I don't see any reason why it shouldn't work, but I will test this branch on top of #2524 . I believe worst case vw.canGoBack() does not return true and then it calls the old code (alert + quitting the app).

When/where does a navigation add to the navigation history?

This is a good question. There is history.pushState https://developer.mozilla.org/en-US/docs/Web/API/History/pushState to add entries to the browser history. And it looks like our router somehow adds history entries, but I am not sure why it doesn't correctly go back when calling vw.goBack();.
Afaik react-router does window.history.pushState, but I will try to check.

@thisconnect
Copy link
Collaborator Author

rebased

// See the following for details about task and activity stacks:
// https://developer.android.com/guide/components/activities/tasks-and-back-stack
@Override
public void onBackPressed() {
Copy link
Collaborator Author

@thisconnect thisconnect May 8, 2024

Choose a reason for hiding this comment

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

This method was deprecated in API level 33.

API level 33.
Use OnBackInvokedCallback or androidx.activity.OnBackPressedCallback to handle back navigation instead.

https://developer.android.com/reference/android/app/Activity#onBackPressed()

@thisconnect
Copy link
Collaborator Author

can a change like #2524 change this behavior?

here is a POC with this commit on top of #2524 it seems to behave the same.
#2705

@benma
Copy link
Contributor

benma commented May 14, 2024

A way to unit test this stuff would be very nice, to document the navigation behavior clearly.

To be explicit, you want to test vw.canGoBack() and vw.goBack() and or the whole thing around it?

I guess that would be very nice. I have no idea what that takes, would it test with a real webview with our production bundled.js and simulating user clicking in react UI and then simulating webview.goBack() and testing if the URI is correct?

Sounds to me more like integration / e2e test, rather than unit test? As far as I see we don't have any java test or maybe I am blind.

Also I am afraid that I have no idea how to setup something like this.

I also wonder how robust this is - can a change like #2524 change this behavior? Again some sort of automated tests would be very very good to have for this.

I don't see any reason why it shouldn't work, but I will test this branch on top of #2524 . I believe worst case vw.canGoBack() does not return true and then it calls the old code (alert + quitting the app).

When/where does a navigation add to the navigation history?

This is a good question. There is history.pushState https://developer.mozilla.org/en-US/docs/Web/API/History/pushState to add entries to the browser history. And it looks like our router somehow adds history entries, but I am not sure why it doesn't correctly go back when calling vw.goBack();. Afaik react-router does window.history.pushState, but I will try to check.

The tests don't have to be inside Android/Java, you can assume it works like a Chromium browser.

It would just be very good if we had a good list of tests (manual tests in the worst case, automated would be best) that show that history-back goes to the expected place. Worse than a back button that closes the app is one that does not work as expected 😄

@thisconnect
Copy link
Collaborator Author

It would just be very good if we had a good list of tests (manual tests in the worst case, automated would be best) that show that history-back goes to the expected place. Worse than a back button that closes the app is one that does not work as expected 😄

unfortunately it already does not work as expected. I expect that it does history back, but it always just goes back to account summary. I am not sure why, and am not sure how to remote debug right-now.

In webdev history back works as expected.. It would be very nice if both behave the same, need to investigate what exactly is happending in the webview when we call goBack() (or the js version vw.evaluateJavascript("history.back();", null);)

This should replace native Android back behavior with either going
back in the webview history or closing the app.

In theory the react-router pushes history entries to the webview
navigation history, as soon as the user clicks a Link or navigate
is called. The navigate function can also be called with replace
option so that it does not add to the history.

This is using the native goBack method but it seems that the app
only goes back to account summary. In theory this should behave
the same as native browser back. It is not clear why it jumps to
the beginning.

So this could probably be improved, but seems already useful with
just going back to the account summary.

On the account summary it cannot go back any further so it will
trigger the old do-you-want-to-quit alert.

Replaced deprecated onBackPressed override.
The previous commit added a condition webview.canGoBack() and then
uses goBack() if the webview can go back in history.

This does not behave the same as native browser back in webdev.

The webview loads with loadDataWithBaseURL, where the 4th argument
is historyUrl which is null. Introduced in: 3b62772

This commit uses same base url to initialize historyUrl.
@thisconnect
Copy link
Collaborator Author

testing if initializing with correct historyUrl changes the goBack behavior.

@thisconnect thisconnect force-pushed the frontend-android-back branch 2 times, most recently from fa7639e to c3bd59e Compare May 22, 2024 15:00
@thisconnect thisconnect marked this pull request as draft May 23, 2024 08:05
@thisconnect thisconnect force-pushed the frontend-android-back branch 2 times, most recently from 520aea4 to 1a70cce Compare May 27, 2024 07:16
Alert component exposes a function to be called from anywhere.

This is a bit a hack and if called multiple times, last alerts
are lost.

Changed to check if alert is already active and append new messages
to the same alert.
seems sd there can be similar issues when using history back from
a webview that was loaded by loadDataWithBaseURL.

This is wild guess

https://stackoverflow.com/questions/3574674/android-webviews-method-goback-shows-a-blank-page
test to check if loadDataWithBaseURL with shiftcrypto.ch origin might
cause history to go back to "https://shiftcrypto.ch/"
introduced in 3b62772
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.

None yet

2 participants