Fix legacy WebView same-origin URL routing#7043
Conversation
There was a problem hiding this comment.
⚠️ Not ready to approve
The new origin-based helper call can throw for non-HTTP(S)/hostless URIs (e.g., mailto:) and the added Robolectric test is missing the module-standard @Config(application = HiltTestApplication::class) configuration.
Pull request overview
This PR refactors the legacy WebViewActivity URL override logic to use the shared origin-comparison helper (hasSameOrigin) instead of a substring check, so same-origin Home Assistant links don’t get misrouted to an external browser (related to #7027 behavior).
Changes:
- Introduce
shouldOpenInExternalBrowser(...)to centralize the origin-based external-link decision. - Update
WebViewActivity.shouldOverrideUrlLoadingto use the new helper instead ofcontains(...). - Add Robolectric unit tests covering same-origin vs different-origin routing (and extendable to scheme edge cases).
File summaries
| File | Description |
|---|---|
| app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt | Switch legacy URL override decision from substring matching to origin-based helper usage |
| app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewUrlOverride.kt | New helper function encapsulating “open externally?” decision using hasSameOrigin |
| app/src/test/kotlin/io/homeassistant/companion/android/webview/WebViewUrlOverrideTest.kt | New Robolectric unit tests validating URL routing decisions |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 4
Note
Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.
This doesn't explain why navigation would happen on closing the app, and linked to the last update. The line you're referring to hasn't changed in months. |
9463402 to
5298fec
Compare
|
Thanks, agreed. I checked the attached issue log and it shows the WebView renderer going away first, followed by I narrowed the PR wording to "potentially addresses part of #7027" and kept the code scoped to one legacy routing issue: same-origin URLs with a different path no longer get treated as external. I also addressed the review findings by parsing the target URL once, guarding non-HTTP(S)/hostless targets before |
|
Could you describe how to reproduce the initial issue? I never managed to end up into this situation. |
Summary
Potentially addresses part of #7027.
The legacy WebViewActivity URL override path used a substring check against the current WebView URL to decide whether a URL should open externally. That can incorrectly classify same-origin Home Assistant URLs with a different path as external.
This updates the legacy override path to compare origins instead. Same-origin Home Assistant URLs stay in the app, different origins still open externally, and non-HTTP(S)/hostless URLs continue to be handed to Android without calling the origin helper.
Checklist
Screenshots
Not applicable; this changes URL routing behavior only.
Any other notes
Verification performed:
git diff --check./gradlew --no-daemon --console=plain :app:testFullDebugUnitTest --tests io.homeassistant.companion.android.webview.WebViewUrlOverrideTest./gradlew --no-daemon --console=plain :app:ktlintCheck