-
-
Notifications
You must be signed in to change notification settings - Fork 956
Refactor WebViewActivity path handling #6447
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
base: main
Are you sure you want to change the base?
Changes from all commits
65cbdce
065eab5
6bbaf17
46a54a3
f8b1965
ce033b4
0bcc194
eecbd93
edb1c27
6df7cde
e6f229a
4749190
f5a7621
da0d124
b764c84
9b1b064
dee94e5
7529564
369bb28
e30d005
1f2e7ed
3e47e4c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| package io.homeassistant.companion.android.util.compose.webview | ||
|
github-advanced-security[bot] marked this conversation as resolved.
Fixed
|
||
|
|
||
| import android.net.Uri | ||
| import android.webkit.WebView | ||
| import androidx.core.net.toUri | ||
| import io.homeassistant.companion.android.util.hasNonRootPath | ||
| import io.homeassistant.companion.android.util.hasSameOrigin | ||
|
|
||
| /** | ||
| * Determines the appropriate back action based on the [WebView]'s back/forward list | ||
| * and the current loaded URL. | ||
| * | ||
| * The resolution logic: | ||
| * 1. If the previous back-stack entry is a same-origin HTTP URL, returns | ||
| * [BackAction.GoBack] so the user can navigate back normally. | ||
| * 2. If there is a previous back-stack entry that is not same-origin HTTP and | ||
| * the current URL has a non-root path, returns [BackAction.NavigateToRoot] | ||
| * so the user is taken to the home page first. | ||
| * 3. Otherwise returns [BackAction.None] — the caller decides what to do | ||
| * (e.g. exit the activity, pop the navigation stack, or let the system | ||
| * handle back to show the predictive-back animation). | ||
| * | ||
| * @param webView the WebView whose back/forward list is inspected | ||
| * @param loadedUrl the current URL shown in the WebView (as tracked by the caller, | ||
| * not necessarily [WebView.getUrl] which can be `about:blank` during loads) | ||
| */ | ||
| fun resolveBackAction(webView: WebView, loadedUrl: Uri?): BackAction { | ||
| val previousUrl = if (webView.canGoBack()) { | ||
| val backForwardList = webView.copyBackForwardList() | ||
| val previousIndex = backForwardList.currentIndex - 1 | ||
| if (previousIndex >= 0) { | ||
| backForwardList.getItemAtIndex(previousIndex).url.toUri() | ||
| } else { | ||
| null | ||
| } | ||
| } else { | ||
| null | ||
| } | ||
| return resolveBackAction(previousUrl, loadedUrl) | ||
| } | ||
|
|
||
| private fun resolveBackAction(previousUrl: Uri?, loadedUrl: Uri?): BackAction { | ||
| if (previousUrl != null && | ||
| loadedUrl != null && | ||
| previousUrl.scheme?.startsWith("http") == true && | ||
| previousUrl.hasSameOrigin(loadedUrl) | ||
| ) { | ||
| return BackAction.GoBack | ||
| } | ||
|
|
||
| if (previousUrl != null && loadedUrl != null && loadedUrl.hasNonRootPath()) { | ||
| val rootUrl = loadedUrl.buildUpon() | ||
| .path("/") | ||
| .clearQuery() | ||
| .appendQueryParameter("external_auth", "1") | ||
| .fragment(null) | ||
| .build() | ||
| return BackAction.NavigateToRoot(rootUrl) | ||
| } | ||
|
|
||
| return BackAction.None | ||
| } | ||
|
|
||
| /** | ||
| * Represents the action to take when the user presses back in a WebView. | ||
| */ | ||
| sealed interface BackAction { | ||
| /** Navigate back in the WebView history. */ | ||
| data object GoBack : BackAction | ||
|
|
||
| /** Clear history and navigate to the root URL of the current server. */ | ||
| data class NavigateToRoot(val rootUrl: Uri) : BackAction | ||
|
|
||
| /** No more back navigation possible — the caller decides what to do. */ | ||
| data object None : BackAction | ||
| } | ||
|
Gifford47 marked this conversation as resolved.
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -138,10 +138,13 @@ import io.homeassistant.companion.android.util.OnSwipeListener | |
| import io.homeassistant.companion.android.util.TLSWebViewClient | ||
| import io.homeassistant.companion.android.util.applyInsets | ||
| import io.homeassistant.companion.android.util.compose.webview.BLANK_URL | ||
| import io.homeassistant.companion.android.util.compose.webview.BackAction | ||
| import io.homeassistant.companion.android.util.compose.webview.resolveBackAction | ||
| import io.homeassistant.companion.android.util.hasNonRootPath | ||
| import io.homeassistant.companion.android.util.hasSameOrigin | ||
| import io.homeassistant.companion.android.util.isStarted | ||
| import io.homeassistant.companion.android.util.sensitive | ||
| import io.homeassistant.companion.android.util.toRelativeUrl | ||
| import io.homeassistant.companion.android.websocket.WebsocketManager | ||
| import io.homeassistant.companion.android.webview.WebView.ErrorType | ||
| import io.homeassistant.companion.android.webview.addto.EntityAddToHandler | ||
|
|
@@ -455,7 +458,25 @@ class WebViewActivity : | |
|
|
||
| val onBackPressed = object : OnBackPressedCallback(webView.canGoBack()) { | ||
| override fun handleOnBackPressed() { | ||
| if (webView.canGoBack()) webView.goBack() | ||
| when (val action = resolveBackAction(webView, loadedUrl)) { | ||
| BackAction.GoBack -> webView.goBack() | ||
| is BackAction.NavigateToRoot -> { | ||
| clearHistory = true | ||
| loadedUrl = action.rootUrl | ||
| webView.loadUrl(action.rootUrl.toString()) | ||
|
Comment on lines
+464
to
+466
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason to not call
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. presenter.load() does a full reconnect cycle (URL resolution, auth check, security level dialog). For NavigateToRoot we stay on the same server and only change the path to / -going through the presenter would add overhead and could trigger side effects like the security level prompt.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually I think we want this side effect, the overhead is very small here and safer. @jpelgrom wdyt?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I looked into replacing the manual loadUrl call with presenter.load(lifecycle), but that wouldn't give us the behavior we need: collectUrlStateChanges only sets keepHistory = false when baseUrlChanged || isNewServer. For a NavigateToRoot action the base URL stays the same -so presenter.load() would pass keepHistory = true and leave the stale entries in the back stack. That's exactly what we want to avoid here (the user pressed back to get out of a sub-page). Going through the presenter would require adding a new loadRoot() method (or a forceClearHistory parameter), which felt like a bigger API change than this PR should do. the current code sets clearHistory/loadedUrl and calls webView.loadUrl directly, which mirrors what WebViewPresenterImpl.loadUrl does internally for this edge case. |
||
| } | ||
| BackAction.None -> { | ||
| // Already on root — let the system handle back (exit app). | ||
| // We must temporarily disable this callback so that the | ||
| // dispatcher invokes the next handler in the chain (the | ||
| // default Activity handler which finishes the activity). | ||
| // Re-enabling afterwards keeps the callback functional in | ||
| // case the activity is not destroyed (e.g. multi-window). | ||
| isEnabled = false | ||
| onBackPressedDispatcher.onBackPressed() | ||
| isEnabled = true | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -646,7 +667,14 @@ class WebViewActivity : | |
|
|
||
| override fun doUpdateVisitedHistory(view: WebView?, url: String?, isReload: Boolean) { | ||
| super.doUpdateVisitedHistory(view, url, isReload) | ||
| onBackPressed.isEnabled = canGoBack() | ||
| // Enable the callback when there's browser history OR when the | ||
| // current URL has a non-root path. Without the non-root check, | ||
| // pressing back on e.g. /history with empty history would skip | ||
| // the NavigateToRoot step and exit the app directly. | ||
| // It also keeps predictive back animations working on Android 14+, | ||
| // since the system needs to know upfront that we'll handle the gesture. | ||
| onBackPressed.isEnabled = canGoBack() || | ||
| url?.toUri()?.hasNonRootPath() == true | ||
|
Gifford47 marked this conversation as resolved.
|
||
| presenter.stopScanningForImprov(false) | ||
| } | ||
| } | ||
|
|
@@ -1559,12 +1587,15 @@ class WebViewActivity : | |
| showSystemUI() | ||
| } | ||
|
|
||
| var path = intent.getStringExtra(EXTRA_PATH) | ||
| if (path?.startsWith("entityId:") == true) { | ||
| val intentPath = intent.getStringExtra(EXTRA_PATH) | ||
| // Let the presenter handle falling back to the current WebView path | ||
| // when no explicit navigation path is set. See https://github.com/home-assistant/android/issues/4983 | ||
| var path: String? = intentPath | ||
| if (intentPath?.startsWith("entityId:") == true) { | ||
| // Get the entity ID from a string formatted "entityId:domain.entity" | ||
| // https://github.com/home-assistant/core/blob/dev/homeassistant/core.py#L159 | ||
| val pattern = "(?<=^entityId:)((?!.+__)(?!_)[\\da-z_]+(?<!_)\\.(?!_)[\\da-z_]+(?<!_)$)".toRegex() | ||
| val entity = pattern.find(path)?.value ?: "" | ||
| val entity = pattern.find(intentPath)?.value ?: "" | ||
| if ( | ||
| entity.isNotBlank() && | ||
| serverManager.getServer(presenter.getActiveServer())?.version?.isAtLeast(2025, 6, 0) == true | ||
|
|
@@ -1580,6 +1611,10 @@ class WebViewActivity : | |
| } | ||
| } | ||
|
|
||
| override fun getCurrentWebViewRelativeUrl(): String? { | ||
| return webView.url?.toUri()?.toRelativeUrl(excludeParams = setOf("external_auth")) | ||
|
TimoPtr marked this conversation as resolved.
|
||
| } | ||
|
Gifford47 marked this conversation as resolved.
|
||
|
|
||
| override suspend fun unlockAppIfNeeded() { | ||
| appLocked.value = presenter.isAppLocked() | ||
| if (appLocked.value) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.