From dfd756f73f1b806e83b202e74bb80571bf7eff17 Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Tue, 23 Apr 2024 09:29:12 -0500 Subject: [PATCH 1/6] Implement prefer new tabs to windows --- .../xcschemes/sandbox-test-tool.xcscheme | 2 +- DuckDuckGo/Common/Localizables/UserText.swift | 1 + .../Common/Utilities/UserDefaultsWrapper.swift | 1 + DuckDuckGo/Localizable.xcstrings | 12 ++++++++++++ DuckDuckGo/Preferences/Model/TabsPreferences.swift | 11 +++++++++++ .../Preferences/View/PreferencesGeneralView.swift | 1 + DuckDuckGo/Tab/Model/Tab+UIDelegate.swift | 4 ++++ UnitTests/Preferences/TabsPreferencesTests.swift | 7 ++++++- 8 files changed, 37 insertions(+), 2 deletions(-) diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme index eb7e5e26bb..41730d7069 100644 --- a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme +++ b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme @@ -1,7 +1,7 @@ + version = "1.7"> String { diff --git a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift index 7eb2d9c6ad..f76e0b1362 100644 --- a/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift +++ b/DuckDuckGo/Common/Utilities/UserDefaultsWrapper.swift @@ -85,6 +85,7 @@ public struct UserDefaultsWrapper { case currentThemeName = "com.duckduckgo.macos.currentThemeNameKey" case showFullURL = "preferences.appearance.show-full-url" case showAutocompleteSuggestions = "preferences.appearance.show-autocomplete-suggestions" + case preferNewTabsToWindows = "preferences.tabs.prefer-new-tabs-to-windows" case switchToNewTabWhenOpened = "preferences.tabs.switch-to-new-tab-when-opened" case defaultPageZoom = "preferences.appearance.default-page-zoom" case bookmarksBarAppearance = "preferences.appearance.bookmarks-bar" diff --git a/DuckDuckGo/Localizable.xcstrings b/DuckDuckGo/Localizable.xcstrings index 25c32e46c9..b56c48488a 100644 --- a/DuckDuckGo/Localizable.xcstrings +++ b/DuckDuckGo/Localizable.xcstrings @@ -42888,6 +42888,18 @@ } } }, + "preferences-tabs.prefer.new.tabs.to.windows" : { + "comment" : "Option to prefer opening new tabs instead of windows when opening links", + "extractionState" : "extracted_with_value", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "new", + "value" : "Prefer new tabs instead of new windows when opening links" + } + } + } + }, "preferences-tabs.switch.tab.when.opened" : { "comment" : "Option to switch to a new tab when it is opened", "extractionState" : "extracted_with_value", diff --git a/DuckDuckGo/Preferences/Model/TabsPreferences.swift b/DuckDuckGo/Preferences/Model/TabsPreferences.swift index cc5f5c29cf..8e9693b564 100644 --- a/DuckDuckGo/Preferences/Model/TabsPreferences.swift +++ b/DuckDuckGo/Preferences/Model/TabsPreferences.swift @@ -20,9 +20,13 @@ import Foundation protocol TabsPreferencesPersistor { var switchToNewTabWhenOpened: Bool { get set } + var preferNewTabsToWindows: Bool { get set } } struct TabsPreferencesUserDefaultsPersistor: TabsPreferencesPersistor { + @UserDefaultsWrapper(key: .preferNewTabsToWindows, defaultValue: true) + var preferNewTabsToWindows: Bool + @UserDefaultsWrapper(key: .switchToNewTabWhenOpened, defaultValue: false) var switchToNewTabWhenOpened: Bool } @@ -31,6 +35,12 @@ final class TabsPreferences: ObservableObject, PreferencesTabOpening { static let shared = TabsPreferences() + @Published var preferNewTabsToWindows: Bool { + didSet { + persistor.preferNewTabsToWindows = preferNewTabsToWindows + } + } + @Published var switchToNewTabWhenOpened: Bool { didSet { persistor.switchToNewTabWhenOpened = switchToNewTabWhenOpened @@ -39,6 +49,7 @@ final class TabsPreferences: ObservableObject, PreferencesTabOpening { init(persistor: TabsPreferencesPersistor = TabsPreferencesUserDefaultsPersistor()) { self.persistor = persistor + preferNewTabsToWindows = persistor.preferNewTabsToWindows switchToNewTabWhenOpened = persistor.switchToNewTabWhenOpened } diff --git a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift index 67471d9783..23a2e345bf 100644 --- a/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift +++ b/DuckDuckGo/Preferences/View/PreferencesGeneralView.swift @@ -53,6 +53,7 @@ extension Preferences { // SECTION 2: Tabs PreferencePaneSection(UserText.tabs) { PreferencePaneSubSection { + ToggleMenuItem(UserText.preferNewTabsToWindows, isOn: $tabsModel.preferNewTabsToWindows) ToggleMenuItem(UserText.switchToNewTabWhenOpened, isOn: $tabsModel.switchToNewTabWhenOpened) } } diff --git a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift index 8c3cd033ed..ffee4aef26 100644 --- a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift +++ b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift @@ -136,6 +136,10 @@ extension Tab: WKUIDelegate, PrintingUserScriptDelegate { } private func newWindowPolicy(for navigationAction: WKNavigationAction) -> NavigationDecision? { + if !tabsPreferences.preferNewTabsToWindows { + return .allow(.window(active: true, burner: burnerMode.isBurner)) + } + if let newWindowPolicy = self.decideNewWindowPolicy(for: navigationAction) { return newWindowPolicy } diff --git a/UnitTests/Preferences/TabsPreferencesTests.swift b/UnitTests/Preferences/TabsPreferencesTests.swift index a6f6f63861..1d5c52b539 100644 --- a/UnitTests/Preferences/TabsPreferencesTests.swift +++ b/UnitTests/Preferences/TabsPreferencesTests.swift @@ -20,6 +20,7 @@ import XCTest @testable import DuckDuckGo_Privacy_Browser class MockTabsPreferencesPersistor: TabsPreferencesPersistor { + var preferNewTabsToWindows: Bool = false var switchToNewTabWhenOpened: Bool = false } @@ -27,17 +28,21 @@ final class TabsPreferencesTests: XCTestCase { func testWhenInitializedThenItLoadsPersistedValues() { let mockPersistor = MockTabsPreferencesPersistor() + mockPersistor.preferNewTabsToWindows = true mockPersistor.switchToNewTabWhenOpened = true let tabsPreferences = TabsPreferences(persistor: mockPersistor) + XCTAssertTrue(tabsPreferences.preferNewTabsToWindows) XCTAssertTrue(tabsPreferences.switchToNewTabWhenOpened) } - func testWhenSwitchToNewTabUpdatedThenPersistorUpdates() { + func testWhenPreferencesUpdatedThenPersistorUpdates() { let mockPersistor = MockTabsPreferencesPersistor() let tabsPreferences = TabsPreferences(persistor: mockPersistor) + tabsPreferences.preferNewTabsToWindows = true tabsPreferences.switchToNewTabWhenOpened = true + XCTAssertTrue(mockPersistor.preferNewTabsToWindows) XCTAssertTrue(mockPersistor.switchToNewTabWhenOpened) } } From 29f3dfdb40cb6e0097c8fc4261037344f33773ab Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Tue, 23 Apr 2024 09:34:32 -0500 Subject: [PATCH 2/6] Revert sandbox tool --- .../xcshareddata/xcschemes/sandbox-test-tool.xcscheme | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme index 41730d7069..eb7e5e26bb 100644 --- a/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme +++ b/DuckDuckGo.xcodeproj/xcshareddata/xcschemes/sandbox-test-tool.xcscheme @@ -1,7 +1,7 @@ + version = "1.8"> Date: Tue, 23 Apr 2024 10:06:38 -0500 Subject: [PATCH 3/6] Update new window policy to handle cancel case --- DuckDuckGo/Tab/Model/Tab+UIDelegate.swift | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift index ffee4aef26..964b04af1e 100644 --- a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift +++ b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift @@ -136,24 +136,35 @@ extension Tab: WKUIDelegate, PrintingUserScriptDelegate { } private func newWindowPolicy(for navigationAction: WKNavigationAction) -> NavigationDecision? { - if !tabsPreferences.preferNewTabsToWindows { - return .allow(.window(active: true, burner: burnerMode.isBurner)) + func handleWithPreferences(newPolicy: NavigationDecision?) -> NavigationDecision? { + guard let newPolicy else { return nil } + + switch newPolicy { + case .allow(let targetKind): + if !tabsPreferences.preferNewTabsToWindows { + return .allow(.window(active: targetKind.isSelectedTab, burner: burnerMode.isBurner)) + } + case .cancel: + break + } + + return newPolicy } if let newWindowPolicy = self.decideNewWindowPolicy(for: navigationAction) { - return newWindowPolicy + return handleWithPreferences(newPolicy: newWindowPolicy) } // Are we handling custom Context Menu navigation action or link click with a hotkey? for handler in self.newWindowPolicyDecisionMakers ?? [] { guard let newWindowPolicy = handler.decideNewWindowPolicy(for: navigationAction) else { continue } - return newWindowPolicy + return handleWithPreferences(newPolicy: newWindowPolicy) } // allow popups opened from an empty window console let sourceUrl = navigationAction.safeSourceFrame?.safeRequest?.url ?? self.url ?? .empty if sourceUrl.isEmpty || sourceUrl.scheme == URL.NavigationalScheme.about.rawValue { - return .allow(.tab(selected: true, burner: burnerMode.isBurner)) + return handleWithPreferences(newPolicy: .allow(.tab(selected: true, burner: burnerMode.isBurner))) } return nil From b37aa79a227e0cbeaf08c4bb72ec4336786a7347 Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Tue, 23 Apr 2024 13:42:21 -0500 Subject: [PATCH 4/6] Clean up API around window preference --- DuckDuckGo/Tab/Model/NewWindowPolicy.swift | 31 +++++++++++++++--- DuckDuckGo/Tab/Model/Tab+UIDelegate.swift | 32 ++++--------------- .../TabExtensions/ContextMenuManager.swift | 12 ++++++- .../Tab/View/BrowserTabViewController.swift | 2 +- 4 files changed, 45 insertions(+), 32 deletions(-) diff --git a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift index ca9e575d29..f09b1fd53d 100644 --- a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift +++ b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift @@ -20,14 +20,15 @@ import Foundation import WebKit enum NewWindowPolicy { - case tab(selected: Bool, burner: Bool) + case tab(selected: Bool, burner: Bool, contextMenuInitiated: Bool = false) case popup(origin: NSPoint?, size: NSSize?) case window(active: Bool, burner: Bool) - init(_ windowFeatures: WKWindowFeatures, shouldSelectNewTab: Bool = false, isBurner: Bool) { + init(_ windowFeatures: WKWindowFeatures, shouldSelectNewTab: Bool = false, isBurner: Bool, contextMenuInitiated: Bool = false) { if windowFeatures.toolbarsVisibility?.boolValue == true { self = .tab(selected: shouldSelectNewTab, - burner: isBurner) + burner: isBurner, + contextMenuInitiated: contextMenuInitiated) } else if windowFeatures.width != nil { self = .popup(origin: windowFeatures.origin, size: windowFeatures.size) @@ -37,7 +38,7 @@ enum NewWindowPolicy { // See https://app.asana.com/0/1177771139624306/1205690527704551/f. if #available(macOS 14.1, *), windowFeatures.statusBarVisibility == nil && windowFeatures.menuBarVisibility == nil { - self = .tab(selected: shouldSelectNewTab, burner: isBurner) + self = .tab(selected: shouldSelectNewTab, burner: isBurner, contextMenuInitiated: contextMenuInitiated) } else { self = .window(active: true, burner: isBurner) @@ -49,10 +50,30 @@ enum NewWindowPolicy { return false } var isSelectedTab: Bool { - if case .tab(selected: true, burner: _) = self { return true } + if case .tab(selected: true, burner: _, contextMenuInitiated: _) = self { return true } return false } + /** + * Replaces `.tab` with `.window` when user prefers windows over tabs. + */ + func preferringTabsToWindows(_ prefersTabsToWindows: Bool) -> NewWindowPolicy { + guard case .tab(let isSelected, let isBurner, contextMenuInitiated: false) = self, !prefersTabsToWindows else { + return self + } + return .window(active: isSelected, burner: isBurner) + } + + /** + * Forces selecting a tab if `true` is passed as argument. + */ + func preferringSelectedTabs(_ prefersSelectedTabs: Bool) -> NewWindowPolicy { + guard case .tab(selected: false, burner: let isBurner, contextMenuInitiated: let contextMenuInitiated) = self, prefersSelectedTabs else { + return self + } + return .tab(selected: true, burner: isBurner, contextMenuInitiated: contextMenuInitiated) + } + } extension WKWindowFeatures { diff --git a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift index 964b04af1e..81b12ca0b0 100644 --- a/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift +++ b/DuckDuckGo/Tab/Model/Tab+UIDelegate.swift @@ -85,16 +85,12 @@ extension Tab: WKUIDelegate, PrintingUserScriptDelegate { windowFeatures: WKWindowFeatures, completionHandler: @escaping (WKWebView?) -> Void) { - switch newWindowPolicy(for: navigationAction) { + switch newWindowPolicy(for: navigationAction)?.preferringTabsToWindows(tabsPreferences.preferNewTabsToWindows) { // popup kind is known, action doesn‘t require Popup Permission - case .allow(.tab(selected: false, let burner)): - // update selected flag based on tab preferences - let targetKind = NewWindowPolicy.tab(selected: tabsPreferences.switchToNewTabWhenOpened, burner: burner) - completionHandler(self.createWebView(from: webView, with: configuration, for: navigationAction, of: targetKind)) - return case .allow(let targetKind): // proceed to web view creation - completionHandler(self.createWebView(from: webView, with: configuration, for: navigationAction, of: targetKind)) + completionHandler(self.createWebView(from: webView, with: configuration, + for: navigationAction, of: targetKind.preferringSelectedTabs(tabsPreferences.switchToNewTabWhenOpened))) return case .cancel: // navigation action was handled before and cancelled @@ -107,6 +103,7 @@ extension Tab: WKUIDelegate, PrintingUserScriptDelegate { let shouldSelectNewTab = !NSApp.isCommandPressed || tabsPreferences.switchToNewTabWhenOpened // this is actually not correct, to be fixed later // try to guess popup kind from provided windowFeatures let targetKind = NewWindowPolicy(windowFeatures, shouldSelectNewTab: shouldSelectNewTab, isBurner: burnerMode.isBurner) + .preferringTabsToWindows(tabsPreferences.preferNewTabsToWindows) // action doesn‘t require Popup Permission as it‘s user-initiated // TO BE FIXED: this also opens a new window when a popup ad is shown on click simultaneously with the main frame navigation: @@ -136,35 +133,20 @@ extension Tab: WKUIDelegate, PrintingUserScriptDelegate { } private func newWindowPolicy(for navigationAction: WKNavigationAction) -> NavigationDecision? { - func handleWithPreferences(newPolicy: NavigationDecision?) -> NavigationDecision? { - guard let newPolicy else { return nil } - - switch newPolicy { - case .allow(let targetKind): - if !tabsPreferences.preferNewTabsToWindows { - return .allow(.window(active: targetKind.isSelectedTab, burner: burnerMode.isBurner)) - } - case .cancel: - break - } - - return newPolicy - } - if let newWindowPolicy = self.decideNewWindowPolicy(for: navigationAction) { - return handleWithPreferences(newPolicy: newWindowPolicy) + return newWindowPolicy } // Are we handling custom Context Menu navigation action or link click with a hotkey? for handler in self.newWindowPolicyDecisionMakers ?? [] { guard let newWindowPolicy = handler.decideNewWindowPolicy(for: navigationAction) else { continue } - return handleWithPreferences(newPolicy: newWindowPolicy) + return newWindowPolicy } // allow popups opened from an empty window console let sourceUrl = navigationAction.safeSourceFrame?.safeRequest?.url ?? self.url ?? .empty if sourceUrl.isEmpty || sourceUrl.scheme == URL.NavigationalScheme.about.rawValue { - return handleWithPreferences(newPolicy: .allow(.tab(selected: true, burner: burnerMode.isBurner))) + return .allow(.tab(selected: true, burner: burnerMode.isBurner)) } return nil diff --git a/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift b/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift index 34a8172576..57174a6ec7 100644 --- a/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift +++ b/DuckDuckGo/Tab/TabExtensions/ContextMenuManager.swift @@ -24,6 +24,16 @@ import WebKit enum NavigationDecision { case allow(NewWindowPolicy) case cancel + + /** + * Replaces `.tab` with `.window` when user prefers windows over tabs. + */ + func preferringTabsToWindows(_ prefersTabsToWindows: Bool) -> NavigationDecision { + guard case .allow(let targetKind) = self, !prefersTabsToWindows else { + return self + } + return .allow(targetKind.preferringTabsToWindows(prefersTabsToWindows)) + } } @MainActor @@ -365,7 +375,7 @@ private extension ContextMenuManager { } onNewWindow = { [weak self] _ in - .allow(.tab(selected: self?.tabsPreferences.switchToNewTabWhenOpened ?? false, burner: burner)) + .allow(.tab(selected: self?.tabsPreferences.switchToNewTabWhenOpened ?? false, burner: burner, contextMenuInitiated: true)) } NSApp.sendAction(action, to: originalItem.target, from: originalItem) } diff --git a/DuckDuckGo/Tab/View/BrowserTabViewController.swift b/DuckDuckGo/Tab/View/BrowserTabViewController.swift index 327dc8975f..58f9c199bd 100644 --- a/DuckDuckGo/Tab/View/BrowserTabViewController.swift +++ b/DuckDuckGo/Tab/View/BrowserTabViewController.swift @@ -834,7 +834,7 @@ extension BrowserTabViewController: TabDelegate { case .window(active: let active, let isBurner): assert(isBurner == childTab.burnerMode.isBurner) WindowsManager.openNewWindow(with: childTab, showWindow: active) - case .tab(selected: let selected, _): + case .tab(selected: let selected, _, _): self.tabCollectionViewModel.insert(childTab, after: parentTab, selected: selected) } } From 2ab8caa439ce6df1146e02a0e921d8e24b1491af Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Wed, 24 Apr 2024 11:30:04 -0500 Subject: [PATCH 5/6] Fix selected window --- DuckDuckGo/Tab/Model/NewWindowPolicy.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift index f09b1fd53d..bfcb69f1f1 100644 --- a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift +++ b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift @@ -61,7 +61,7 @@ enum NewWindowPolicy { guard case .tab(let isSelected, let isBurner, contextMenuInitiated: false) = self, !prefersTabsToWindows else { return self } - return .window(active: isSelected, burner: isBurner) + return .window(active: true, burner: isBurner) } /** From e28bc854112a41da8460d56724f50ee7c1fb5b7e Mon Sep 17 00:00:00 2001 From: Brad Slayter Date: Wed, 24 Apr 2024 11:38:16 -0500 Subject: [PATCH 6/6] Discard unused value --- DuckDuckGo/Tab/Model/NewWindowPolicy.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift index bfcb69f1f1..80e237929e 100644 --- a/DuckDuckGo/Tab/Model/NewWindowPolicy.swift +++ b/DuckDuckGo/Tab/Model/NewWindowPolicy.swift @@ -58,7 +58,7 @@ enum NewWindowPolicy { * Replaces `.tab` with `.window` when user prefers windows over tabs. */ func preferringTabsToWindows(_ prefersTabsToWindows: Bool) -> NewWindowPolicy { - guard case .tab(let isSelected, let isBurner, contextMenuInitiated: false) = self, !prefersTabsToWindows else { + guard case .tab(_, let isBurner, contextMenuInitiated: false) = self, !prefersTabsToWindows else { return self } return .window(active: true, burner: isBurner)