Fix same-document navigation breaking current main frame navigation (#…

…873) Task/Issue URL: https://app.asana.com/0/1202406491309510/1207272003030126/f
duckduckgo · Jul 1, 2024 · ae719ad · ae719ad
1 parent be669f8
commit ae719ad
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 11 deletions.
diff --git a/Sources/Navigation/DistributedNavigationDelegate.swift b/Sources/Navigation/DistributedNavigationDelegate.swift
@@ -881,21 +881,21 @@ extension DistributedNavigationDelegate: WKNavigationDelegate {
             }
 
         } else {
-            // don‘t mark extra Session State Pop navigations as `current` when there‘s a `current` Anchor navigation stored in `startedNavigation`
-            let isCurrent = if let startedNavigation {
-                !(startedNavigation.navigationAction.navigationType.isSameDocumentNavigation && startedNavigation.isCurrent)
-            } else {
-                true
-            }
+            let shouldBecomeCurrent = {
+                guard let startedNavigation else { return true } // no current navigation, make the same-doc navigation current
+                guard startedNavigation.navigationAction.navigationType.isSameDocumentNavigation else { return false } // don‘t intrude into current non-same-doc navigation
+                // don‘t mark extra Session State Pop navigations as `current` when there‘s a `current` same-doc Anchor navigation stored in `startedNavigation`
+                return !startedNavigation.isCurrent
+            }()
 
-            navigation = Navigation(identity: NavigationIdentity(wkNavigation), responders: responders, state: .expected(nil), isCurrent: isCurrent)
+            navigation = Navigation(identity: NavigationIdentity(wkNavigation), responders: responders, state: .expected(nil), isCurrent: shouldBecomeCurrent)
             let request = wkNavigation?.request ?? URLRequest(url: webView.url ?? .empty)
             let navigationAction = NavigationAction(request: request, navigationType: .sameDocumentNavigation(navigationType), currentHistoryItemIdentity: currentHistoryItemIdentity, redirectHistory: nil, isUserInitiated: wkNavigation?.isUserInitiated ?? false, sourceFrame: .mainFrame(for: webView), targetFrame: .mainFrame(for: webView), shouldDownload: false, mainFrameNavigation: navigation)
             navigation.navigationActionReceived(navigationAction)
-            os_log("new same-doc navigation(.%d): %s (%s): %s, isCurrent: %d", log: log, type: .debug, wkNavigationType, wkNavigation.debugDescription, navigation.debugDescription, navigationAction.debugDescription, isCurrent ? 1 : 0)
+            os_log("new same-doc navigation(.%d): %s (%s): %s, isCurrent: %d", log: log, type: .debug, wkNavigationType, wkNavigation.debugDescription, navigation.debugDescription, navigationAction.debugDescription, shouldBecomeCurrent ? 1 : 0)
 
             // store `current` navigations in `startedNavigation` to get `currentNavigation` published
-            if isCurrent {
+            if shouldBecomeCurrent {
                 self.startedNavigation = navigation
             }
             // mark Navigation as finished as we‘re in __did__SameDocumentNavigation

diff --git a/Tests/NavigationTests/SameDocumentNavigationTests.swift b/Tests/NavigationTests/SameDocumentNavigationTests.swift
@@ -442,9 +442,9 @@ class SameDocumentNavigationTests: DistributedNavigationDelegateTestsBase {
             .response(Nav(action: navAct(1), .responseReceived, resp: .resp(urls.local, data.sessionStatePushClientRedirectData.count, headers: .default + ["Content-Type": "text/html"]))),
             .didCommit(Nav(action: navAct(1), .responseReceived, resp: resp(0), .committed)),
 
-            .didSameDocumentNavigation(Nav(action: NavAction(req(urls.localHashed1, [:]), .sameDocumentNavigation(.sessionStatePush), from: history[1], src: main(urls.localHashed1)), .finished), 1),
+            .didSameDocumentNavigation(Nav(action: NavAction(req(urls.localHashed1, [:]), .sameDocumentNavigation(.sessionStatePush), from: history[1], src: main(urls.localHashed1)), .finished, isCurrent: false), 1),
 
-            .didFinish(Nav(action: navAct(1), .finished, resp: resp(0), .committed, isCurrent: false)),
+            .didFinish(Nav(action: navAct(1), .finished, resp: resp(0), .committed, isCurrent: true)),
         ])
     }