fix SettingsVC readding on pane change (#2243)

Task/Issue URL: https://app.asana.com/0/1177771139624306/1206667437436846/f - fix SettingsVC readding on pane change; - fix native ui session restoration losing forward history and re-restoring
duckduckgo · Feb 23, 2024 · 863266a · 863266a
1 parent c574193
commit 863266a
Show file tree

Hide file tree

Showing 2 changed files with 50 additions and 35 deletions.
diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift
@@ -943,43 +943,55 @@ protocol NewWindowPolicyDecisionMaker {
         audioState = webView.audioState()
     }
 
+    private func tabContentReloadInfo(for content: TabContent, shouldLoadInBackground: Bool) -> (url: URL, source: TabContent.URLSource, forceReload: Bool)? {
+        switch content {
+        case .url(let url, _, source: let source):
+            let forceReload = url.absoluteString == source.userEnteredValue ? shouldLoadInBackground : (source == .reload)
+            return (url, source, forceReload: forceReload)
+
+        case .subscription(let url):
+            return (url, .ui, forceReload: false)
+
+        case .newtab, .bookmarks, .onboarding, .dataBrokerProtection, .settings:
+            guard let contentUrl = content.urlForWebView, webView.url != contentUrl else { return nil }
+
+            return (contentUrl, .ui, forceReload: true) // always navigate built-in ui (duck://) urls
+
+        case .none:
+            return nil
+        }
+    }
+
     @MainActor(unsafe)
     @discardableResult
     private func reloadIfNeeded(shouldLoadInBackground: Bool = false) -> ExpectedNavigation? {
-        let source: TabContent.URLSource
-        let url: URL
-        if case .url(let contentUrl, _, source: let urlSource) = content {
-            url = contentUrl
-            source = urlSource
-        } else if let contentUrl = content.urlForWebView {
-            url = contentUrl
-            source = .ui
-        } else {
+        guard let (url, source, forceReload) = tabContentReloadInfo(for: content, shouldLoadInBackground: shouldLoadInBackground),
+              forceReload || shouldReload(url, shouldLoadInBackground: shouldLoadInBackground) else { return nil }
+
+        if case .settings = content, case .settings = webView.url.flatMap({ TabContent.contentFromURL($0, source: .ui) }) {
+            // replace WebView URL without adding a new history item if switching settings panes
+            webView.evaluateJavaScript("location.replace('\(url.absoluteString.escapedJavaScriptString())')", in: nil, in: .defaultClient)
             return nil
         }
 
-        let forceReload = (url.absoluteString == content.userEnteredValue) ? shouldLoadInBackground : (source == .reload)
-        if forceReload || shouldReload(url, shouldLoadInBackground: shouldLoadInBackground) {
-            if webView.url == url, webView.backForwardList.currentItem?.url == url, !webView.isLoading {
-                return reload()
-            }
-            if restoreInteractionStateDataIfNeeded() { return nil /* session restored */ }
-            invalidateInteractionStateData()
-
-            if url.isFileURL {
-                return webView.navigator(distributedNavigationDelegate: navigationDelegate)
-                    .loadFileURL(url, allowingReadAccessTo: URL(fileURLWithPath: "/"), withExpectedNavigationType: source.navigationType)
-            }
-
-            var request = URLRequest(url: url, cachePolicy: source.cachePolicy)
-            if #available(macOS 12.0, *), content.isUserEnteredUrl {
-                request.attribution = .user
-            }
+        if webView.url == url, webView.backForwardList.currentItem?.url == url, !webView.isLoading {
+            return reload()
+        }
+        if restoreInteractionStateDataIfNeeded() { return nil /* session restored */ }
+        invalidateInteractionStateData()
 
+        if url.isFileURL {
             return webView.navigator(distributedNavigationDelegate: navigationDelegate)
-                .load(request, withExpectedNavigationType: source.navigationType)
+                .loadFileURL(url, allowingReadAccessTo: URL(fileURLWithPath: "/"), withExpectedNavigationType: source.navigationType)
         }
-        return nil
+
+        var request = URLRequest(url: url, cachePolicy: source.cachePolicy)
+        if #available(macOS 12.0, *), content.isUserEnteredUrl {
+            request.attribution = .user
+        }
+
+        return webView.navigator(distributedNavigationDelegate: navigationDelegate)
+            .load(request, withExpectedNavigationType: source.navigationType)
     }
 
     @MainActor

diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift
@@ -507,13 +507,16 @@ final class BrowserTabViewController: NSViewController {
             addAndLayoutChild(bookmarksViewControllerCreatingIfNeeded())
 
         case let .settings(pane):
-            removeAllTabContent()
             let preferencesViewController = preferencesViewControllerCreatingIfNeeded()
+            if preferencesViewController.parent !== self {
+                removeAllTabContent()
+            }
             if let pane = pane, preferencesViewController.model.selectedPane != pane {
                 preferencesViewController.model.selectPane(pane)
             }
-            addAndLayoutChild(preferencesViewController)
-
+            if preferencesViewController.parent !== self {
+                addAndLayoutChild(preferencesViewController)
+            }
         case .onboarding:
             removeAllTabContent()
             if !OnboardingViewModel.isOnboardingFinished {
@@ -971,12 +974,12 @@ extension BrowserTabViewController: TabDownloadsDelegate {
 extension BrowserTabViewController: BrowserTabSelectionDelegate {
 
     func selectedTabContent(_ content: Tab.TabContent) {
-        tabCollectionViewModel.selectedTabViewModel?.tab.setContent(content)
-        showTabContent(of: tabCollectionViewModel.selectedTabViewModel)
+        tabViewModel?.tab.setContent(content)
+        showTabContent(of: tabViewModel)
     }
 
     func selectedPreferencePane(_ identifier: PreferencePaneIdentifier) {
-        guard let selectedTab = tabCollectionViewModel.selectedTabViewModel?.tab else {
+        guard let selectedTab = tabViewModel?.tab else {
             return
         }
 
@@ -1097,7 +1100,7 @@ extension BrowserTabViewController {
                 if isWebViewFirstResponder {
                     self.setFirstResponderAfterAdding = true
                 }
-                self.showTabContent(of: self.tabCollectionViewModel.selectedTabViewModel)
+                self.showTabContent(of: self.tabViewModel)
                 self.webViewSnapshot?.removeFromSuperview()
             }
         }