From 6fc8a2e3ba467654adc690c9e8f069b5aab44aae Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Tue, 8 Oct 2024 17:23:24 +0200 Subject: [PATCH 1/3] Open DuckPlayer Videos in New Tab #1 --- Core/UserDefaultsPropertyWrapper.swift | 1 + DuckDuckGo/AppSettings.swift | 1 + DuckDuckGo/AppUserDefaults.swift | 5 ++++ .../DuckPlayerNavigationHandler.swift | 29 +++++++++++++++++-- .../DuckPlayerNavigationHandling.swift | 2 ++ DuckDuckGo/TabViewController.swift | 8 ++++- 6 files changed, 43 insertions(+), 3 deletions(-) diff --git a/Core/UserDefaultsPropertyWrapper.swift b/Core/UserDefaultsPropertyWrapper.swift index 6df337d168..2e119e09df 100644 --- a/Core/UserDefaultsPropertyWrapper.swift +++ b/Core/UserDefaultsPropertyWrapper.swift @@ -159,6 +159,7 @@ public struct UserDefaultsWrapper { case duckPlayerMode = "com.duckduckgo.ios.duckPlayerMode" case duckPlayerAskModeOverlayHidden = "com.duckduckgo.ios.duckPlayerAskModeOverlayHidden" case userInteractedWithDuckPlayer = "com.duckduckgo.ios.userInteractedWithDuckPlayer" + case duckPlayerOpenInNewTab = "com.duckduckgo.ios.duckPlayerOpenInNewTab" case vpnRedditWorkaroundInstalled = "com.duckduckgo.ios.vpn.workaroundInstalled" diff --git a/DuckDuckGo/AppSettings.swift b/DuckDuckGo/AppSettings.swift index d3b99acc70..e2c31dda2b 100644 --- a/DuckDuckGo/AppSettings.swift +++ b/DuckDuckGo/AppSettings.swift @@ -82,6 +82,7 @@ protocol AppSettings: AnyObject, AppDebugSettings { var duckPlayerMode: DuckPlayerMode { get set } var duckPlayerAskModeOverlayHidden: Bool { get set } + var duckPlayerOpenInNewTab: Bool { get set } } protocol AppDebugSettings { diff --git a/DuckDuckGo/AppUserDefaults.swift b/DuckDuckGo/AppUserDefaults.swift index 2c54a3a749..ad58c046db 100644 --- a/DuckDuckGo/AppUserDefaults.swift +++ b/DuckDuckGo/AppUserDefaults.swift @@ -78,6 +78,7 @@ public class AppUserDefaults: AppSettings { static let duckPlayerMode = "com.duckduckgo.ios.duckPlayerMode" static let duckPlayerAskModeOverlayHidden = "com.duckduckgo.ios.duckPlayerAskModeOverlayHidden" + static let duckPlayerOpenInNewTab = "com.duckduckgo.ios.duckPlayerOpenInNewTab" } private struct DebugKeys { @@ -414,6 +415,10 @@ public class AppUserDefaults: AppSettings { object: duckPlayerMode) } } + + @UserDefaultsWrapper(key: .duckPlayerOpenInNewTab, defaultValue: false) + var duckPlayerOpenInNewTab: Bool + @UserDefaultsWrapper(key: .debugOnboardingHighlightsEnabledKey, defaultValue: false) var onboardingHighlightsEnabled: Bool diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift index b2316a331e..3c77deabbc 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift @@ -33,6 +33,7 @@ final class DuckPlayerNavigationHandler { var lastHandledVideoID: String? var featureFlagger: FeatureFlagger var appSettings: AppSettings + var navigationType: WKNavigationType = .other var experiment: DuckPlayerLaunchExperimentHandling private lazy var internalUserDecider = AppDependencyProvider.shared.internalUserDecider @@ -338,7 +339,15 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { webView: WKWebView) { Logger.duckPlayer.debug("Handling DecidePolicyFor for \(navigationAction.request.url?.absoluteString ?? "")") - + + // This means navigation originated in user Event + // and not automatic. This is used further to + // determine how navigation is performed (new tab, etc) + // Resets on next attachment + if navigationAction.navigationType == .linkActivated { + self.navigationType = navigationAction.navigationType + } + guard let url = navigationAction.request.url else { completion(.cancel) return @@ -404,6 +413,9 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { return } + // Assume JS Navigation is user-triggered + self.navigationType = .linkActivated + handleURLChange(url: url, webView: webView) } @@ -491,12 +503,25 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { } // Handle custom events + // This method is used to delegate tasks to DuckPlayerHandler, such as firing pixels and etc. func handleEvent(event: DuckPlayerNavigationEvent, url: URL?, navigationAction: WKNavigationAction?) { - switch event { case .youtubeVideoPageVisited: handleYouTubePageVisited(url: url, navigationAction: navigationAction) } } + // Determine if the links should be open in a new tab, based on the User setting + func shouldOpenInNewTab(_ navigationAction: WKNavigationAction, webView: WKWebView) -> Bool { + // let openInNewTab = appSettings.duckPlayerOpenInNewTab + let openInNewTab = true + let isDuckPlayer = navigationAction.request.url?.isDuckPlayer ?? false + let isDuckPlayerEnabled = duckPlayer.settings.mode == .enabled || duckPlayer.settings.mode == .alwaysAsk + + if openInNewTab && isDuckPlayer && navigationType == .linkActivated && isDuckPlayerEnabled { + return true + } + return false + } + } diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift index 766959fdd2..f2a6e391a8 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift @@ -38,4 +38,6 @@ protocol DuckPlayerNavigationHandling: AnyObject { func handleEvent(event: DuckPlayerNavigationEvent, url: URL?, navigationAction: WKNavigationAction?) + func shouldOpenInNewTab(_ navigationAction: WKNavigationAction, webView: WKWebView) -> Bool + } diff --git a/DuckDuckGo/TabViewController.swift b/DuckDuckGo/TabViewController.swift index 668ff0364a..4d76752360 100644 --- a/DuckDuckGo/TabViewController.swift +++ b/DuckDuckGo/TabViewController.swift @@ -1886,7 +1886,13 @@ extension TabViewController: WKNavigationDelegate { duckPlayerNavigationHandler?.handleEvent(event: .youtubeVideoPageVisited, url: url, navigationAction: navigationAction) - duckPlayerNavigationHandler?.handleNavigation(navigationAction, webView: webView) + + // Validate Duck Player setting to open in new tab or locally + if duckPlayerNavigationHandler?.shouldOpenInNewTab(navigationAction, webView: webView) ?? false { + delegate?.tab(self, didRequestNewTabForUrl: url, openedByPage: false, inheritingAttribution: nil) + } else { + duckPlayerNavigationHandler?.handleNavigation(navigationAction, webView: webView) + } completion(.cancel) return From cd0fceb1ee6cab8aa2e0ed46914d691baa9b48cf Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Wed, 9 Oct 2024 13:23:20 +0200 Subject: [PATCH 2/3] Stop sending user values if DP experiment is Control --- DuckDuckGo/DuckPlayer/DuckPlayer.swift | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/DuckDuckGo/DuckPlayer/DuckPlayer.swift b/DuckDuckGo/DuckPlayer/DuckPlayer.swift index e3630118c1..fd2277e216 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayer.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayer.swift @@ -184,7 +184,12 @@ final class DuckPlayer: DuckPlayerProtocol { } public func getUserValues(params: Any, message: WKScriptMessage) -> Encodable? { - encodeUserValues() + let duckPlayerExperiment = DuckPlayerLaunchExperiment() + if duckPlayerExperiment.isEnrolled && duckPlayerExperiment.isExperimentCohort { + return encodeUserValues() + } + return nil + } @MainActor From 5e3cae30bbc3503a2703065a71702928112445e4 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Wed, 9 Oct 2024 13:33:48 +0200 Subject: [PATCH 3/3] DP Experiment fix --- DuckDuckGo/Debug.storyboard | 31 ++++++++++++------- .../DuckPlayerLaunchExperiment.swift | 4 +-- DuckDuckGo/RootDebugViewController.swift | 4 +++ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/DuckDuckGo/Debug.storyboard b/DuckDuckGo/Debug.storyboard index 5f5c4c25d2..7409306245 100644 --- a/DuckDuckGo/Debug.storyboard +++ b/DuckDuckGo/Debug.storyboard @@ -1,9 +1,9 @@ - + - + @@ -306,7 +306,7 @@ - + @@ -390,7 +390,16 @@ - + + + + + + + + + + @@ -1040,17 +1049,17 @@ - + - + - + - + diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerLaunchExperiment.swift b/DuckDuckGo/DuckPlayer/DuckPlayerLaunchExperiment.swift index 8edd25ed56..be9821fe3e 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayerLaunchExperiment.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayerLaunchExperiment.swift @@ -227,9 +227,9 @@ final class DuckPlayerLaunchExperiment: DuckPlayerLaunchExperimentHandling { lastVideoIDReportedV2 = nil } - func override() { + func override(control: Bool = false) { enrollmentDateV2 = Date() - experimentCohortV2 = "experiment" + experimentCohortV2 = control ? "control" : "experiment" lastDayPixelFiredV2 = nil lastWeekPixelFiredV2 = nil lastVideoIDReportedV2 = nil diff --git a/DuckDuckGo/RootDebugViewController.swift b/DuckDuckGo/RootDebugViewController.swift index 287ef73822..23500ebf8c 100644 --- a/DuckDuckGo/RootDebugViewController.swift +++ b/DuckDuckGo/RootDebugViewController.swift @@ -49,6 +49,7 @@ class RootDebugViewController: UITableViewController { case resetSyncPromoPrompts = 677 case resetDuckPlayerExperiment = 678 case overrideDuckPlayerExperiment = 679 + case overrideDuckPlayerExperimentControl = 680 } @IBOutlet weak var shareButton: UIBarButtonItem! @@ -189,6 +190,9 @@ class RootDebugViewController: UITableViewController { case .overrideDuckPlayerExperiment: DuckPlayerLaunchExperiment().override() ActionMessageView.present(message: "Overriding experiment. You are now in the 'experiment' group. Restart the app to complete") + case .overrideDuckPlayerExperimentControl: + DuckPlayerLaunchExperiment().override(control: true) + ActionMessageView.present(message: "Overriding experiment. You are now in the 'control' group. Restart the app to complete") } } }