From 5ba0ccbe93157234912d194c054a5e952d905837 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Thu, 22 Feb 2024 20:28:21 +0600 Subject: [PATCH 1/4] fix SettingsVC readding on pane change; fix native ui session restoration losing forward history and re-restoring --- DuckDuckGo/Tab/Model/Tab.swift | 6 +++++- DuckDuckGo/Tab/View/BrowserTabViewController.swift | 9 ++++++--- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index 5c1c11d1db..684540e635 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -984,9 +984,13 @@ protocol NewWindowPolicyDecisionMaker { @MainActor private func shouldReload(_ url: URL, shouldLoadInBackground: Bool) -> Bool { + var isNativeUI: Bool { + let content = TabContent.contentFromURL(url, source: .ui) + return !content.isUrl && content.urlForWebView != nil + } // don‘t reload in background unless shouldLoadInBackground guard url.isValid, - webView.superview != nil || shouldLoadInBackground, + webView.superview != nil || shouldLoadInBackground || isNativeUI, // don‘t reload when already loaded webView.url != url || error != nil else { return false } diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index 69a680f172..6e88867376 100644 --- 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 { From a84f70acf71e476966dcbd96a1bdabe645654bae Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Thu, 22 Feb 2024 20:34:10 +0600 Subject: [PATCH 2/4] only shouldReload for native UI session restore --- DuckDuckGo/Tab/Model/Tab.swift | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index 684540e635..9a3a13d0a0 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -984,13 +984,14 @@ protocol NewWindowPolicyDecisionMaker { @MainActor private func shouldReload(_ url: URL, shouldLoadInBackground: Bool) -> Bool { - var isNativeUI: Bool { + var isSessionRestoredNativeUI: Bool { + guard case .loadCachedFromTabContent = self.interactionState else { return false } let content = TabContent.contentFromURL(url, source: .ui) return !content.isUrl && content.urlForWebView != nil } // don‘t reload in background unless shouldLoadInBackground guard url.isValid, - webView.superview != nil || shouldLoadInBackground || isNativeUI, + webView.superview != nil || shouldLoadInBackground || isSessionRestoredNativeUI, // don‘t reload when already loaded webView.url != url || error != nil else { return false } From 278744013ef45e5eb6b93080131b135dcf09b50a Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Fri, 23 Feb 2024 09:48:28 +0600 Subject: [PATCH 3/4] fix switching between settings tabs; refactor reloadIfNeeded --- DuckDuckGo/Tab/Model/Tab.swift | 75 ++++++++++--------- .../Tab/View/BrowserTabViewController.swift | 8 +- 2 files changed, 45 insertions(+), 38 deletions(-) diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index 9a3a13d0a0..5901866b29 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -943,55 +943,62 @@ 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 = 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) + } + + var request = URLRequest(url: url, cachePolicy: source.cachePolicy) + if #available(macOS 12.0, *), content.isUserEnteredUrl { + request.attribution = .user } - return nil + + return webView.navigator(distributedNavigationDelegate: navigationDelegate) + .load(request, withExpectedNavigationType: source.navigationType) } @MainActor private func shouldReload(_ url: URL, shouldLoadInBackground: Bool) -> Bool { - var isSessionRestoredNativeUI: Bool { - guard case .loadCachedFromTabContent = self.interactionState else { return false } - let content = TabContent.contentFromURL(url, source: .ui) - return !content.isUrl && content.urlForWebView != nil - } // don‘t reload in background unless shouldLoadInBackground guard url.isValid, - webView.superview != nil || shouldLoadInBackground || isSessionRestoredNativeUI, + webView.superview != nil || shouldLoadInBackground, // don‘t reload when already loaded webView.url != url || error != nil else { return false } diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index 6e88867376..8970e60ea5 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -974,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 } @@ -1100,7 +1100,7 @@ extension BrowserTabViewController { if isWebViewFirstResponder { self.setFirstResponderAfterAdding = true } - self.showTabContent(of: self.tabCollectionViewModel.selectedTabViewModel) + self.showTabContent(of: self.tabViewModel) self.webViewSnapshot?.removeFromSuperview() } } From ce196b8f15450bff59108c27f3c6aa7e06bb69a2 Mon Sep 17 00:00:00 2001 From: Alexey Martemyanov Date: Fri, 23 Feb 2024 09:51:19 +0600 Subject: [PATCH 4/4] extra check --- DuckDuckGo/Tab/Model/Tab.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/Tab/Model/Tab.swift b/DuckDuckGo/Tab/Model/Tab.swift index 5901866b29..b174ace747 100644 --- a/DuckDuckGo/Tab/Model/Tab.swift +++ b/DuckDuckGo/Tab/Model/Tab.swift @@ -968,7 +968,7 @@ protocol NewWindowPolicyDecisionMaker { guard let (url, source, forceReload) = tabContentReloadInfo(for: content, shouldLoadInBackground: shouldLoadInBackground), forceReload || shouldReload(url, shouldLoadInBackground: shouldLoadInBackground) else { return nil } - if case .settings = webView.url.flatMap({ TabContent.contentFromURL($0, source: .ui) }) { + 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