Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Daniel/dp.experiment.fix #3420

Closed
wants to merge 3 commits into from
Closed

Daniel/dp.experiment.fix #3420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6fc8a2e
Open DuckPlayer Videos in New Tab #1
afterxleep Oct 8, 2024
cd0fceb
Stop sending user values if DP experiment is Control
afterxleep Oct 9, 2024
5e3cae3
DP Experiment fix
afterxleep Oct 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Core/UserDefaultsPropertyWrapper.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ public struct UserDefaultsWrapper<T> {
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"

Expand Down
1 change: 1 addition & 0 deletions DuckDuckGo/AppSettings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions DuckDuckGo/AppUserDefaults.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
31 changes: 20 additions & 11 deletions DuckDuckGo/Debug.storyboard
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
<?xml version="1.0" encoding="UTF-8"?>
<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="32700.99.1234" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="fgi-g1-scz">
<document type="com.apple.InterfaceBuilder3.CocoaTouch.Storyboard.XIB" version="3.0" toolsVersion="23094" targetRuntime="iOS.CocoaTouch" propertyAccessControl="none" useAutolayout="YES" useTraitCollections="YES" useSafeAreas="YES" colorMatched="YES" initialViewController="fgi-g1-scz">
<device id="retina6_1" orientation="portrait" appearance="light"/>
<dependencies>
<deployment identifier="iOS"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="22685"/>
<plugIn identifier="com.apple.InterfaceBuilder.IBCocoaTouchPlugin" version="23084"/>
<capability name="Named colors" minToolsVersion="9.0"/>
<capability name="Safe area layout guides" minToolsVersion="9.0"/>
<capability name="System colors in document resources" minToolsVersion="11.0"/>
Expand Down Expand Up @@ -306,7 +306,7 @@
<rect key="frame" x="0.0" y="841" width="414" height="44.5"/>
<autoresizingMask key="autoresizingMask"/>
<tableViewCellContentView key="contentView" opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" preservesSuperviewLayoutMargins="YES" insetsLayoutMarginsFromSafeArea="NO" tableViewCell="AgK-xW-xB6" id="onY-PV-AQp">
<rect key="frame" x="0.0" y="0.0" width="382" height="44.5"/>
<rect key="frame" x="0.0" y="0.0" width="370" height="44.5"/>
<autoresizingMask key="autoresizingMask"/>
</tableViewCellContentView>
<listContentConfiguration key="contentConfiguration" text="Internal User State" secondaryText=""/>
Expand Down Expand Up @@ -390,7 +390,16 @@
<rect key="frame" x="0.0" y="0.0" width="414" height="44.5"/>
<autoresizingMask key="autoresizingMask"/>
</tableViewCellContentView>
<listContentConfiguration key="contentConfiguration" text="Override DuckPlayer Experiment"/>
<listContentConfiguration key="contentConfiguration" text="Override DuckPlayer Experiment (Experiment)"/>
</tableViewCell>
<tableViewCell clipsSubviews="YES" tag="680" contentMode="scaleToFill" preservesSuperviewLayoutMargins="YES" selectionStyle="default" indentationWidth="10" id="hZs-5w-MXA">
<rect key="frame" x="0.0" y="1286" width="414" height="44.5"/>
<autoresizingMask key="autoresizingMask"/>
<tableViewCellContentView key="contentView" opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" preservesSuperviewLayoutMargins="YES" insetsLayoutMarginsFromSafeArea="NO" tableViewCell="hZs-5w-MXA" id="5VY-iv-hVc">
<rect key="frame" x="0.0" y="0.0" width="414" height="44.5"/>
<autoresizingMask key="autoresizingMask"/>
</tableViewCellContentView>
<listContentConfiguration key="contentConfiguration" text="Override DuckPlayer Experiment (Control)"/>
</tableViewCell>
</cells>
</tableViewSection>
Expand Down Expand Up @@ -1040,17 +1049,17 @@
<color key="backgroundColor" systemColor="systemBackgroundColor"/>
<prototypes>
<tableViewCell clipsSubviews="YES" contentMode="scaleToFill" preservesSuperviewLayoutMargins="YES" selectionStyle="default" indentationWidth="10" reuseIdentifier="ConfigurationURLTableViewCell" id="i6Y-Di-PX3" customClass="ConfigurationURLTableViewCell" customModule="DuckDuckGo" customModuleProvider="target">
<rect key="frame" x="0.0" y="50" width="414" height="92.5"/>
<rect key="frame" x="0.0" y="50" width="414" height="93"/>
<autoresizingMask key="autoresizingMask"/>
<tableViewCellContentView key="contentView" opaque="NO" clipsSubviews="YES" multipleTouchEnabled="YES" contentMode="center" preservesSuperviewLayoutMargins="YES" insetsLayoutMarginsFromSafeArea="NO" tableViewCell="i6Y-Di-PX3" id="qn4-gq-5fa">
<rect key="frame" x="0.0" y="0.0" width="414" height="92.5"/>
<rect key="frame" x="0.0" y="0.0" width="414" height="93"/>
<autoresizingMask key="autoresizingMask"/>
<subviews>
<stackView opaque="NO" contentMode="scaleToFill" distribution="equalSpacing" spacing="8" translatesAutoresizingMaskIntoConstraints="NO" id="pKD-Xm-Eu1">
<rect key="frame" x="20" y="11" width="374" height="70.5"/>
<rect key="frame" x="20" y="11" width="374" height="71"/>
<subviews>
<stackView opaque="NO" contentMode="scaleToFill" axis="vertical" distribution="fillEqually" translatesAutoresizingMaskIntoConstraints="NO" id="j3A-OZ-DWy">
<rect key="frame" x="0.0" y="0.0" width="44" height="70.5"/>
<rect key="frame" x="0.0" y="0.0" width="44" height="71"/>
<subviews>
<label opaque="NO" multipleTouchEnabled="YES" contentMode="left" insetsLayoutMarginsFromSafeArea="NO" text="Title" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" id="gKw-J7-XIW">
<rect key="frame" x="0.0" y="0.0" width="44" height="23.5"/>
Expand All @@ -1060,22 +1069,22 @@
<nil key="highlightedColor"/>
</label>
<label opaque="NO" multipleTouchEnabled="YES" contentMode="left" insetsLayoutMarginsFromSafeArea="NO" text="Subtitle" textAlignment="natural" lineBreakMode="tailTruncation" numberOfLines="0" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" id="UrI-B0-rWf">
<rect key="frame" x="0.0" y="23.5" width="44" height="23"/>
<rect key="frame" x="0.0" y="23.5" width="44" height="23.5"/>
<autoresizingMask key="autoresizingMask" flexibleMaxX="YES" flexibleMaxY="YES"/>
<fontDescription key="fontDescription" type="system" pointSize="12"/>
<color key="textColor" name="accent"/>
<nil key="highlightedColor"/>
</label>
<label opaque="NO" userInteractionEnabled="NO" contentMode="left" horizontalHuggingPriority="251" verticalHuggingPriority="251" text="Label" textAlignment="natural" lineBreakMode="tailTruncation" baselineAdjustment="alignBaselines" adjustsFontSizeToFit="NO" translatesAutoresizingMaskIntoConstraints="NO" id="6RK-ug-mZa">
<rect key="frame" x="0.0" y="47" width="44" height="23.5"/>
<rect key="frame" x="0.0" y="47.5" width="44" height="23.5"/>
<fontDescription key="fontDescription" type="system" pointSize="12"/>
<nil key="textColor"/>
<nil key="highlightedColor"/>
</label>
</subviews>
</stackView>
<button opaque="NO" contentMode="scaleToFill" contentHorizontalAlignment="right" contentVerticalAlignment="center" lineBreakMode="middleTruncation" translatesAutoresizingMaskIntoConstraints="NO" id="Nkj-yK-cgm">
<rect key="frame" x="350" y="0.0" width="24" height="70.5"/>
<rect key="frame" x="350" y="0.0" width="24" height="71"/>
<inset key="imageEdgeInsets" minX="0.0" minY="0.0" maxX="2.2250738585072014e-308" maxY="0.0"/>
<state key="normal" image="Reload-24"/>
</button>
Expand Down
7 changes: 6 additions & 1 deletion DuckDuckGo/DuckPlayer/DuckPlayer.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions DuckDuckGo/DuckPlayer/DuckPlayerLaunchExperiment.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 27 additions & 2 deletions DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -404,6 +413,9 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
return
}

// Assume JS Navigation is user-triggered
self.navigationType = .linkActivated

handleURLChange(url: url, webView: webView)
}

Expand Down Expand Up @@ -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
}

}
2 changes: 2 additions & 0 deletions DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ protocol DuckPlayerNavigationHandling: AnyObject {
func handleEvent(event: DuckPlayerNavigationEvent,
url: URL?,
navigationAction: WKNavigationAction?)
func shouldOpenInNewTab(_ navigationAction: WKNavigationAction, webView: WKWebView) -> Bool

}
4 changes: 4 additions & 0 deletions DuckDuckGo/RootDebugViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ class RootDebugViewController: UITableViewController {
case resetSyncPromoPrompts = 677
case resetDuckPlayerExperiment = 678
case overrideDuckPlayerExperiment = 679
case overrideDuckPlayerExperimentControl = 680
}

@IBOutlet weak var shareButton: UIBarButtonItem!
Expand Down Expand Up @@ -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")
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion DuckDuckGo/TabViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading