From 67ab3ab0cbf7c4098299c36110481892910fd765 Mon Sep 17 00:00:00 2001
From: Daniel Bernal <dbernal@duckduckgo.com>
Date: Mon, 23 Dec 2024 17:32:03 +0100
Subject: [PATCH] Fix Youtube Player Links

---
 .../DuckPlayerNavigationHandler.swift         | 13 ++++++-----
 .../DuckPlayerNavigationHandling.swift        | 13 ++++++++---
 ...YoutublePlayerNavigationHandlerTests.swift | 22 ++++++++++++++++++-
 3 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
index 82c9215c9f..8b65df97a8 100644
--- a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
+++ b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandler.swift
@@ -438,11 +438,11 @@ final class DuckPlayerNavigationHandler: NSObject {
         let referrerValue = queryItems.first(where: { $0.name == Constants.duckPlayerReferrerParameter })?.value
         let allowFirstVideoValue = queryItems.first(where: { $0.name == Constants.allowFirstVideoParameter })?.value
         let isNewTabValue = queryItems.first(where: { $0.name == Constants.newTabParameter })?.value
-        let youtubeEmbedURI = queryItems.first(where: { $0.name == Constants.youtubeEmbedURI })?.value
+        let youtubeEmbedURI = queryItems.first(where: { $0.name == Constants.youtubeEmbedURI })?.value ?? ""
         
         // Use the from(string:) method to parse referrer
         let referrer = DuckPlayerReferrer(string: referrerValue ?? "")
-        let allowFirstVideo = allowFirstVideoValue == "1" || youtubeEmbedURI.map(\.isEmpty) ?? false
+        let allowFirstVideo = allowFirstVideoValue == "1" || !youtubeEmbedURI.isEmpty
         let isNewTab = isNewTabValue == "1"
         
         return DuckPlayerParameters(referrer: referrer, isNewTap: isNewTab, allowFirstVideo: allowFirstVideo)
@@ -720,16 +720,19 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
         
         guard videoID != lastWatchInYoutubeVideo else {
             lastURLChangeHandling = Date()
-            return .handled
+            return .handled(.newVideo)
         }
         
         let parameters = getDuckPlayerParameters(url: url)
         
+        // If this is an internal Youtube Link (i.e Clicking in youtube logo in the player)
+        // Do not handle it
+        
         // If the URL has the allow first video, we just don't handle it
         if parameters.allowFirstVideo {
             lastWatchInYoutubeVideo = videoID
             lastURLChangeHandling = Date()
-            return .handled
+            return .handled(.allowFirstVideo)
         }
         
         guard duckPlayerMode == .enabled else {
@@ -743,7 +746,7 @@ extension DuckPlayerNavigationHandler: DuckPlayerNavigationHandling {
             })
             lastURLChangeHandling = Date()
             Logger.duckPlayer.debug("Handling URL change for \(webView.url?.absoluteString ?? "")")
-            return .handled
+            return .handled(.duckPlayerEnabled)
         } else {
             
         }
diff --git a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift
index 168512c112..a0d50303b6 100644
--- a/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift
+++ b/DuckDuckGo/DuckPlayer/DuckPlayerNavigationHandling.swift
@@ -42,7 +42,7 @@ extension DuckPlayerReferrer {
 enum DuckPlayerNavigationHandlerURLChangeResult {
     
     /// Possible reasons for not handling a URL change.
-    enum HandlingResult {
+    enum NotHandledResult {
         case featureOff
         case invalidURL
         case duckPlayerDisabled
@@ -50,9 +50,16 @@ enum DuckPlayerNavigationHandlerURLChangeResult {
         case disabledForVideo
         case duplicateNavigation
     }
+    
+    /// Possible reasons for handling a URL change.
+    enum HandledResult {
+        case newVideo
+        case allowFirstVideo
+        case duckPlayerEnabled
+    }
 
-    case handled
-    case notHandled(HandlingResult)
+    case handled(HandledResult)
+    case notHandled(NotHandledResult)
 }
 
 /// Represents the direction of navigation in the Duck Player.
diff --git a/DuckDuckGoTests/YoutublePlayerNavigationHandlerTests.swift b/DuckDuckGoTests/YoutublePlayerNavigationHandlerTests.swift
index c804b5047f..05ec1bd5ca 100644
--- a/DuckDuckGoTests/YoutublePlayerNavigationHandlerTests.swift
+++ b/DuckDuckGoTests/YoutublePlayerNavigationHandlerTests.swift
@@ -283,7 +283,7 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
         let result2 = handler.handleURLChange(webView: mockWebView)
 
         // Assert
-        if case .handled = result1 {
+        if case .handled(.duckPlayerEnabled) = result1 {
             // Success
         } else {
             XCTFail("Expected first call to return .handled")
@@ -371,6 +371,26 @@ class DuckPlayerNavigationHandlerTests: XCTestCase {
         }
     }
     
+    @MainActor
+    func testHandleURLChange_WithYoutubeEmbedURIParam_ReturnsHandled() async {
+        // Arrange
+        let youtubeURL = URL(string: "https://www.youtube.com/watch?v=abc123&&embeds_referring_euri=true")!
+        mockWebView.setCurrentURL(youtubeURL)
+        playerSettings.mode = .enabled
+        featureFlagger.enabledFeatures = [.duckPlayer, .duckPlayerOpenInNewTab]
+
+        // Act
+        let result = handler.handleURLChange(webView: mockWebView)
+        
+        // Assert
+        if case .handled(.allowFirstVideo) = result {
+            // Success
+        } else {
+            XCTFail("Expected first call to return .handled")
+        }
+    }
+
+    
     @MainActor
     func testHandleDelegateNavigation_NotToMainFrame_ReturnsFalse() async {
         // Arrange