From f03bc0141eb95b6f785d9a80a7f7d869fd64b3f8 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Mon, 14 Oct 2024 16:39:28 +0200 Subject: [PATCH] Add missing auto pixel when Duck Player set to always and navigating from YT thumbnail --- .../app/browser/duckplayer/DuckPlayerJSHelper.kt | 11 ++++++++++- .../duckduckgo/duckplayer/impl/RealDuckPlayer.kt | 14 ++++++-------- .../duckplayer/impl/RealDuckPlayerTest.kt | 16 ++++++++++++++-- 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/app/src/main/java/com/duckduckgo/app/browser/duckplayer/DuckPlayerJSHelper.kt b/app/src/main/java/com/duckduckgo/app/browser/duckplayer/DuckPlayerJSHelper.kt index 82069989f174..41ade5c69aee 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/duckplayer/DuckPlayerJSHelper.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/duckplayer/DuckPlayerJSHelper.kt @@ -37,8 +37,10 @@ import com.duckduckgo.duckplayer.api.DuckPlayer import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED import com.duckduckgo.duckplayer.api.DuckPlayer.UserPreferences import com.duckduckgo.duckplayer.api.ORIGIN_QUERY_PARAM +import com.duckduckgo.duckplayer.api.ORIGIN_QUERY_PARAM_AUTO import com.duckduckgo.duckplayer.api.ORIGIN_QUERY_PARAM_OVERLAY import com.duckduckgo.duckplayer.api.PrivatePlayerMode +import com.duckduckgo.duckplayer.api.PrivatePlayerMode.Enabled import com.duckduckgo.js.messaging.api.JsCallbackData import com.duckduckgo.js.messaging.api.SubscriptionEventData import javax.inject.Inject @@ -204,7 +206,14 @@ class DuckPlayerJSHelper @Inject constructor( } "openDuckPlayer" -> { return data?.getString("href")?.let { - Navigate(it.toUri().buildUpon().appendQueryParameter(ORIGIN_QUERY_PARAM, ORIGIN_QUERY_PARAM_OVERLAY).build().toString(), mapOf()) + if (duckPlayer.getUserPreferences().privatePlayerMode == Enabled) { + Navigate(it.toUri().buildUpon().appendQueryParameter(ORIGIN_QUERY_PARAM, ORIGIN_QUERY_PARAM_AUTO).build().toString(), mapOf()) + } else { + Navigate( + it.toUri().buildUpon().appendQueryParameter(ORIGIN_QUERY_PARAM, ORIGIN_QUERY_PARAM_OVERLAY).build().toString(), + mapOf(), + ) + } } } "initialSetup" -> { diff --git a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/RealDuckPlayer.kt b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/RealDuckPlayer.kt index 31c5160d14f0..65f8f433abc2 100644 --- a/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/RealDuckPlayer.kt +++ b/duckplayer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/RealDuckPlayer.kt @@ -262,7 +262,8 @@ class RealDuckPlayer @Inject constructor( private suspend fun createDuckPlayerUriFromYoutube(uri: Uri): String { val videoIdQueryParam = duckPlayerFeatureRepository.getVideoIDQueryParam() - return "$DUCK_PLAYER_URL_BASE${uri.getQueryParameter(videoIdQueryParam)}?origin=auto" + val origin = uri.getQueryParameter(ORIGIN_QUERY_PARAM)?.let { it } ?: ORIGIN_QUERY_PARAM_AUTO + return "$DUCK_PLAYER_URL_BASE${uri.getQueryParameter(videoIdQueryParam)}?$ORIGIN_QUERY_PARAM=$origin" } override suspend fun intercept( @@ -346,11 +347,6 @@ class RealDuckPlayer @Inject constructor( withContext(dispatchers.main()) { webView.loadUrl(createDuckPlayerUriFromYoutube(url)) } - if (url.getQueryParameter(ORIGIN_QUERY_PARAM) != ORIGIN_QUERY_PARAM_SERP_AUTO) { - pixel.fire(DUCK_PLAYER_VIEW_FROM_YOUTUBE_AUTOMATIC) - } else { - pixel.fire(DUCK_PLAYER_VIEW_FROM_SERP) - } return WebResourceResponse(null, null, null) } return null @@ -398,9 +394,11 @@ class RealDuckPlayer @Inject constructor( webView.loadUrl(youtubeUrl) } val origin = url.getQueryParameter(ORIGIN_QUERY_PARAM) - if (origin == ORIGIN_QUERY_PARAM_SERP) { + if (origin == ORIGIN_QUERY_PARAM_SERP || origin == ORIGIN_QUERY_PARAM_SERP_AUTO) { pixel.fire(DUCK_PLAYER_VIEW_FROM_SERP) - } else if (origin != ORIGIN_QUERY_PARAM_OVERLAY && origin != ORIGIN_QUERY_PARAM_AUTO) { + } else if (origin == ORIGIN_QUERY_PARAM_AUTO) { + pixel.fire(DUCK_PLAYER_VIEW_FROM_YOUTUBE_AUTOMATIC) + } else if (origin != ORIGIN_QUERY_PARAM_OVERLAY) { pixel.fire(DUCK_PLAYER_VIEW_FROM_OTHER) } } diff --git a/duckplayer/duckplayer-impl/src/test/kotlin/com/duckduckgo/duckplayer/impl/RealDuckPlayerTest.kt b/duckplayer/duckplayer-impl/src/test/kotlin/com/duckduckgo/duckplayer/impl/RealDuckPlayerTest.kt index 53c01236aecb..190130935939 100644 --- a/duckplayer/duckplayer-impl/src/test/kotlin/com/duckduckgo/duckplayer/impl/RealDuckPlayerTest.kt +++ b/duckplayer/duckplayer-impl/src/test/kotlin/com/duckduckgo/duckplayer/impl/RealDuckPlayerTest.kt @@ -549,6 +549,20 @@ class RealDuckPlayerTest { assertNotNull(result) } + @Test + fun whenUriIsDuckPlayerUriWithOriginAuto_interceptProcessesDuckPlayerUri() = runTest { + val request: WebResourceRequest = mock() + val url: Uri = Uri.parse("duck://player/12345?origin=auto") + val webView: WebView = mock() + whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + + val result = testee.intercept(request, url, webView) + + verify(webView).loadUrl("https://www.youtube-nocookie.com?videoID=12345") + verify(mockPixel).fire(DUCK_PLAYER_VIEW_FROM_YOUTUBE_AUTOMATIC) + assertNotNull(result) + } + @Test fun whenUriIsDuckPlayerUriWithOpenInYouTube_interceptLoadsYouTubeUri() = runTest { val request: WebResourceRequest = mock() @@ -634,7 +648,6 @@ class RealDuckPlayerTest { val result = testee.intercept(request, url, webView) verify(webView).loadUrl("duck://player/12345?origin=auto") - verify(mockPixel).fire(DUCK_PLAYER_VIEW_FROM_YOUTUBE_AUTOMATIC) assertNotNull(result) } @@ -677,7 +690,6 @@ class RealDuckPlayerTest { val result = testee.intercept(request, url, webView) verify(webView).loadUrl("duck://player/123456?origin=auto") - verify(mockPixel).fire(DUCK_PLAYER_VIEW_FROM_YOUTUBE_AUTOMATIC) assertNotNull(result) }