From 675bbb7645570b337566b81cc99e75ac4e801e12 Mon Sep 17 00:00:00 2001 From: Pete Smith Date: Thu, 14 Mar 2024 11:24:10 +0000 Subject: [PATCH] Prevent Dismissal of Autofill UI When Window Loses Focus (#2393) Task/Issue URL: https://app.asana.com/0/276630244458377/1204870766898373/f Description: Currently, if we open the Autofill dialog and then tap out of the browser, the autofill window immediately dismisses. This is not a great UX. This change ensures that the Autofill dialog remains open until the user explicitly decides to close it. Note that the user can close the autofill dialog by selecting anywhere inside the DDG browser, but outside the Autofill dialog. --- DuckDuckGo.xcodeproj/project.pbxproj | 44 ++++++++ .../MainWindow/MainViewController.swift | 48 +++++++- DuckDuckGo/Menus/MainMenu.swift | 6 +- .../View/NavigationBarPopovers.swift | 70 +++++++----- .../View/NavigationBarViewController.swift | 32 +++--- .../View/NetPPopoverManagerMock.swift | 82 ++++++++++++++ .../NetworkProtectionNavBarButtonModel.swift | 25 +---- ...etworkProtectionNavBarPopoverManager.swift | 27 ++++- ...NetworkProtectionIPCTunnelController.swift | 6 +- .../View/AutofillPopoverPresenter.swift | 84 ++++++++++++++ .../View/PasswordManagementPopover.swift | 19 ---- DuckDuckGo/Windows/View/WindowsManager.swift | 11 +- .../Geolocation/CLLocationManagerMock.swift | 17 +++ .../Geolocation/GeolocationServiceTests.swift | 14 ++- .../Mocks/MockAutofillPopoverPresenter.swift | 53 +++++++++ .../View/NavigationBarPopoversTests.swift | 104 ++++++++++++++++++ .../AutofillPreferencesModelTests.swift | 20 +++- 17 files changed, 555 insertions(+), 107 deletions(-) create mode 100644 DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift create mode 100644 DuckDuckGo/SecureVault/View/AutofillPopoverPresenter.swift create mode 100644 UnitTests/NavigationBar/Mocks/MockAutofillPopoverPresenter.swift create mode 100644 UnitTests/NavigationBar/View/NavigationBarPopoversTests.swift diff --git a/DuckDuckGo.xcodeproj/project.pbxproj b/DuckDuckGo.xcodeproj/project.pbxproj index 1ebb57cfa5..90fc764ac7 100644 --- a/DuckDuckGo.xcodeproj/project.pbxproj +++ b/DuckDuckGo.xcodeproj/project.pbxproj @@ -2581,6 +2581,9 @@ B31055C427A1BA1D001AC618 /* AutoconsentUserScript.swift in Sources */ = {isa = PBXBuildFile; fileRef = B31055BC27A1BA1D001AC618 /* AutoconsentUserScript.swift */; }; B31055C627A1BA1D001AC618 /* userscript.js in Resources */ = {isa = PBXBuildFile; fileRef = B31055BE27A1BA1D001AC618 /* userscript.js */; }; B31055CB27A1BA1D001AC618 /* autoconsent-bundle.js in Resources */ = {isa = PBXBuildFile; fileRef = B31055C327A1BA1D001AC618 /* autoconsent-bundle.js */; }; + B60293E62BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B60293E52BA19ECD0033186B /* NetPPopoverManagerMock.swift */; }; + B60293E72BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B60293E52BA19ECD0033186B /* NetPPopoverManagerMock.swift */; }; + B60293E82BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */ = {isa = PBXBuildFile; fileRef = B60293E52BA19ECD0033186B /* NetPPopoverManagerMock.swift */; }; B602E7CF2A93A5FF00F12201 /* WKBackForwardListExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B602E7CE2A93A5FF00F12201 /* WKBackForwardListExtension.swift */; }; B602E7D02A93A5FF00F12201 /* WKBackForwardListExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = B602E7CE2A93A5FF00F12201 /* WKBackForwardListExtension.swift */; }; B602E8162A1E2570006D261F /* URL+NetworkProtection.swift in Sources */ = {isa = PBXBuildFile; fileRef = B602E8152A1E2570006D261F /* URL+NetworkProtection.swift */; }; @@ -3075,6 +3078,13 @@ C168B9AC2B31DC7E001AFAD9 /* AutofillNeverPromptWebsitesManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = C168B9AB2B31DC7E001AFAD9 /* AutofillNeverPromptWebsitesManager.swift */; }; C168B9AD2B31DC7F001AFAD9 /* AutofillNeverPromptWebsitesManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = C168B9AB2B31DC7E001AFAD9 /* AutofillNeverPromptWebsitesManager.swift */; }; C168B9AE2B31DC7F001AFAD9 /* AutofillNeverPromptWebsitesManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = C168B9AB2B31DC7E001AFAD9 /* AutofillNeverPromptWebsitesManager.swift */; }; + C17CA7AD2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C17CA7AC2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift */; }; + C17CA7AE2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C17CA7AC2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift */; }; + C17CA7B22B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C17CA7B12B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift */; }; + C17CA7B32B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C17CA7B12B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift */; }; + C1DAF3B52B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1DAF3B42B9A44860059244F /* AutofillPopoverPresenter.swift */; }; + C1DAF3B62B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1DAF3B42B9A44860059244F /* AutofillPopoverPresenter.swift */; }; + C1DAF3B72B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1DAF3B42B9A44860059244F /* AutofillPopoverPresenter.swift */; }; C1E961EB2B879E79001760E1 /* MockAutofillActionPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1E961E72B879E4D001760E1 /* MockAutofillActionPresenter.swift */; }; C1E961ED2B879ED9001760E1 /* MockAutofillActionExecutor.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1E961EC2B879ED9001760E1 /* MockAutofillActionExecutor.swift */; }; C1E961EF2B87AA29001760E1 /* AutofillActionBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = C1E961EE2B87AA29001760E1 /* AutofillActionBuilder.swift */; }; @@ -4161,6 +4171,7 @@ B31055BC27A1BA1D001AC618 /* AutoconsentUserScript.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AutoconsentUserScript.swift; sourceTree = ""; }; B31055BE27A1BA1D001AC618 /* userscript.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = userscript.js; sourceTree = ""; }; B31055C327A1BA1D001AC618 /* autoconsent-bundle.js */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.javascript; path = "autoconsent-bundle.js"; sourceTree = ""; }; + B60293E52BA19ECD0033186B /* NetPPopoverManagerMock.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetPPopoverManagerMock.swift; sourceTree = ""; }; B602E7CE2A93A5FF00F12201 /* WKBackForwardListExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WKBackForwardListExtension.swift; sourceTree = ""; }; B602E8152A1E2570006D261F /* URL+NetworkProtection.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "URL+NetworkProtection.swift"; sourceTree = ""; }; B602E81C2A1E25B0006D261F /* NEOnDemandRuleExtension.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NEOnDemandRuleExtension.swift; sourceTree = ""; }; @@ -4460,6 +4471,9 @@ C13909F32B85FD79001626ED /* AutofillDeleteAllPasswordsExecutorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillDeleteAllPasswordsExecutorTests.swift; sourceTree = ""; }; C13909FA2B861039001626ED /* AutofillActionPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillActionPresenter.swift; sourceTree = ""; }; C168B9AB2B31DC7E001AFAD9 /* AutofillNeverPromptWebsitesManager.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AutofillNeverPromptWebsitesManager.swift; sourceTree = ""; }; + C17CA7AC2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NavigationBarPopoversTests.swift; sourceTree = ""; }; + C17CA7B12B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockAutofillPopoverPresenter.swift; sourceTree = ""; }; + C1DAF3B42B9A44860059244F /* AutofillPopoverPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillPopoverPresenter.swift; sourceTree = ""; }; C1E961E72B879E4D001760E1 /* MockAutofillActionPresenter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockAutofillActionPresenter.swift; sourceTree = ""; }; C1E961EC2B879ED9001760E1 /* MockAutofillActionExecutor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MockAutofillActionExecutor.swift; sourceTree = ""; }; C1E961EE2B87AA29001760E1 /* AutofillActionBuilder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutofillActionBuilder.swift; sourceTree = ""; }; @@ -6539,6 +6553,7 @@ 4B8A4DFE27C83B29005F40E8 /* SaveIdentityViewController.swift */, 4BE4005227CF3DC3007D3161 /* SavePaymentMethodPopover.swift */, 4BE4005427CF3F19007D3161 /* SavePaymentMethodViewController.swift */, + C1DAF3B42B9A44860059244F /* AutofillPopoverPresenter.swift */, ); path = View; sourceTree = ""; @@ -7220,6 +7235,7 @@ AAA0CC32252F181A0079BC96 /* NavigationButtonMenuDelegate.swift */, 85012B0129133F9F003D0DCC /* NavigationBarPopovers.swift */, AA68C3D22490ED62001B8783 /* NavigationBarViewController.swift */, + B60293E52BA19ECD0033186B /* NetPPopoverManagerMock.swift */, D64A5FF72AEA5C2B00B6D6E7 /* HomeButtonMenuFactory.swift */, ); path = View; @@ -7292,9 +7308,11 @@ AA91F83627076ED100771A0D /* NavigationBar */ = { isa = PBXGroup; children = ( + C17CA7B02B9B52FF008EC3C1 /* Mocks */, AA91F83727076EEE00771A0D /* ViewModel */, 4BF6961F28BEEE8B00D402D4 /* LocalPinningManagerTests.swift */, 56B234BE2A84EFD200F2A1CC /* NavigationBarUrlExtensionsTests.swift */, + C17CA7AF2B9B52EB008EC3C1 /* View */, ); path = NavigationBar; sourceTree = ""; @@ -8300,6 +8318,22 @@ path = Tests; sourceTree = ""; }; + C17CA7AF2B9B52EB008EC3C1 /* View */ = { + isa = PBXGroup; + children = ( + C17CA7AC2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift */, + ); + path = View; + sourceTree = ""; + }; + C17CA7B02B9B52FF008EC3C1 /* Mocks */ = { + isa = PBXGroup; + children = ( + C17CA7B12B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift */, + ); + path = Mocks; + sourceTree = ""; + }; C1E961E62B879E2A001760E1 /* Mocks */ = { isa = PBXGroup; children = ( @@ -10252,6 +10286,7 @@ 4B9DB0452A983B24000927DB /* WaitlistModalViewController.swift in Sources */, B66CA41F2AD910B300447CF0 /* DataImportView.swift in Sources */, 3706FC0C293F65D500E42796 /* NSAttributedStringExtension.swift in Sources */, + C1DAF3B62B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */, 3706FC0D293F65D500E42796 /* AnimationView.swift in Sources */, 3706FC0E293F65D500E42796 /* NSRectExtension.swift in Sources */, 3706FC0F293F65D500E42796 /* YoutubeOverlayUserScript.swift in Sources */, @@ -10338,6 +10373,7 @@ 3706FC51293F65D500E42796 /* RecentlyVisitedView.swift in Sources */, B645D8F729FA95440024461F /* WKProcessPoolExtension.swift in Sources */, 3706FC52293F65D500E42796 /* MouseOverAnimationButton.swift in Sources */, + B60293E72BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */, 3706FC53293F65D500E42796 /* TabBarScrollView.swift in Sources */, 3706FC54293F65D500E42796 /* BookmarkListTreeControllerDataSource.swift in Sources */, 3706FC55293F65D500E42796 /* AddressBarViewController.swift in Sources */, @@ -10541,6 +10577,7 @@ B6619EF72B10DFF700CD9186 /* InstructionsFormatParserTests.swift in Sources */, 3706FE21293F661700E42796 /* DownloadsPreferencesTests.swift in Sources */, 3706FE22293F661700E42796 /* FireproofDomainsTests.swift in Sources */, + C17CA7AE2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift in Sources */, 3706FE23293F661700E42796 /* SuggestionLoadingMock.swift in Sources */, 3706FE24293F661700E42796 /* PasteboardFolderTests.swift in Sources */, B603971229B9D67E00902A34 /* PublishersExtensions.swift in Sources */, @@ -10578,6 +10615,7 @@ 3706FE3C293F661700E42796 /* FireproofDomainsStoreMock.swift in Sources */, 3706FE3D293F661700E42796 /* DataEncryptionTests.swift in Sources */, 3706FE3E293F661700E42796 /* ClickToLoadModelTests.swift in Sources */, + C17CA7B32B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift in Sources */, 3706FE3F293F661700E42796 /* FileStoreMock.swift in Sources */, B6619F042B17123200CD9186 /* DataImportViewModelTests.swift in Sources */, 1D8C2FE62B70F4C4005E4BBD /* TabSnapshotExtensionTests.swift in Sources */, @@ -11169,6 +11207,7 @@ 4B957A3E2AC7AE700062CA31 /* EnableWaitlistFeatureView.swift in Sources */, 4B957A3F2AC7AE700062CA31 /* GrammarFeaturesManager.swift in Sources */, 4B957A402AC7AE700062CA31 /* WaitlistModalViewController.swift in Sources */, + B60293E82BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */, B6BCC53E2AFD15DF002C5499 /* DataImportProfilePicker.swift in Sources */, 4B957A412AC7AE700062CA31 /* WKMenuItemIdentifier.swift in Sources */, 4B957A422AC7AE700062CA31 /* SafariFaviconsReader.swift in Sources */, @@ -11523,6 +11562,7 @@ 4B957B7B2AC7AE700062CA31 /* DeviceAuthenticator.swift in Sources */, 4B957B7C2AC7AE700062CA31 /* NetworkProtectionWaitlistFeatureFlagOverridesMenu.swift in Sources */, 4B957B7D2AC7AE700062CA31 /* TabBarCollectionView.swift in Sources */, + C1DAF3B72B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */, 4B957B7E2AC7AE700062CA31 /* NetworkProtection+ConvenienceInitializers.swift in Sources */, 7BA7CC502AD11F6F0042E5CE /* NetworkProtectionIPCTunnelController.swift in Sources */, 4B957B7F2AC7AE700062CA31 /* NavigationActionExtension.swift in Sources */, @@ -11955,6 +11995,7 @@ 4B9DB0442A983B24000927DB /* WaitlistModalViewController.swift in Sources */, B6DA06E8291401D700225DE2 /* WKMenuItemIdentifier.swift in Sources */, 4B0AACAE28BC6FD0001038AC /* SafariFaviconsReader.swift in Sources */, + B60293E62BA19ECD0033186B /* NetPPopoverManagerMock.swift in Sources */, B6B3E0E12657EA7A0040E0A2 /* NSScreenExtension.swift in Sources */, B65E6BA026D9F10600095F96 /* NSBezierPathExtension.swift in Sources */, 4B4D60E02A0C875F00BCD287 /* Bundle+VPN.swift in Sources */, @@ -12309,6 +12350,7 @@ 4B9292AF26670F5300AD2C21 /* NSOutlineViewExtensions.swift in Sources */, AA585D82248FD31100E9A3E2 /* AppDelegate.swift in Sources */, 7B1E81A027C8874900FF0E60 /* ContentOverlayViewController.swift in Sources */, + C1DAF3B52B9A44860059244F /* AutofillPopoverPresenter.swift in Sources */, B687B7CA2947A029001DEA6F /* ContentBlockingTabExtension.swift in Sources */, 85B7184C27677C6500B4277F /* OnboardingViewController.swift in Sources */, 4B379C1E27BDB7FF008A968E /* DeviceAuthenticator.swift in Sources */, @@ -12431,6 +12473,7 @@ 569277C429DEE09D00B633EF /* ContinueSetUpModelTests.swift in Sources */, 85F1B0C925EF9759004792B6 /* URLEventHandlerTests.swift in Sources */, 4B9292BD2667103100AD2C21 /* BookmarkOutlineViewDataSourceTests.swift in Sources */, + C17CA7B22B9B5317008EC3C1 /* MockAutofillPopoverPresenter.swift in Sources */, 4BF6961D28BE911100D402D4 /* RecentlyVisitedSiteModelTests.swift in Sources */, B6619F062B17138D00CD9186 /* DataImportSourceViewModelTests.swift in Sources */, 4BBF0917282DD6EF00EE1418 /* TemporaryFileHandlerTests.swift in Sources */, @@ -12642,6 +12685,7 @@ B6C2C9EF276081AB005B7F0A /* DeallocationTests.swift in Sources */, B63ED0D826AE729600A9DAD1 /* PermissionModelTests.swift in Sources */, B69B504B2726CA2900758A2B /* MockStatisticsStore.swift in Sources */, + C17CA7AD2B9B52E6008EC3C1 /* NavigationBarPopoversTests.swift in Sources */, 37CD54BD27F2ECAE00F1F7B9 /* AutofillPreferencesModelTests.swift in Sources */, C1E961ED2B879ED9001760E1 /* MockAutofillActionExecutor.swift in Sources */, 37D23789288009CF00BCE03B /* TabCollectionViewModelTests+PinnedTabs.swift in Sources */, diff --git a/DuckDuckGo/MainWindow/MainViewController.swift b/DuckDuckGo/MainWindow/MainViewController.swift index 5741f2f55e..5ef25c47e9 100644 --- a/DuckDuckGo/MainWindow/MainViewController.swift +++ b/DuckDuckGo/MainWindow/MainViewController.swift @@ -23,6 +23,7 @@ import Common #if NETWORK_PROTECTION import NetworkProtection +import NetworkProtectionIPC #endif final class MainViewController: NSViewController { @@ -56,14 +57,53 @@ final class MainViewController: NSViewController { fatalError("MainViewController: Bad initializer") } - init(tabCollectionViewModel: TabCollectionViewModel? = nil, - bookmarkManager: BookmarkManager = LocalBookmarkManager.shared) { + init(tabCollectionViewModel: TabCollectionViewModel? = nil, bookmarkManager: BookmarkManager = LocalBookmarkManager.shared, autofillPopoverPresenter: AutofillPopoverPresenter) { let tabCollectionViewModel = tabCollectionViewModel ?? TabCollectionViewModel() self.tabCollectionViewModel = tabCollectionViewModel self.isBurner = tabCollectionViewModel.isBurner tabBarViewController = TabBarViewController.create(tabCollectionViewModel: tabCollectionViewModel) - navigationBarViewController = NavigationBarViewController.create(tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner) + +#if NETWORK_PROTECTION + let networkProtectionPopoverManager: NetPPopoverManager = { +#if DEBUG + guard case .normal = NSApp.runType else { + return NetPPopoverManagerMock() + } +#endif + let vpnBundleID = Bundle.main.vpnMenuAgentBundleId + let ipcClient = TunnelControllerIPCClient(machServiceName: vpnBundleID) + ipcClient.register() + + return NetworkProtectionNavBarPopoverManager(ipcClient: ipcClient) + }() + let networkProtectionStatusReporter: NetworkProtectionStatusReporter = { + var connectivityIssuesObserver: ConnectivityIssueObserver! + var controllerErrorMessageObserver: ControllerErrorMesssageObserver! +#if DEBUG + if ![.normal, .integrationTests].contains(NSApp.runType) { + connectivityIssuesObserver = ConnectivityIssueObserverMock() + controllerErrorMessageObserver = ControllerErrorMesssageObserverMock() + } +#endif + connectivityIssuesObserver = connectivityIssuesObserver ?? DisabledConnectivityIssueObserver() + controllerErrorMessageObserver = controllerErrorMessageObserver ?? ControllerErrorMesssageObserverThroughDistributedNotifications() + + let ipcClient = networkProtectionPopoverManager.ipcClient + return DefaultNetworkProtectionStatusReporter( + statusObserver: ipcClient.ipcStatusObserver, + serverInfoObserver: ipcClient.ipcServerInfoObserver, + connectionErrorObserver: ipcClient.ipcConnectionErrorObserver, + connectivityIssuesObserver: connectivityIssuesObserver, + controllerErrorMessageObserver: controllerErrorMessageObserver + ) + }() + + navigationBarViewController = NavigationBarViewController.create(tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, networkProtectionPopoverManager: networkProtectionPopoverManager, networkProtectionStatusReporter: networkProtectionStatusReporter, autofillPopoverPresenter: autofillPopoverPresenter) +#else + navigationBarViewController = NavigationBarViewController.create(tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, autofillPopoverPresenter: AutofillPopoverPresenter) +#endif + browserTabViewController = BrowserTabViewController(tabCollectionViewModel: tabCollectionViewModel, bookmarkManager: bookmarkManager) findInPageViewController = FindInPageViewController.create() fireViewController = FireViewController.create(tabCollectionViewModel: tabCollectionViewModel) @@ -560,7 +600,7 @@ extension MainViewController { ])) bkman.loadBookmarks() - let vc = MainViewController(bookmarkManager: bkman) + let vc = MainViewController(bookmarkManager: bkman, autofillPopoverPresenter: DefaultAutofillPopoverPresenter()) var c: AnyCancellable! c = vc.publisher(for: \.view.window).sink { window in window?.titlebarAppearsTransparent = true diff --git a/DuckDuckGo/Menus/MainMenu.swift b/DuckDuckGo/Menus/MainMenu.swift index dac124ab05..d6d7ac227a 100644 --- a/DuckDuckGo/Menus/MainMenu.swift +++ b/DuckDuckGo/Menus/MainMenu.swift @@ -609,8 +609,10 @@ import SubscriptionUI #endif #if NETWORK_PROTECTION - NSMenuItem(title: "VPN") - .submenu(NetworkProtectionDebugMenu()) + if case .normal = NSApp.runType { + NSMenuItem(title: "VPN") + .submenu(NetworkProtectionDebugMenu()) + } #endif NSMenuItem(title: "Trigger Fatal Error", action: #selector(MainViewController.triggerFatalError)) diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift b/DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift index 593fb76fbf..b8607c8f85 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarPopovers.swift @@ -27,7 +27,30 @@ import NetworkProtectionUI import NetworkProtectionIPC #endif -final class NavigationBarPopovers { +protocol PopoverPresenter { + func show(_ popover: NSPopover, positionedBelow view: NSView) +} + +#if NETWORK_PROTECTION +protocol NetPPopoverManager: AnyObject { + var ipcClient: NetworkProtectionIPCClient { get } + var isShown: Bool { get } + + func show(positionedBelow view: NSView, withDelegate delegate: NSPopoverDelegate) + func close() + + func toggle(positionedBelow view: NSView, withDelegate delegate: NSPopoverDelegate) +} +#endif + +extension PopoverPresenter { + func show(_ popover: NSPopover, positionedBelow view: NSView) { + view.isHidden = false + popover.show(positionedBelow: view.bounds.insetFromLineOfDeath(flipped: view.isFlipped), in: view) + } +} + +final class NavigationBarPopovers: PopoverPresenter { enum Constants { static let downloadsPopoverAutoHidingInterval: TimeInterval = 10 @@ -37,20 +60,26 @@ final class NavigationBarPopovers { private(set) var saveCredentialsPopover: SaveCredentialsPopover? private(set) var saveIdentityPopover: SaveIdentityPopover? private(set) var savePaymentMethodPopover: SavePaymentMethodPopover? - private(set) var passwordManagementPopover: PasswordManagementPopover? + private(set) var autofillPopoverPresenter: AutofillPopoverPresenter private(set) var downloadsPopover: DownloadsPopover? #if NETWORK_PROTECTION - private let networkProtectionPopoverManager: NetworkProtectionNavBarPopoverManager + private let networkProtectionPopoverManager: NetPPopoverManager - init(networkProtectionPopoverManager: NetworkProtectionNavBarPopoverManager) { + init(networkProtectionPopoverManager: NetPPopoverManager, autofillPopoverPresenter: AutofillPopoverPresenter) { self.networkProtectionPopoverManager = networkProtectionPopoverManager + self.autofillPopoverPresenter = autofillPopoverPresenter + } + +#else + init(passwordPopoverPresenter: PasswordPopoverPresenter) { + self.passwordPopoverPresenter = passwordPopoverPresenter } #endif var passwordManagementDomain: String? { didSet { - passwordManagementPopover?.viewController.domain = passwordManagementDomain + autofillPopoverPresenter.passwordDomain = passwordManagementDomain } } @@ -63,11 +92,11 @@ final class NavigationBarPopovers { } var isPasswordManagementDirty: Bool { - passwordManagementPopover?.viewController.isDirty ?? false + autofillPopoverPresenter.popoverIsDirty } var isPasswordManagementPopoverShown: Bool { - passwordManagementPopover?.isShown ?? false + autofillPopoverPresenter.popoverIsShown } @MainActor @@ -92,8 +121,8 @@ final class NavigationBarPopovers { } func passwordManagementButtonPressed(usingView view: NSView, withDelegate delegate: NSPopoverDelegate) { - if passwordManagementPopover?.isShown == true { - passwordManagementPopover?.close() + if autofillPopoverPresenter.popoverIsShown == true && view.window == autofillPopoverPresenter.popoverPresentingWindow { + autofillPopoverPresenter.dismiss() } else { showPasswordManagementPopover(selectedCategory: nil, usingView: view, withDelegate: delegate) } @@ -152,8 +181,8 @@ final class NavigationBarPopovers { bookmarkListPopover?.close() } - if passwordManagementPopover?.isShown ?? false { - passwordManagementPopover?.close() + if autofillPopoverPresenter.popoverIsShown { + autofillPopoverPresenter.dismiss() } if downloadsPopover?.isShown ?? false { @@ -187,20 +216,11 @@ final class NavigationBarPopovers { func showPasswordManagementPopover(selectedCategory: SecureVaultSorting.Category?, usingView view: NSView, withDelegate delegate: NSPopoverDelegate) { guard closeTransientPopovers() else { return } - let popover = passwordManagementPopover ?? PasswordManagementPopover() - passwordManagementPopover = popover - popover.viewController.domain = passwordManagementDomain - popover.delegate = delegate - show(popover, positionedBelow: view) - popover.select(category: selectedCategory) + autofillPopoverPresenter.show(positionedBelow: view, withDomain: passwordManagementDomain, selectedCategory: selectedCategory) } func showPasswordManagerPopover(selectedWebsiteAccount: SecureVaultModels.WebsiteAccount, usingView view: NSView, withDelegate delegate: NSPopoverDelegate) { - let popover = passwordManagementPopover ?? PasswordManagementPopover() - passwordManagementPopover = popover - popover.delegate = delegate - show(popover, positionedBelow: view) - popover.select(websiteAccount: selectedWebsiteAccount) + autofillPopoverPresenter.show(positionedBelow: view, withSelectedAccount: selectedWebsiteAccount) } func hasAnySavePopoversVisible() -> Bool { @@ -236,10 +256,6 @@ final class NavigationBarPopovers { bookmarkListPopover = nil } - func passwordManagementPopoverClosed() { - passwordManagementPopover = nil - } - func saveIdentityPopoverClosed() { saveIdentityPopover = nil } @@ -273,7 +289,7 @@ final class NavigationBarPopovers { show(popover, positionedBelow: view) } - private func show(_ popover: NSPopover, positionedBelow view: NSView) { + func show(_ popover: NSPopover, positionedBelow view: NSView) { view.isHidden = false popover.show(positionedBelow: view.bounds.insetFromLineOfDeath(flipped: view.isFlipped), in: view) diff --git a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift index b45ee536a9..aa050d3a98 100644 --- a/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift +++ b/DuckDuckGo/NavigationBar/View/NavigationBarViewController.swift @@ -113,23 +113,22 @@ final class NavigationBarViewController: NSViewController { #endif #if NETWORK_PROTECTION - static func create(tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, networkProtectionFeatureActivation: NetworkProtectionFeatureActivation = NetworkProtectionKeychainTokenStore(), downloadListCoordinator: DownloadListCoordinator = .shared) -> NavigationBarViewController { + static func create(tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, + networkProtectionFeatureActivation: NetworkProtectionFeatureActivation = NetworkProtectionKeychainTokenStore(), + downloadListCoordinator: DownloadListCoordinator = .shared, + networkProtectionPopoverManager: NetPPopoverManager, + networkProtectionStatusReporter: NetworkProtectionStatusReporter, + autofillPopoverPresenter: AutofillPopoverPresenter) -> NavigationBarViewController { NSStoryboard(name: "NavigationBar", bundle: nil).instantiateInitialController { coder in - self.init(coder: coder, tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, networkProtectionFeatureActivation: networkProtectionFeatureActivation, downloadListCoordinator: downloadListCoordinator) + self.init(coder: coder, tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, networkProtectionFeatureActivation: networkProtectionFeatureActivation, downloadListCoordinator: downloadListCoordinator, networkProtectionPopoverManager: networkProtectionPopoverManager, networkProtectionStatusReporter: networkProtectionStatusReporter, autofillPopoverPresenter: autofillPopoverPresenter) }! } - init?(coder: NSCoder, tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, networkProtectionFeatureActivation: NetworkProtectionFeatureActivation, downloadListCoordinator: DownloadListCoordinator) { + init?(coder: NSCoder, tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, networkProtectionFeatureActivation: NetworkProtectionFeatureActivation, downloadListCoordinator: DownloadListCoordinator, networkProtectionPopoverManager: NetPPopoverManager, networkProtectionStatusReporter: NetworkProtectionStatusReporter, autofillPopoverPresenter: AutofillPopoverPresenter) { - let vpnBundleID = Bundle.main.vpnMenuAgentBundleId - let ipcClient = TunnelControllerIPCClient(machServiceName: vpnBundleID) - ipcClient.register() - - let networkProtectionPopoverManager = NetworkProtectionNavBarPopoverManager(ipcClient: ipcClient) - - self.popovers = NavigationBarPopovers(networkProtectionPopoverManager: networkProtectionPopoverManager) + self.popovers = NavigationBarPopovers(networkProtectionPopoverManager: networkProtectionPopoverManager, autofillPopoverPresenter: autofillPopoverPresenter) self.tabCollectionViewModel = tabCollectionViewModel - self.networkProtectionButtonModel = NetworkProtectionNavBarButtonModel(popoverManager: networkProtectionPopoverManager) + self.networkProtectionButtonModel = NetworkProtectionNavBarButtonModel(popoverManager: networkProtectionPopoverManager, statusReporter: networkProtectionStatusReporter) self.isBurner = isBurner self.networkProtectionFeatureActivation = networkProtectionFeatureActivation self.downloadListCoordinator = downloadListCoordinator @@ -138,14 +137,15 @@ final class NavigationBarViewController: NSViewController { super.init(coder: coder) } #else - static func create(tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, downloadListCoordinator: DownloadListCoordinator = .shared) -> NavigationBarViewController { + static func create(tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, downloadListCoordinator: DownloadListCoordinator = .shared, autofillPopoverPresenter: AutofillPopoverPresenter) -> NavigationBarViewController { NSStoryboard(name: "NavigationBar", bundle: nil).instantiateInitialController { coder in - self.init(coder: coder, tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, downloadListCoordinator: downloadListCoordinator) + self.init(coder: coder, tabCollectionViewModel: tabCollectionViewModel, isBurner: isBurner, downloadListCoordinator: downloadListCoordinator, passwordPopoverPresenter: passwordPopoverPresenter) }! } - init?(coder: NSCoder, tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, downloadListCoordinator: DownloadListCoordinator) { - self.popovers = NavigationBarPopovers() + init?(coder: NSCoder, tabCollectionViewModel: TabCollectionViewModel, isBurner: Bool, downloadListCoordinator: DownloadListCoordinator, + autofillPopoverPresenter: AutofillPopoverPresenter) { + self.popovers = NavigationBarPopovers(autofillPopoverPresenter: autofillPopoverPresenter) self.tabCollectionViewModel = tabCollectionViewModel self.isBurner = isBurner self.downloadListCoordinator = downloadListCoordinator @@ -1109,8 +1109,6 @@ extension NavigationBarViewController: NSPopoverDelegate { } else if let popover = popovers.bookmarkListPopover, notification.object as AnyObject? === popover { popovers.bookmarkListPopoverClosed() updateBookmarksButton() - } else if let popover = popovers.passwordManagementPopover, notification.object as AnyObject? === popover { - popovers.passwordManagementPopoverClosed() } else if let popover = popovers.saveIdentityPopover, notification.object as AnyObject? === popover { popovers.saveIdentityPopoverClosed() updatePasswordManagementButton() diff --git a/DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift b/DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift new file mode 100644 index 0000000000..ef82b84da6 --- /dev/null +++ b/DuckDuckGo/NavigationBar/View/NetPPopoverManagerMock.swift @@ -0,0 +1,82 @@ +// +// NetPPopoverManagerMock.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#if DEBUG && NETWORK_PROTECTION + +import Combine +import Foundation +import NetworkProtection + +final class NetPPopoverManagerMock: NetPPopoverManager { + var isShown: Bool { false } + var ipcClient: NetworkProtectionIPCClient = IPCClientMock() + + func toggle(positionedBelow view: NSView, withDelegate delegate: NSPopoverDelegate) {} + func show(positionedBelow view: NSView, withDelegate delegate: any NSPopoverDelegate) {} + func close() {} +} + +final class IPCClientMock: NetworkProtectionIPCClient { + + final class ConnectionStatusObserverMock: NetworkProtection.ConnectionStatusObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: NetworkProtection.ConnectionStatus = .notConfigured + } + var ipcStatusObserver: any NetworkProtection.ConnectionStatusObserver = ConnectionStatusObserverMock() + + final class ConnectionServerInfoObserverMock: NetworkProtection.ConnectionServerInfoObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: NetworkProtection.NetworkProtectionStatusServerInfo = .unknown + } + var ipcServerInfoObserver: any NetworkProtection.ConnectionServerInfoObserver = ConnectionServerInfoObserverMock() + + final class ConnectionErrorObserverMock: NetworkProtection.ConnectionErrorObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: String? + } + var ipcConnectionErrorObserver: any NetworkProtection.ConnectionErrorObserver = ConnectionErrorObserverMock() + + final class ConnectivityIssueObserverMock: NetworkProtection.ConnectivityIssueObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: Bool = false + } + var ipcConnectivityIssuesObserver: any NetworkProtection.ConnectivityIssueObserver = ConnectivityIssueObserverMock() + + final class ControllerErrorMesssageObserverMock: NetworkProtection.ControllerErrorMesssageObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: String? + } + var ipcControllerErrorMessageObserver: any NetworkProtection.ControllerErrorMesssageObserver = ControllerErrorMesssageObserverMock() + + func start() {} + + func stop() {} + +} + +final class ConnectivityIssueObserverMock: ConnectivityIssueObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue = false +} + +final class ControllerErrorMesssageObserverMock: ControllerErrorMesssageObserver { + var publisher: AnyPublisher = PassthroughSubject().eraseToAnyPublisher() + var recentValue: String? +} + +#endif diff --git a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift index 32c25df9b5..c756d18ac6 100644 --- a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift +++ b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarButtonModel.swift @@ -31,15 +31,9 @@ final class NetworkProtectionNavBarButtonModel: NSObject, ObservableObject { private let networkProtectionStatusReporter: NetworkProtectionStatusReporter private var status: NetworkProtection.ConnectionStatus = .default - private let popoverManager: NetworkProtectionNavBarPopoverManager + private let popoverManager: NetPPopoverManager private let waitlistActivationDateStore: DefaultWaitlistActivationDateStore - // MARK: - IPC - - public var ipcClient: TunnelControllerIPCClient { - popoverManager.ipcClient - } - // MARK: - Subscriptions private var cancellables = Set() @@ -76,23 +70,13 @@ final class NetworkProtectionNavBarButtonModel: NSObject, ObservableObject { // MARK: - Initialization - init(popoverManager: NetworkProtectionNavBarPopoverManager, + init(popoverManager: NetPPopoverManager, pinningManager: PinningManager = LocalPinningManager.shared, - statusReporter: NetworkProtectionStatusReporter? = nil, + statusReporter: NetworkProtectionStatusReporter, iconProvider: IconProvider = NavigationBarIconProvider()) { self.popoverManager = popoverManager - - let ipcClient = popoverManager.ipcClient - self.networkProtectionStatusReporter = statusReporter - ?? DefaultNetworkProtectionStatusReporter( - statusObserver: ipcClient.connectionStatusObserver, - serverInfoObserver: ipcClient.serverInfoObserver, - connectionErrorObserver: ipcClient.connectionErrorObserver, - connectivityIssuesObserver: DisabledConnectivityIssueObserver(), - controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications() - ) self.iconPublisher = NetworkProtectionIconPublisher(statusReporter: networkProtectionStatusReporter, iconProvider: iconProvider) self.pinningManager = pinningManager self.shortcutTitle = pinningManager.shortcutTitle(for: .networkProtection) @@ -125,6 +109,9 @@ final class NetworkProtectionNavBarButtonModel: NSObject, ObservableObject { /// This will be removed once the waitlist beta has ended. private func buttonImageFromWaitlistState(icon: NetworkProtectionAsset?) -> NSImage { let icon = icon ?? iconPublisher.icon +#if DEBUG + guard [.normal, .integrationTests].contains(NSApp.runType) else { return NSImage() } +#endif let isWaitlistUser = NetworkProtectionWaitlist().waitlistStorage.isWaitlistUser let hasAuthToken = NetworkProtectionKeychainTokenStore().isFeatureActivated diff --git a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift index 1d5a18c48c..16853163e8 100644 --- a/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift +++ b/DuckDuckGo/NetworkProtection/AppTargets/BothAppTargets/NetworkProtectionNavBarPopoverManager.swift @@ -25,11 +25,26 @@ import NetworkProtectionIPC import NetworkProtectionUI #if NETWORK_PROTECTION -final class NetworkProtectionNavBarPopoverManager { + +protocol NetworkProtectionIPCClient { + var ipcStatusObserver: ConnectionStatusObserver { get } + var ipcServerInfoObserver: ConnectionServerInfoObserver { get } + var ipcConnectionErrorObserver: ConnectionErrorObserver { get } + + func start() + func stop() +} +extension TunnelControllerIPCClient: NetworkProtectionIPCClient { + public var ipcStatusObserver: any NetworkProtection.ConnectionStatusObserver { connectionStatusObserver } + public var ipcServerInfoObserver: any NetworkProtection.ConnectionServerInfoObserver { serverInfoObserver } + public var ipcConnectionErrorObserver: any NetworkProtection.ConnectionErrorObserver { connectionErrorObserver } +} + +final class NetworkProtectionNavBarPopoverManager: NetPPopoverManager { private var networkProtectionPopover: NetworkProtectionPopover? - let ipcClient: TunnelControllerIPCClient + let ipcClient: NetworkProtectionIPCClient - init(ipcClient: TunnelControllerIPCClient) { + init(ipcClient: NetworkProtectionIPCClient) { self.ipcClient = ipcClient } @@ -55,9 +70,9 @@ final class NetworkProtectionNavBarPopoverManager { let controller = NetworkProtectionIPCTunnelController(ipcClient: ipcClient) let statusReporter = DefaultNetworkProtectionStatusReporter( - statusObserver: ipcClient.connectionStatusObserver, - serverInfoObserver: ipcClient.serverInfoObserver, - connectionErrorObserver: ipcClient.connectionErrorObserver, + statusObserver: ipcClient.ipcStatusObserver, + serverInfoObserver: ipcClient.ipcServerInfoObserver, + connectionErrorObserver: ipcClient.ipcConnectionErrorObserver, connectivityIssuesObserver: ConnectivityIssueObserverThroughDistributedNotifications(), controllerErrorMessageObserver: ControllerErrorMesssageObserverThroughDistributedNotifications() ) diff --git a/DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift b/DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift index bfe8f7ce36..c61b63d7b3 100644 --- a/DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift +++ b/DuckDuckGo/NetworkProtection/AppTargets/DeveloperIDTarget/NetworkProtectionIPCTunnelController.swift @@ -25,10 +25,10 @@ import NetworkProtectionIPC final class NetworkProtectionIPCTunnelController: TunnelController { private let loginItemsManager: LoginItemsManager - private let ipcClient: TunnelControllerIPCClient + private let ipcClient: NetworkProtectionIPCClient init(loginItemsManager: LoginItemsManager = LoginItemsManager(), - ipcClient: TunnelControllerIPCClient) { + ipcClient: NetworkProtectionIPCClient) { self.loginItemsManager = loginItemsManager self.ipcClient = ipcClient @@ -54,7 +54,7 @@ final class NetworkProtectionIPCTunnelController: TunnelController { /// var isConnected: Bool { get { - if case .connected = ipcClient.connectionStatusObserver.recentValue { + if case .connected = ipcClient.ipcStatusObserver.recentValue { return true } diff --git a/DuckDuckGo/SecureVault/View/AutofillPopoverPresenter.swift b/DuckDuckGo/SecureVault/View/AutofillPopoverPresenter.swift new file mode 100644 index 0000000000..871641ebb0 --- /dev/null +++ b/DuckDuckGo/SecureVault/View/AutofillPopoverPresenter.swift @@ -0,0 +1,84 @@ +// +// AutofillPopoverPresenter.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import BrowserServicesKit +import Foundation + +protocol AutofillPopoverPresenter { + var passwordDomain: String? { get set } + var popoverIsDirty: Bool { get } + var popoverIsShown: Bool { get } + var popoverPresentingWindow: NSWindow? { get } + func show(positionedBelow view: NSView, withDomain domain: String?, selectedCategory category: SecureVaultSorting.Category?) + func show(positionedBelow view: NSView, withSelectedAccount: SecureVaultModels.WebsiteAccount) + func dismiss() +} + +final class DefaultAutofillPopoverPresenter: AutofillPopoverPresenter, PopoverPresenter { + + private var popover: PasswordManagementPopover? + + var passwordDomain: String? { + get { + popover?.viewController.domain + } set { + popover?.viewController.domain = newValue + } + } + + /// Property indicating whether the popover view controller is "dirty", i.e it's state has been edited but is unsaved + var popoverIsDirty: Bool { + popover?.viewController.isDirty ?? false + } + + var popoverIsShown: Bool { + popover?.isShown ?? false + } + + var popoverPresentingWindow: NSWindow? { + popover?.mainWindow + } + + /// Note: Dismisses any previously displayed popover before showing a new one + func show(positionedBelow view: NSView, withDomain domain: String?, selectedCategory category: SecureVaultSorting.Category?) { + show(under: view, withDomain: domain).select(category: category) + } + + func show(positionedBelow view: NSView, withSelectedAccount account: SecureVaultModels.WebsiteAccount) { + show(under: view, withDomain: nil).select(websiteAccount: account) + } + + func dismiss() { + guard popoverIsShown, let popover else { return } + popover.close() + self.popover = nil + } +} + +private extension DefaultAutofillPopoverPresenter { + + func show(under view: NSView, withDomain domain: String?) -> PasswordManagementPopover { + dismiss() + + let popover = PasswordManagementPopover() + self.popover = popover + popover.viewController.domain = domain + show(popover, positionedBelow: view) + return popover + } +} diff --git a/DuckDuckGo/SecureVault/View/PasswordManagementPopover.swift b/DuckDuckGo/SecureVault/View/PasswordManagementPopover.swift index f25f41632e..7e439c1e25 100644 --- a/DuckDuckGo/SecureVault/View/PasswordManagementPopover.swift +++ b/DuckDuckGo/SecureVault/View/PasswordManagementPopover.swift @@ -23,8 +23,6 @@ import SwiftUI final class PasswordManagementPopover: NSPopover { - private var cancellables = Set() - override init() { super.init() @@ -61,23 +59,6 @@ final class PasswordManagementPopover: NSPopover { extension PasswordManagementPopover: NSPopoverDelegate { - func popoverDidShow(_ notification: Notification) { - NotificationCenter.default.publisher(for: NSWindow.didResignMainNotification) - .receive(on: DispatchQueue.main) - .sink { [weak self] _ in - guard let self = self, self.isShown else { return } - - if !DeviceAuthenticator.shared.isAuthenticating { - self.close() - } - } - .store(in: &cancellables) - } - - func popoverShouldClose(_ popover: NSPopover) -> Bool { - return !DeviceAuthenticator.shared.isAuthenticating - } - func popoverDidClose(_ notification: Notification) { if let window = viewController.view.window { for sheet in window.sheets { diff --git a/DuckDuckGo/Windows/View/WindowsManager.swift b/DuckDuckGo/Windows/View/WindowsManager.swift index bfe93e0329..cb622baed4 100644 --- a/DuckDuckGo/Windows/View/WindowsManager.swift +++ b/DuckDuckGo/Windows/View/WindowsManager.swift @@ -30,6 +30,9 @@ final class WindowsManager { NSApplication.shared.windows.compactMap { $0 as? MainWindow } } + // Shared type to enable managing `PasswordManagementPopover`s in multiple windows + private static let autofillPopoverPresenter: AutofillPopoverPresenter = DefaultAutofillPopoverPresenter() + class func closeWindows(except windows: [NSWindow] = []) { for controller in WindowControllersManager.shared.mainWindowControllers { guard let window = controller.window, !windows.contains(window) else { continue } @@ -61,7 +64,8 @@ final class WindowsManager { lazyLoadTabs: Bool = false) -> MainWindow? { let mainWindowController = makeNewWindow(tabCollectionViewModel: tabCollectionViewModel, popUp: popUp, - burnerMode: burnerMode) + burnerMode: burnerMode, + autofillPopoverPresenter: autofillPopoverPresenter) if let contentSize { mainWindowController.window?.setContentSize(contentSize) @@ -161,8 +165,9 @@ final class WindowsManager { private class func makeNewWindow(tabCollectionViewModel: TabCollectionViewModel? = nil, contentSize: NSSize? = nil, popUp: Bool = false, - burnerMode: BurnerMode) -> MainWindowController { - let mainViewController = MainViewController(tabCollectionViewModel: tabCollectionViewModel ?? TabCollectionViewModel(burnerMode: burnerMode)) + burnerMode: BurnerMode, + autofillPopoverPresenter: AutofillPopoverPresenter) -> MainWindowController { + let mainViewController = MainViewController(tabCollectionViewModel: tabCollectionViewModel ?? TabCollectionViewModel(burnerMode: burnerMode), autofillPopoverPresenter: autofillPopoverPresenter) var contentSize = contentSize ?? NSSize(width: 1024, height: 790) contentSize.width = min(NSScreen.main?.frame.size.width ?? 1024, max(contentSize.width, 300)) diff --git a/UnitTests/Geolocation/CLLocationManagerMock.swift b/UnitTests/Geolocation/CLLocationManagerMock.swift index bf61216d4f..6921d58667 100644 --- a/UnitTests/Geolocation/CLLocationManagerMock.swift +++ b/UnitTests/Geolocation/CLLocationManagerMock.swift @@ -103,4 +103,21 @@ final class CLLocationManagerMock: CLLocationManager { isUpdatingHeading = true } + override func requestLocation() { + fatalError("Unexpected call") + } + + var whenInUseAuthorizationRequested: (() -> Void)! + override func requestWhenInUseAuthorization() { + whenInUseAuthorizationRequested!() + } + + override func requestAlwaysAuthorization() { + fatalError("Unexpected call") + } + + override func requestTemporaryFullAccuracyAuthorization(withPurposeKey purposeKey: String, completion: (((any Error)?) -> Void)? = nil) { + fatalError("Unexpected call") + } + } diff --git a/UnitTests/Geolocation/GeolocationServiceTests.swift b/UnitTests/Geolocation/GeolocationServiceTests.swift index af0e5fe74a..b0547f2a95 100644 --- a/UnitTests/Geolocation/GeolocationServiceTests.swift +++ b/UnitTests/Geolocation/GeolocationServiceTests.swift @@ -196,8 +196,13 @@ final class GeolocationServiceTests: XCTestCase { } func testWhenLastSubscriberUnsubscribedThenLocationManagerStopped() { + let eAuthRequested = expectation(description: "Authorization requested") + locationManagerMock.whenInUseAuthorizationRequested = { eAuthRequested.fulfill() } + let c1 = service.locationPublisher.sink { _ in } _=service.locationPublisher.sink { _ in } + waitForExpectations(timeout: 0) + let c3 = service.locationPublisher.sink { [locationManagerMock] _ in XCTAssertTrue(locationManagerMock!.isUpdatingLocation) } @@ -211,6 +216,8 @@ final class GeolocationServiceTests: XCTestCase { let loc1 = CLLocation(latitude: 51.1, longitude: 23.4) let loc2 = CLLocation(latitude: 11.1, longitude: -12.2) + let eAuthRequested = expectation(description: "Authorization requested") + locationManagerMock.whenInUseAuthorizationRequested = { eAuthRequested.fulfill() } let c1 = service.locationPublisher.sink { _ in } var c2: AnyCancellable! = service.locationPublisher.sink { _ in } let e = expectation(description: "expect still receiving location") @@ -235,6 +242,7 @@ final class GeolocationServiceTests: XCTestCase { func testWhenResubscribedThenLastLocationIsPublished() { let location = CLLocation(latitude: 51.1, longitude: 23.4) + locationManagerMock.whenInUseAuthorizationRequested = { } let c1 = service.locationPublisher.sink { _ in } locationManagerMock.currentLocation = location c1.cancel() @@ -248,8 +256,9 @@ final class GeolocationServiceTests: XCTestCase { waitForExpectations(timeout: 0) } - func testWhenAtoppedThenNoErrorIsPublished() { + func testWhenStoppedThenNoErrorIsPublished() { let location = CLLocation(latitude: 51.1, longitude: 23.4) + locationManagerMock.whenInUseAuthorizationRequested = { } let c1 = service.locationPublisher.sink { _ in } locationManagerMock.currentLocation = location c1.cancel() @@ -267,6 +276,8 @@ final class GeolocationServiceTests: XCTestCase { } func testWhenHighAccuracyRequestedThenAccuracyIsSetToBest() { + let eAuthRequested = expectation(description: "Authorization requested") + locationManagerMock.whenInUseAuthorizationRequested = { eAuthRequested.fulfill() } let c1 = service.locationPublisher.sink { _ in } XCTAssertEqual(locationManagerMock.desiredAccuracy, kCLLocationAccuracyHundredMeters) @@ -276,6 +287,7 @@ final class GeolocationServiceTests: XCTestCase { c2.cancel() XCTAssertEqual(locationManagerMock.desiredAccuracy, kCLLocationAccuracyHundredMeters) withExtendedLifetime(c1) {} + waitForExpectations(timeout: 0) } } diff --git a/UnitTests/NavigationBar/Mocks/MockAutofillPopoverPresenter.swift b/UnitTests/NavigationBar/Mocks/MockAutofillPopoverPresenter.swift new file mode 100644 index 0000000000..c25d96950e --- /dev/null +++ b/UnitTests/NavigationBar/Mocks/MockAutofillPopoverPresenter.swift @@ -0,0 +1,53 @@ +// +// MockAutofillPopoverPresenter.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import BrowserServicesKit +@testable import DuckDuckGo_Privacy_Browser +import Foundation + +final class MockAutofillPopoverPresenter: AutofillPopoverPresenter { + + var didShowWithCategory = false + var didShowWithSelectedAccount = false + var didDismiss = false + var isDirty = false + var isShown = false + + var passwordDomain: String? + + var popoverIsDirty: Bool { + isDirty + } + var popoverIsShown: Bool { + isShown + } + + var popoverPresentingWindow: NSWindow? + + func show(positionedBelow view: NSView, withDomain domain: String?, selectedCategory category: DuckDuckGo_Privacy_Browser.SecureVaultSorting.Category?) { + didShowWithCategory = true + } + + func show(positionedBelow view: NSView, withSelectedAccount: BrowserServicesKit.SecureVaultModels.WebsiteAccount) { + didShowWithSelectedAccount = true + } + + func dismiss() { + didDismiss = true + } +} diff --git a/UnitTests/NavigationBar/View/NavigationBarPopoversTests.swift b/UnitTests/NavigationBar/View/NavigationBarPopoversTests.swift new file mode 100644 index 0000000000..461c0109be --- /dev/null +++ b/UnitTests/NavigationBar/View/NavigationBarPopoversTests.swift @@ -0,0 +1,104 @@ +// +// NavigationBarPopoversTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import BrowserServicesKit +@testable import DuckDuckGo_Privacy_Browser +import XCTest + +private final class MockNSPopoverDelegate: NSObject, NSPopoverDelegate {} + +final class NavigationBarPopoversTests: XCTestCase { + + private var sut: NavigationBarPopovers! + private var autofillPopoverPresenter: MockAutofillPopoverPresenter! + + override func setUpWithError() throws { + autofillPopoverPresenter = MockAutofillPopoverPresenter() + + #if NETWORK_PROTECTION + sut = NavigationBarPopovers(networkProtectionPopoverManager: NetPPopoverManagerMock(), autofillPopoverPresenter: autofillPopoverPresenter) + #else + sut = NavigationBarPopovers(passwordPopoverPresenter: popoverPresenter) + #endif + } + + func testSetsPasswordPopoverDomainOnPopover() throws { + // Given + let domain = "test" + + // When + sut.passwordManagementDomain = domain + + // Then + XCTAssertEqual(autofillPopoverPresenter.passwordDomain, domain) + } + + func testGetsPasswordPopoverDirtyState() throws { + // When + var dirtyResult = sut.isPasswordManagementDirty + + // Then + XCTAssertFalse(dirtyResult) + + // Given + autofillPopoverPresenter.isDirty = true + + // When + dirtyResult = sut.isPasswordManagementDirty + + // Then + XCTAssertTrue(dirtyResult) + } + + func testGetsPasswordPopoverShownState() throws { + // When + var displayedResult = sut.isPasswordManagementPopoverShown + + // Then + XCTAssertFalse(displayedResult) + + // Given + autofillPopoverPresenter.isShown = true + + // When + displayedResult = sut.isPasswordManagementPopoverShown + + // Then + XCTAssertTrue(displayedResult) + } + + func testShowsPasswordPopoverWithCategory() throws { + // When + sut.showPasswordManagementPopover(selectedCategory: nil, usingView: NSView(), withDelegate: MockNSPopoverDelegate()) + + // Then + XCTAssertTrue(autofillPopoverPresenter.didShowWithCategory) + } + + func testShowsPasswordPopoverWithSelectedWebsite() throws { + // Given + let account = SecureVaultModels.WebsiteAccount(id: "") + + // When + sut.showPasswordManagerPopover(selectedWebsiteAccount: account, usingView: NSView(), withDelegate: MockNSPopoverDelegate()) + + // Then + XCTAssertTrue(autofillPopoverPresenter.didShowWithSelectedAccount) + } + +} diff --git a/UnitTests/Preferences/AutofillPreferencesModelTests.swift b/UnitTests/Preferences/AutofillPreferencesModelTests.swift index 044a56b252..79341abe79 100644 --- a/UnitTests/Preferences/AutofillPreferencesModelTests.swift +++ b/UnitTests/Preferences/AutofillPreferencesModelTests.swift @@ -46,13 +46,17 @@ final class UserAuthenticatorMock: UserAuthenticating { } } -@MainActor final class AutofillPreferencesModelTests: XCTestCase { + func neverPromptWebsitesManager() throws -> AutofillNeverPromptWebsitesManager { + try AutofillNeverPromptWebsitesManager(secureVault: MockSecureVaultFactory.makeVault(errorReporter: nil)) + } + + @MainActor func testThatPreferencesArePersisted() throws { let persistor = AutofillPreferencesPersistorMock() let userAuthenticator = UserAuthenticatorMock() - let model = AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator) + let model = try AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator, neverPromptWebsitesManager: neverPromptWebsitesManager()) model.askToSaveUsernamesAndPasswords.toggle() XCTAssertEqual(persistor.askToSaveUsernamesAndPasswords, model.askToSaveUsernamesAndPasswords) @@ -64,10 +68,11 @@ final class AutofillPreferencesModelTests: XCTestCase { XCTAssertEqual(persistor.askToSavePaymentMethods, model.askToSavePaymentMethods) } + @MainActor func testWhenUserIsAuthenticatedThenAutoLockCanBeDisabled() throws { let persistor = AutofillPreferencesPersistorMock() let userAuthenticator = UserAuthenticatorMock() - let model = AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator) + let model = try AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator, neverPromptWebsitesManager: neverPromptWebsitesManager()) userAuthenticator._authenticateUser = { _ in return .success} @@ -76,10 +81,11 @@ final class AutofillPreferencesModelTests: XCTestCase { XCTAssertEqual(persistor.isAutoLockEnabled, model.isAutoLockEnabled) } + @MainActor func testWhenUserIsNotAuthenticatedThenAutoLockCannotBeDisabled() throws { let persistor = AutofillPreferencesPersistorMock() let userAuthenticator = UserAuthenticatorMock() - let model = AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator) + let model = try AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator, neverPromptWebsitesManager: neverPromptWebsitesManager()) userAuthenticator._authenticateUser = { _ in return .failure} @@ -88,10 +94,11 @@ final class AutofillPreferencesModelTests: XCTestCase { XCTAssertEqual(persistor.isAutoLockEnabled, model.isAutoLockEnabled) } + @MainActor func testWhenUserIsAuthenticatedThenAutoLockThresholdCanBeChanged() throws { let persistor = AutofillPreferencesPersistorMock() let userAuthenticator = UserAuthenticatorMock() - let model = AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator) + let model = try AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator, neverPromptWebsitesManager: neverPromptWebsitesManager()) userAuthenticator._authenticateUser = { _ in return .success} @@ -101,10 +108,11 @@ final class AutofillPreferencesModelTests: XCTestCase { XCTAssertEqual(persistor.autoLockThreshold, model.autoLockThreshold) } + @MainActor func testWhenUserIsNotAuthenticatedThenAutoLockThresholdCannotBeChanged() throws { let persistor = AutofillPreferencesPersistorMock() let userAuthenticator = UserAuthenticatorMock() - let model = AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator) + let model = try AutofillPreferencesModel(persistor: persistor, userAuthenticator: userAuthenticator, neverPromptWebsitesManager: neverPromptWebsitesManager()) userAuthenticator._authenticateUser = { _ in return .failure}