From b1b4b0f33d25d9e054dd8aeaa64352eae273a311 Mon Sep 17 00:00:00 2001 From: Daniel Bernal Date: Wed, 13 Nov 2024 16:48:44 +0100 Subject: [PATCH] Fixes duplicate Refresh Pixel --- .../DuckPlayerNavigationHandler.swift | 14 +++++++- .../DuckPlayerOverlayUsagePixels.swift | 34 +++++++++++-------- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift index 15bca907e3..609b1d3af8 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift @@ -683,6 +683,7 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { // Overlay Usage Pixel handling if let url = webView.url { duckPlayerOverlayUsagePixels?.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayerMode) + lastURLChangeHandling = Date() } // Check if DuckPlayer feature is enabled @@ -835,6 +836,17 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { // Reset allowFirstVideo duckPlayer.settings.allowFirstVideo = false + // Overlay Usage Pixel handling for Direct Navigation + if let url = webView.url, !url.isYoutube { + duckPlayerOverlayUsagePixels?.handleNavigationAndFirePixels(url: url, duckPlayerMode: duckPlayerMode) + } + // Reset Overlay Last Fired pixel after the page is loaded + // A delay is required as Youtube sometimes performs an extra redirect on load + DispatchQueue.main.asyncAfter(deadline: .now() + 3) { + self.duckPlayerOverlayUsagePixels?.lastFiredPixel = nil + } + + } /// Resets settings when the web view starts loading a new page. @@ -892,7 +904,7 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling { guard isDuckPlayerFeatureEnabled else { return false } - + // Only account for in 'Always' mode if duckPlayerMode == .disabled { return false diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift b/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift index 1a3250ebe6..6f3339dd72 100644 --- a/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift +++ b/DuckDuckGo/DuckPlayer/DuckPlayerOverlayUsagePixels.swift @@ -23,6 +23,7 @@ protocol DuckPlayerOverlayPixelFiring { var pixelFiring: PixelFiring.Type { get set } var navigationHistory: [URL] { get set } + var lastFiredPixel: Pixel.Event? { get set } func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode) } @@ -31,6 +32,7 @@ final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring { var pixelFiring: PixelFiring.Type var navigationHistory: [URL] = [] + var lastFiredPixel: Pixel.Event? private var idleTimer: Timer? private var idleTimeInterval: TimeInterval @@ -42,12 +44,6 @@ final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring { self.idleTimeInterval = timeoutInterval } - // Method to reset the idle timer - private func resetIdleTimer() { - idleTimer?.invalidate() - idleTimer = nil - } - func handleNavigationAndFirePixels(url: URL?, duckPlayerMode: DuckPlayerMode) { guard let url = url else { return } let comparisonURL = url.forComparison() @@ -72,7 +68,7 @@ final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring { // Fire the reload pixel if this is a reload navigation if isReload { - pixelFiring.fire(.duckPlayerYouTubeOverlayNavigationRefresh, withAdditionalParameters: [:]) + firePixel(.duckPlayerYouTubeOverlayNavigationRefresh) } else { // Determine if it’s a back navigation by looking further back in history let isBackNavigation = navigationHistory.count > 2 && @@ -80,23 +76,31 @@ final class DuckPlayerOverlayUsagePixels: DuckPlayerOverlayPixelFiring { // Fire the appropriate pixel based on navigation type if isBackNavigation { - pixelFiring.fire(.duckPlayerYouTubeOverlayNavigationBack, withAdditionalParameters: [:]) + firePixel(.duckPlayerYouTubeOverlayNavigationBack) } else if previousURL.isYoutubeWatch && currentURL.isYoutube { // Forward navigation within YouTube (including non-video URLs) - pixelFiring.fire(.duckPlayerYouTubeNavigationWithinYouTube, withAdditionalParameters: [:]) + firePixel(.duckPlayerYouTubeNavigationWithinYouTube) } else if previousURL.isYoutubeWatch && !currentURL.isYoutube && !currentURL.isDuckPlayer { // Navigation outside YouTube - pixelFiring.fire(.duckPlayerYouTubeOverlayNavigationOutsideYoutube, withAdditionalParameters: [:]) + firePixel(.duckPlayerYouTubeOverlayNavigationOutsideYoutube) + navigationHistory.removeAll() } } // Truncation logic: Remove all URLs up to the last occurrence of the current URL in normalized form - if let lastOccurrenceIndex = (0.. 0 { + if let lastOccurrenceIndex = (0..