Fix notification/deep-link URL being discarded on cold start (#4145)#4739
Draft
dimdal wants to merge 3 commits into
Draft
Fix notification/deep-link URL being discarded on cold start (#4145)#4739dimdal wants to merge 3 commits into
dimdal wants to merge 3 commits into
Conversation
…t#4145) On cold start webview starts blank, so loadActiveURLIfNeeded() (fired from viewWillAppear + HA-connect notification) races open(inline:) and loads the default server URL instead of the notification's URL. Track the explicitly requested URL (pendingOpenInlineURL); give it priority in loadActiveURLIfNeeded over restored/default; clear on didFinish. Wins every load ordering, so URL no longer discarded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![home-assistant[bot]](https://avatars.githubusercontent.com/in/97978?s=60&v=4){kind=small}
|
Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for prioritizing an explicitly requested open(inline:) URL over the default/restored active server URL to prevent cold-start navigation races (re: #4145).
Changes:
- Introduces
pendingOpenInlineURLstate to preserve an explicit deep-link/notification navigation until it completes. - Updates active URL loading to prefer a pending explicit URL when it targets the active server.
- Adds unit tests for URL prioritization and for recording the pending inline URL.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Tests/App/WebView/WebViewControllerTests.swift | Adds tests covering prioritized inline URL selection and recording pending inline URLs. |
| Sources/App/Frontend/WebView/WebViewController.swift | Introduces pendingOpenInlineURL to track explicit navigations that should override default/restored loads. |
| Sources/App/Frontend/WebView/WebViewController+WebKitDelegates.swift | Clears pendingOpenInlineURL when navigation finishes to stop forcing the explicit URL. |
| Sources/App/Frontend/WebView/WebViewController+URLLoading.swift | Prefers the pending explicit inline URL in loadActiveURLIfNeeded() when it targets the active server. |
| Sources/App/Frontend/WebView/WebViewController+Navigation.swift | Records open(inline:) requests into pendingOpenInlineURL to survive cold-start reload races. |
Comment on lines
+90
to
+91
| // the explicit navigation has landed; stop forcing it on subsequent active-URL loads (#4145) | ||
| pendingOpenInlineURL = nil |
| /// `loadActiveURLIfNeeded()` — fired from `viewWillAppear` and the HA-connect notification — would | ||
| /// otherwise race with and overwrite this navigation with the default server URL, discarding the | ||
| /// requested URL (#4145). Cleared once a navigation finishes (`didFinish`). | ||
| var pendingOpenInlineURL: URL? |
Comment on lines
+168
to
+175
| func testOpenInlineRecordsPendingURLForFrontendPath() { | ||
| let sut = makeSUT() | ||
| let url = URL(string: "https://example.com:8123/lovelace/default")! | ||
|
|
||
| sut.open(inline: url) | ||
|
|
||
| XCTAssertEqual(sut.pendingOpenInlineURL, url) | ||
| } |
…lank - Clear pendingOpenInlineURL on non-cancelled didFail/didFailProvisional so a permanently-failing URL doesn't get re-forced on later active-URL loads. Cancelled failures keep it (a superseding load must not lose it). - didFinish: don't clear on about:blank (loaded by showNoActiveURLError), which would drop the pending URL before the real navigation runs. - Note baseIsEqual same-server semantics on prioritizedInlineURL. - Add tests for real-failure clear and cancelled-keeps-pending. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… too Move the pendingOpenInlineURL clear out of the URLError cast in didFail so any non-cancelled committed-navigation failure clears it, not just URLError. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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
Fixes #4145. Tapping a notification with a
url(or aURIaction targeting the Home Assistant frontend) navigates correctly when the app is already running, but on a cold start the URL is discarded and the app just opens to its default page.Root cause — a cold-start race. On cold start the webview starts blank (
webView.url == nil). The notification tap drivesopen(inline:)→load(notificationURL), butloadActiveURLIfNeeded()— fired independently fromviewWillAppearandHomeAssistantAPI.didConnectNotification— loads the default server URL wheneverwebView.url == nil. Whicheverload()lands last wins, and it is usually the default, so the notification URL is lost. When the app is already runningwebView.urlis non-nil,loadActiveURLIfNeeded()early-returns, andopen(inline:)always wins — which matches the reports (works when open, and one reporter seeing it work intermittently is the race surfacing).Fix. Mirror the existing
initialURLrestoration mechanism with apendingOpenInlineURL:open(inline:)records the explicitly requested URL before loading it.loadActiveURLIfNeeded()gives that pending URL priority over the restored/default URL (new pure helperprioritizedInlineURL(...)).didFinishclears it once the navigation lands.Because the pending URL is re-asserted while the webview is still blank and only cleared on a successful navigation, it wins in every ordering of the two loads, so the URL is no longer discarded. Deep links benefit too, since they share the same
open(inline:)path.Scope: this covers URLs that resolve to the HA frontend (server URLs and relative paths such as
/frigate/review/...), which is the path in the issue. The separateaction: URI→ external browser path (openURLInBrowser) is not touched here.Added unit tests in
WebViewControllerTestsfor the priority helper and foropen(inline:)recording the pending URL.Screenshots
No UI change — this is a navigation/timing fix, so there is nothing visual to capture. Behaviour: with the app force-closed, tapping a notification carrying a
urlnow lands on that URL instead of the default dashboard.Link to pull request in Documentation repository
Documentation: home-assistant/companion.home-assistant#
Any other notes
🤖 Generated with Claude Code