From fa16cfa28fe8245ab4da468e3a4e8f5b4ba774c0 Mon Sep 17 00:00:00 2001 From: Balazs Perlaki-Horvath Date: Mon, 13 Nov 2023 21:34:05 +0100 Subject: [PATCH] Fix opening new tabs on macos(#518) --- App/CompactViewController.swift | 2 +- Model/Utilities/URL.swift | 6 ++- ViewModel/BrowserViewModel.swift | 52 +++++++++++++++---- Views/BrowserTab.swift | 2 +- Views/ViewModifiers/ExternalLinkHandler.swift | 7 ++- 5 files changed, 53 insertions(+), 16 deletions(-) diff --git a/App/CompactViewController.swift b/App/CompactViewController.swift index 9cb0a84e..a1348726 100644 --- a/App/CompactViewController.swift +++ b/App/CompactViewController.swift @@ -130,7 +130,7 @@ private struct Content: View { .focusedSceneValue(\.browserViewModel, browser) .focusedSceneValue(\.canGoBack, browser.canGoBack) .focusedSceneValue(\.canGoForward, browser.canGoForward) - .modifier(ExternalLinkHandler()) + .modifier(ExternalLinkHandler(externalURL: browser.externalURL)) .onAppear { browser.updateLastOpened() } diff --git a/Model/Utilities/URL.swift b/Model/Utilities/URL.swift index a8a468f1..fed817ed 100644 --- a/Model/Utilities/URL.swift +++ b/Model/Utilities/URL.swift @@ -16,6 +16,10 @@ extension URL { var isKiwixURL: Bool { return scheme?.caseInsensitiveCompare("kiwix") == .orderedSame } - + + var isExternal: Bool { + ["http", "https"].contains(scheme) + } + static let documentDirectory = try! FileManager.default.url(for: .documentDirectory, in: .userDomainMask, appropriateFor: nil, create: false) } diff --git a/ViewModel/BrowserViewModel.swift b/ViewModel/BrowserViewModel.swift index 2354f6b4..87efe7e6 100644 --- a/ViewModel/BrowserViewModel.swift +++ b/ViewModel/BrowserViewModel.swift @@ -13,9 +13,9 @@ import WebKit import OrderedCollections -class BrowserViewModel: NSObject, ObservableObject, - WKNavigationDelegate, WKScriptMessageHandler, WKUIDelegate, - NSFetchedResultsControllerDelegate +final class BrowserViewModel: NSObject, ObservableObject, + WKNavigationDelegate, WKScriptMessageHandler, WKUIDelegate, + NSFetchedResultsControllerDelegate { static private var cache = OrderedDictionary() @@ -45,14 +45,17 @@ class BrowserViewModel: NSObject, ObservableObject, @Published private(set) var outlineItems = [OutlineItem]() @Published private(set) var outlineItemTree = [OutlineItem]() @Published private(set) var url: URL? - + @Published var externalURL: URL? + let tabID: NSManagedObjectID? let webView: WKWebView private var canGoBackObserver: NSKeyValueObservation? private var canGoForwardObserver: NSKeyValueObservation? private var titleURLObserver: AnyCancellable? private var bookmarkFetchedResultsController: NSFetchedResultsController? - + /// A temporary placeholder for the url that should be opened in a new tab, set on macOS only + private static var urlForNewTab: URL? + // MARK: - Lifecycle init(tabID: NSManagedObjectID? = nil) { @@ -66,7 +69,11 @@ class BrowserViewModel: NSObject, ObservableObject, webView.interactionState = tab.interactionState url = webView.url } - + if let urlForNewTab = Self.urlForNewTab { + url = urlForNewTab + load(url: urlForNewTab) + } + // configure web view webView.allowsBackForwardNavigationGestures = true webView.configuration.defaultWebpagePreferences.preferredContentMode = .mobile // for font adjustment to work @@ -178,8 +185,8 @@ class BrowserViewModel: NSObject, ObservableObject, decisionHandler(.cancel) } else if url.isKiwixURL { decisionHandler(.allow) - } else if url.scheme == "http" || url.scheme == "https" { - NotificationCenter.default.post(name: .externalLink, object: nil, userInfo: ["url": url]) + } else if url.isExternal { + externalURL = url decisionHandler(.cancel) } else if url.scheme == "geo" { if FeatureFlags.map { @@ -234,7 +241,34 @@ class BrowserViewModel: NSObject, ObservableObject, } // MARK: - WKUIDelegate - +#if os(macOS) + func webView(_ webView: WKWebView, createWebViewWith configuration: WKWebViewConfiguration, + for navigationAction: WKNavigationAction, windowFeatures: WKWindowFeatures) -> WKWebView? { + + guard navigationAction.targetFrame == nil else { return nil } + guard let newUrl = navigationAction.request.url else { return nil } + + // open external link in default browser + guard newUrl.isExternal == false else { + externalURL = newUrl + return nil + } + + // create new tab + guard let currentWindow = NSApp.keyWindow, + let windowController = currentWindow.windowController else { return nil } + // store the new url in a static way + Self.urlForNewTab = newUrl + // this creates a new BrowserViewModel + windowController.newWindowForTab(self) + // now reset the static url to nil, as the new BrowserViewModel already has it + Self.urlForNewTab = nil + guard let newWindow = NSApp.keyWindow, currentWindow != newWindow else { return nil } + currentWindow.addTabbedWindow(newWindow, ordered: .above) + return nil + } +#endif + #if os(iOS) func webView(_ webView: WKWebView, contextMenuConfigurationForElement elementInfo: WKContextMenuElementInfo, diff --git a/Views/BrowserTab.swift b/Views/BrowserTab.swift index 9a080449..2ade7102 100644 --- a/Views/BrowserTab.swift +++ b/Views/BrowserTab.swift @@ -38,7 +38,7 @@ struct BrowserTab: View { .focusedSceneValue(\.browserViewModel, browser) .focusedSceneValue(\.canGoBack, browser.canGoBack) .focusedSceneValue(\.canGoForward, browser.canGoForward) - .modifier(ExternalLinkHandler()) + .modifier(ExternalLinkHandler(externalURL: $browser.externalURL)) .searchable(text: $search.searchText, placement: .toolbar) .modify { view in #if os(macOS) diff --git a/Views/ViewModifiers/ExternalLinkHandler.swift b/Views/ViewModifiers/ExternalLinkHandler.swift index a3a1d0cb..e35e83fc 100644 --- a/Views/ViewModifiers/ExternalLinkHandler.swift +++ b/Views/ViewModifiers/ExternalLinkHandler.swift @@ -14,8 +14,7 @@ struct ExternalLinkHandler: ViewModifier { @State private var isAlertPresented = false @State private var activeAlert: ActiveAlert? @State private var activeSheet: ActiveSheet? - - private let externalLink = NotificationCenter.default.publisher(for: .externalLink) + @Binding var externalURL: URL? enum ActiveAlert { case ask(url: URL) @@ -28,8 +27,8 @@ struct ExternalLinkHandler: ViewModifier { } func body(content: Content) -> some View { - content.onReceive(externalLink) { notification in - guard let url = notification.userInfo?["url"] as? URL else { return } + content.onChange(of: externalURL) { url in + guard let url else { return } switch Defaults[.externalLinkLoadingPolicy] { case .alwaysAsk: isAlertPresented = true