Skip to content

Commit

Permalink
Add 'Open in New Tab' support for DuckPlayer (#3431)
Browse files Browse the repository at this point in the history
Task/Issue URL:
https://app.asana.com/0/1204099484721401/1208366641490350/f

**Description**:
Adds a setting to allow users to open DuckPlayer videos in a new tab.
(On by default)
  • Loading branch information
afterxleep authored Oct 15, 2024
1 parent c62f077 commit f876b4d
Show file tree
Hide file tree
Showing 16 changed files with 242 additions and 8 deletions.
3 changes: 3 additions & 0 deletions Core/FeatureFlag.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public enum FeatureFlag: String {
case history
case newTabPageSections
case duckPlayer
case duckPlayerOpenInNewTab
case sslCertificatesBypass
case syncPromotionBookmarks
case syncPromotionPasswords
Expand Down Expand Up @@ -83,6 +84,8 @@ extension FeatureFlag: FeatureFlagSourceProviding {
return .remoteDevelopment(.feature(.newTabPageImprovements))
case .duckPlayer:
return .remoteReleasable(.subfeature(DuckPlayerSubfeature.enableDuckPlayer))
case .duckPlayerOpenInNewTab:
return .remoteReleasable(.subfeature(DuckPlayerSubfeature.openInNewTab))
case .sslCertificatesBypass:
return .remoteReleasable(.subfeature(SslCertificatesSubfeature.allowBypass))
case .syncPromotionBookmarks:
Expand Down
4 changes: 4 additions & 0 deletions Core/PixelEvent.swift
Original file line number Diff line number Diff line change
Expand Up @@ -799,6 +799,8 @@ extension Pixel {
case duckPlayerSettingNeverOverlayYoutube
case duckPlayerContingencySettingsDisplayed
case duckPlayerContingencyLearnMoreClicked
case duckPlayerNewTabSettingOn
case duckPlayerNewTabSettingOff

// MARK: enhanced statistics
case usageSegments
Expand Down Expand Up @@ -1618,6 +1620,8 @@ extension Pixel.Event {
case .duckPlayerSettingNeverOverlayYoutube: return "duckplayer_setting_never_overlay_youtube"
case .duckPlayerContingencySettingsDisplayed: return "duckplayer_ios_contingency_settings-displayed"
case .duckPlayerContingencyLearnMoreClicked: return "duckplayer_ios_contingency_learn-more-clicked"
case .duckPlayerNewTabSettingOn: return "duckplayer_ios_newtab_setting-on"
case .duckPlayerNewTabSettingOff: return "duckplayer_ios_newtab_setting-off"

// MARK: Enhanced statistics
case .usageSegments: return "m_retention_segments"
Expand Down
1 change: 1 addition & 0 deletions Core/UserDefaultsPropertyWrapper.swift
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
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
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: true)
var duckPlayerOpenInNewTab: Bool


@UserDefaultsWrapper(key: .debugOnboardingHighlightsEnabledKey, defaultValue: false)
var onboardingHighlightsEnabled: Bool
Expand Down
76 changes: 73 additions & 3 deletions DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
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 @@ -244,6 +245,30 @@ final class DuckPlayerNavigationHandler {

}

// Determines if the link should be opened in a new tab
// And sets the correct navigationType
// This is uses for JS based navigation links
private func setOpenInNewTab(url: URL?) {
guard let url else {
return
}

// let openInNewTab = appSettings.duckPlayerOpenInNewTab
let openInNewTab = appSettings.duckPlayerOpenInNewTab
let isFeatureEnabled = featureFlagger.isFeatureOn(.duckPlayer)
let isSubFeatureEnabled = featureFlagger.isFeatureOn(.duckPlayerOpenInNewTab) || internalUserDecider.isInternalUser
let isDuckPlayerEnabled = duckPlayer.settings.mode == .enabled || duckPlayer.settings.mode == .alwaysAsk

if openInNewTab &&
isFeatureEnabled &&
isSubFeatureEnabled &&
isDuckPlayerEnabled {
navigationType = .linkActivated
} else {
navigationType = .other
}
}

}

extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
Expand Down Expand Up @@ -338,7 +363,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,7 +437,14 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
return
}

handleURLChange(url: url, webView: webView)
// Assume JS Navigation is user-triggered
self.navigationType = .linkActivated

// Only handle URL changes if the allowFirstVideo is set to false
// This prevents Youtube redirects from triggering DuckPlayer when is not expected
if !duckPlayer.settings.allowFirstVideo {
handleURLChange(url: url, webView: webView)
}
}

@MainActor
Expand Down Expand Up @@ -491,12 +531,42 @@ 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)
case .JSTriggeredNavigation:
setOpenInNewTab(url: url)
}
}

// Determine if the links should be open in a new tab, based on the navigationAction and User setting
// This is used for manually activated links
func shouldOpenInNewTab(_ navigationAction: WKNavigationAction, webView: WKWebView) -> Bool {

// let openInNewTab = appSettings.duckPlayerOpenInNewTab
let openInNewTab = appSettings.duckPlayerOpenInNewTab
let isFeatureEnabled = featureFlagger.isFeatureOn(.duckPlayer)
let isSubFeatureEnabled = featureFlagger.isFeatureOn(.duckPlayerOpenInNewTab) || internalUserDecider.isInternalUser
let isDuckPlayer = navigationAction.request.url?.isDuckPlayer ?? false
let isDuckPlayerEnabled = duckPlayer.settings.mode == .enabled || duckPlayer.settings.mode == .alwaysAsk

if openInNewTab &&
isFeatureEnabled &&
isSubFeatureEnabled &&
isDuckPlayer &&
self.navigationType == .linkActivated &&
isDuckPlayerEnabled {
return true
}
return false
}

}

extension WKWebView {
var isEmptyTab: Bool {
return self.url == nil || self.url?.absoluteString == "about:blank"
}
}
3 changes: 3 additions & 0 deletions DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import WebKit

enum DuckPlayerNavigationEvent {
case youtubeVideoPageVisited
case JSTriggeredNavigation
}

protocol DuckPlayerNavigationHandling: AnyObject {
Expand All @@ -38,4 +39,6 @@ protocol DuckPlayerNavigationHandling: AnyObject {
func handleEvent(event: DuckPlayerNavigationEvent,
url: URL?,
navigationAction: WKNavigationAction?)
func shouldOpenInNewTab(_ navigationAction: WKNavigationAction, webView: WKWebView) -> Bool

}
5 changes: 5 additions & 0 deletions DuckDuckGo/SettingsDuckPlayerView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@ struct SettingsDuckPlayerView: View {
options: DuckPlayerMode.allCases,
selectedOption: viewModel.duckPlayerModeBinding)
.disabled(viewModel.shouldDisplayDuckPlayerContingencyMessage)

if viewModel.state.duckPlayerOpenInNewTabEnabled || viewModel.isInternalUser {
SettingsCellView(label: UserText.settingsOpenDuckPlayerNewTabLabel,
accessory: .toggle(isOn: viewModel.duckPlayerOpenInNewTabBinding))
}
}
}
.applySettingsListModifiers(title: UserText.duckPlayerFeatureName,
Expand Down
6 changes: 5 additions & 1 deletion DuckDuckGo/SettingsState.swift
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,8 @@ struct SettingsState {
// Duck Player Mode
var duckPlayerEnabled: Bool
var duckPlayerMode: DuckPlayerMode?
var duckPlayerOpenInNewTab: Bool
var duckPlayerOpenInNewTabEnabled: Bool

static var defaults: SettingsState {
return SettingsState(
Expand Down Expand Up @@ -136,7 +138,9 @@ struct SettingsState {
sync: SyncSettings(enabled: false, title: ""),
syncSource: nil,
duckPlayerEnabled: false,
duckPlayerMode: .alwaysAsk
duckPlayerMode: .alwaysAsk,
duckPlayerOpenInNewTab: true,
duckPlayerOpenInNewTabEnabled: false
)
}
}
20 changes: 19 additions & 1 deletion DuckDuckGo/SettingsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,21 @@ final class SettingsViewModel: ObservableObject {
}
)
}

var duckPlayerOpenInNewTabBinding: Binding<Bool> {
Binding<Bool>(
get: { self.state.duckPlayerOpenInNewTab },
set: {
self.appSettings.duckPlayerOpenInNewTab = $0
self.state.duckPlayerOpenInNewTab = $0
if self.state.duckPlayerOpenInNewTab {
Pixel.fire(pixel: Pixel.Event.duckPlayerNewTabSettingOn)
} else {
Pixel.fire(pixel: Pixel.Event.duckPlayerNewTabSettingOff)
}
}
)
}

func setVoiceSearchEnabled(to value: Bool) {
if value {
Expand Down Expand Up @@ -410,7 +425,10 @@ extension SettingsViewModel {
sync: getSyncState(),
syncSource: nil,
duckPlayerEnabled: featureFlagger.isFeatureOn(.duckPlayer) || shouldDisplayDuckPlayerContingencyMessage,
duckPlayerMode: appSettings.duckPlayerMode
duckPlayerMode: appSettings.duckPlayerMode,
duckPlayerOpenInNewTab: appSettings.duckPlayerOpenInNewTab,
duckPlayerOpenInNewTabEnabled: featureFlagger.isFeatureOn(.duckPlayerOpenInNewTab)

)

updateRecentlyVisitedSitesVisibility()
Expand Down
14 changes: 13 additions & 1 deletion DuckDuckGo/TabViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -766,6 +766,12 @@ class TabViewController: UIViewController {
}
if let url {
duckPlayerNavigationHandler?.referrer = url.isYoutube ? .youtube : .other

// Open in new tab if required
// If the lastRenderedURL is nil, it means we're already in a new tab
if webView.url != nil && lastRenderedURL != nil {
duckPlayerNavigationHandler?.handleEvent(event: .JSTriggeredNavigation, url: webView.url, navigationAction: nil)
}
}
}

Expand Down Expand Up @@ -1886,7 +1892,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
3 changes: 3 additions & 0 deletions DuckDuckGo/UserText.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1277,6 +1277,9 @@ But if you *do* want a peek under the hood, you can find more information about
public static let duckPlayerDisabledLabel = NSLocalizedString("duckPlayer.never.label", value: "Never", comment: "Text displayed when DuckPlayer is in off.")
public static let settingsOpenVideosInDuckPlayerLabel = NSLocalizedString("duckplayer.settings.open-videos-in", value: "Open Videos in Duck Player", comment: "Settings screen cell text for DuckPlayer settings")
public static let duckPlayerFeatureName = NSLocalizedString("duckplayer.settings.title", value: "Duck Player", comment: "Settings screen cell text for DuckPlayer settings")

public static let settingsOpenDuckPlayerNewTabLabel = NSLocalizedString("duckplayer.settings.open-new-tab-label", value: "Open Duck Player in a new tab", comment: "Settings screen cell text for DuckPlayer settings to open in new tab")


public static let settingsOpenVideosInDuckPlayerTitle = NSLocalizedString("duckplayer.settings.title", value: "Duck Player", comment: "Settings screen cell text for DuckPlayer settings")
public static let settingsDuckPlayerFooter = NSLocalizedString("duckplayer.settings.footer", value: "DuckDuckGo provides all the privacy essentials you need to protect yourself as you browse the web.", comment: "Footer label in the settings screen for Duck Player")
Expand Down
3 changes: 3 additions & 0 deletions DuckDuckGo/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,9 @@
/* Button that takes the user to learn more about Duck Player. */
"duckplayer.settings.learn-more" = "Learn More";

/* Settings screen cell text for DuckPlayer settings to open in new tab */
"duckplayer.settings.open-new-tab-label" = "Open Duck Player in a new tab";

/* Settings screen cell text for DuckPlayer settings */
"duckplayer.settings.open-videos-in" = "Open Videos in Duck Player";

Expand Down
5 changes: 4 additions & 1 deletion DuckDuckGoTests/AppSettingsMock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -86,12 +86,15 @@ class AppSettingsMock: AppSettings {

var duckPlayerMode: DuckDuckGo.DuckPlayerMode = .alwaysAsk
var duckPlayerAskModeOverlayHidden: Bool = false

var duckPlayerOpenInNewTab: Bool = false

var newTabPageShortcutsSettings: Data?
var newTabPageSectionsSettings: Data?

var newTabPageIntroMessageEnabled: Bool?
var newTabPageIntroMessageSeenCount: Int = 0

var onboardingHighlightsEnabled: Bool = false


}
8 changes: 7 additions & 1 deletion DuckDuckGoTests/DuckPlayerMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,20 @@ class MockWebView: WKWebView {

class MockNavigationAction: WKNavigationAction {
private let _request: URLRequest
private let _navigationType: WKNavigationType

init(request: URLRequest) {
init(request: URLRequest, navigationType: WKNavigationType = .linkActivated ) {
self._request = request
self._navigationType = navigationType
}

override var request: URLRequest {
return _request
}

override var navigationType: WKNavigationType {
return _navigationType
}
}

final class MockDuckPlayerSettings: DuckPlayerSettings {
Expand Down
Loading

0 comments on commit f876b4d

Please sign in to comment.