[compose UI] Refactor compose path handling#6836
Draft
Gifford47 wants to merge 29 commits into
Draft
Conversation
- Remove duplicate getCurrentWebViewPath() call from Activity (Presenter handles fallback) - Move intent.removeExtra(EXTRA_PATH) before presenter.load() - Fix moreInfoEntity: only set for JS dispatch path, clear for URL path approach - Simplify getCurrentWebViewPath() using takeIf, remove unnecessary try-catch - Use full GitHub issue URL in comment Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Instead of only preserving the path when the connection switches between internal and external URLs, preserve the complete relative URL including query parameters and fragment. This ensures filtered views (e.g. history page with date ranges) survive URL switches seamlessly. - Rename getCurrentWebViewPath() to getCurrentWebViewRelativeUrl() - Extract path + query params + fragment from the current WebView URL - Strip 'external_auth' param to avoid duplication (presenter re-adds it) - Update presenter to use the new method with descriptive variable names Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When the base URL changes (e.g. switching from internal Wi-Fi to external mobile data), old URLs in the back stack become unreachable on the new network. Pressing back would attempt to load those old URLs, causing timeouts and error messages. Track the last base URL in collectUrlStateChanges and set isNewServer=true when a change is detected, which triggers keepHistory=false and clears the navigation history after loading the new URL. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After switching between internal and external URLs, stale history entries from the old connection may remain in the WebView. This change validates that the previous history entry has the same origin as the current URL before navigating back. If the origin differs, it navigates to the base URL instead of attempting to load an unreachable page. Also keeps the back callback enabled when on a non-root path so pressing back navigates to root before exiting. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The WebView interface gained a new getCurrentWebViewRelativeUrl() method but the FakeWebViewContext test wrapper was not updated, causing a compilation failure in unit tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
A relaxed MockK mock returns "" instead of null for String? return types. This caused UrlUtil.handle to normalize the URL with a trailing slash, breaking the exact string assertion in the "previous load in progress" test. Explicitly mock getCurrentWebViewRelativeUrl to return null, matching the real behavior when no WebView page is loaded. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Guard back navigation against about:blank and other non-HTTP history entries that may appear before the first real page has loaded. - Use Uri.Builder in getCurrentWebViewRelativeUrl instead of manual string concatenation for safer URL construction. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Extract URL relative-path logic into a reusable Uri.toRelativeUrl() extension in UrlUtil.kt, using queryParameterNames API instead of string splitting (as suggested by TimoPtr) - Add KDoc documentation to the new extension - Simplify getCurrentWebViewRelativeUrl() to a single-line delegation - Add comments explaining the callback disable/enable pattern and the about:blank edge case in back navigation - Add 10 unit tests for toRelativeUrl() in UriExtensionsTest.kt
Restore original placement to keep git history clean, as requested by TimoPtr.
…tyle - Move hideSystemUI/showSystemUI block back before path processing to preserve original code order and keep git history clean - Refactor toRelativeUrl() query parameter loop to functional style using filterNot/flatMap/forEach as suggested by TimoPtr
- Don't preserve path on server switch to avoid leaking info and broken pages; only preserve on internal/external URL changes - Restore OnBackPressedCallback(webView.canGoBack()) to keep predictive back animations on Android 14+ - Restore entityId comment with core.py link - Make toRelativeUrl KDoc more generic (it's a util function) - Fix misleading test name that suggested null return
- Move cross-origin check + navigate-to-root logic into WebViewBackNavigation.kt so it can be reused by both WebViewActivity and FrontendScreen. - Rename isNewServer → clearHistory in handleUrlState/loadUrl for clarity (requested by TimoPtr). - Fix ktlint indentation in WebViewPresenterImpl.kt:584.
Resolve import conflict: keep BackAction/resolveBackAction imports alongside new BLANK_URL import from upstream.
Track the last base URL in collectUrlStateChanges so a connection-type switch reuses the current WebView relative URL instead of reloading the root. Tighten resolveBackAction tests to call the pure function directly.
- BackAction: inline previousUrl lookup (TimoPtr suggestion) - WebViewPresenterImpl: simplify keepHistory expression with De Morgan - WebViewPresenterImpl: add unit test for base URL change path preservation - WebViewActivity: expand doUpdateVisitedHistory comment with predictive back rationale - WebViewBackNavigationTest: capitalize Given to match project convention - ktlint: rename WebViewBackNavigation.kt -> BackAction.kt, fix import order
- BackAction: make inner resolveBackAction private, move resolution KDoc to public function so it shows at call sites - WebViewPresenterImpl: gate baseUrlChanged path preservation with !isNewServer (Copilot bug: prevents leaking path across server switches) - WebViewPresenterImpl: simplify baseUrlChanged using lastBaseUrl nullability - WebViewBackNavigationTest: drop Robolectric in favor of mockk-based WebView mocks, exercise logic through the public resolveBackAction overload
- BackAction: guard against currentIndex == 0 in previousUrl lookup - WebViewActivity: drop redundant moreInfoEntity = "" reset in the URL-path branch; moreInfoEntity is never set in this branch, so clearing it is unnecessary
…eToRoot guard Require previousUrl != null for BackAction.NavigateToRoot so that pressing back with an empty back-stack (e.g. opened via deeplink to /history) returns None instead of synthesizing a navigation to root. This aligns resolveBackAction with jpelgrom's concern about losing Android's predictive-back peek animation.
The OnBackPressedCallback in WebViewActivity was kept enabled whenever the loaded URL had a non-root path, which caused Android to assume the app would handle the back gesture and suppress the predictive-back peek animation — even on the root screen with no browser history. Drop the hasNonRootPath() fallback in doUpdateVisitedHistory so the callback is only enabled when webView.canGoBack() is true. With the earlier resolveBackAction guard requiring previousUrl != null, NavigateToRoot is now produced only when the WebView genuinely has back-stack entries — so the fallback was redundant and only served to keep the callback armed against predictive-back.
The Compose FrontendScreen's BackHandler was unconditionally enabled when the WebView had an onBackPressed callback attached, which caused Android to always assume the app would handle the back gesture and suppress the predictive-back peek animation — even on the root screen where there was nothing left to go back to. Hoist canGoBack to the caller so HAWebView's BackHandler is only enabled when the WebView has back-stack entries. When false, the gesture falls through to the surrounding NavHost / activity, which lets the system show the peek animation. - HAWebViewClient exposes an onCanGoBackChanged hook from doUpdateVisitedHistory (catches SPA pushState) and onPageFinished. - FrontendViewModel and ConnectionViewModel expose canGoBack as a StateFlow driven by that hook. - HAWebView's API changes: onBackPressed parameter is removed and replaced with a canGoBack: Boolean input. The internal BackHandler is only enabled when canGoBack is true; otherwise the gesture falls through to the surrounding NavHost.
navigateToFrontend() pushed FrontendRoute on top of the (now finished)
OnboardingRoute, so pressing back from the frontend popped to an empty
onboarding screen (white) instead of exiting the app and triggering the
predictive-back peek animation.
Use popUpTo(0) { inclusive = true } when navigating from onboarding to
frontend so FrontendRoute becomes the only entry on the stack.
The WebView's back/forward list accumulated every URL loaded via loadUrl(), including the about:blank placeholder used during the LoadServer / Insecure / SecurityLevelRequired states. Pressing back from the root of the frontend then walked through this stale stack, landing on a white about:blank screen instead of exiting the activity. Track the previously finished URL in HAWebViewClient and call webView.clearHistory() once we detect the transition from BLANK_URL to a real content URL. SPA navigation (history.pushState) does not fire onPageFinished and is unaffected.
The BackHandler in HAWebView is now enabled when either the WebView has back-stack entries (canGoBack) OR the caller supplied an explicit onBackPressed callback. This lets screens like ConnectionScreen in the onboarding flow keep their previous behavior of popping the nav stack when the WebView has nothing to go back to — necessary for the ConnectionNavigationTest / WearOnboardingNavigationTest assertions — while FrontendScreen continues to leave onBackPressed null so the gesture falls through to NavHost and Android 14+ predictive-back peek animations work on the main path.
The blank-only check missed network switches (internal <-> external URL on the same server) and server switches: the WebView's back stack ended up with a stale entry that would fail to load on the new origin. Press back after a network switch on a deep page would walk into the old unreachable URL instead of cleanly exiting. Generalise the trigger to "any cross-origin transition to a real URL". Same-origin in-frontend navigation and SPA pushState (which doesn't fire onPageFinished at all) are unaffected. Transitions INTO about:blank are still skipped so the back-stack survives an error overlay and stays usable after recovery. Origin comparison is done with a small string helper so the unit tests keep running in plain JUnit without needing Robolectric or static Uri mocking.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion PR to #6447 (WebView legacy). Brings the same predictive-back behavior to the Compose FrontendScreen and resolves the navigation chain so Android 14+ shows the peek animation properly.
Checklist
Screenshots
Link to pull request in documentation repositories
Refactoring webview PR
Any other notes
6b1efcc — Enable Android 14+ predictive-back by gating BackHandler on canGoBackThe HAWebView BackHandler was unconditionally enabled, which suppressed the predictive-back peek animation. State-hoist canGoBack from caller (FrontendViewModel/ConnectionViewModel) via a new onCanGoBackChanged hook in HAWebViewClient (fires from doUpdateVisitedHistory to catch SPA pushState + from onPageFinished). HAWebView API now takes canGoBack: Boolean; onBackPressed: (() -> Unit)? becomes an optional fallback for screens like onboarding that explicitly need to pop the nav stack (ConnectionScreen).
7e46150 — Clear navigation back-stack after onboarding completes
navigateToFrontend() was pushing FrontendRoute on top of the finished OnboardingRoute, so back from the frontend popped to an empty (white) onboarding screen instead of exiting the app. Use popUpTo(0) { inclusive = true } so FrontendRoute is the only entry.
44ecb1f — Clear WebView history after transitioning out of about:blank
The WebView accumulated every loadUrl() including the about:blank placeholder used by LoadServer/Error/SecurityLevelRequired states. Pressing back from the root walked back into a white about:blank screen. HAWebViewClient.onPageFinished now calls webView.clearHistory() once it detects the transition BLANK_URL → real URL. SPA pushState is unaffected (only fires doUpdateVisitedHistory, not onPageFinished).
Verification
Tested on Android 16 emulator (API 36.1), gesture navigation, with WIPFeature.USE_FRONTEND_V2 = BuildConfig.DEBUG (Compose path)