From 1eed9680899e291c7344fecb3856b8036a706c64 Mon Sep 17 00:00:00 2001 From: Cris Barreiro Date: Fri, 10 Jan 2025 17:27:12 +0100 Subject: [PATCH] Only navigate to Duck Player if YT was loaded in mainframe (#5440) Task/Issue URL: https://app.asana.com/0/1203249713006009/1209098737366580/f ### Description Only load Duck Player if YouTube requests come from mainFrame Do not open Duck Player in new tab if the original link already opened a new tab ### Steps to test this PR _Feature 1_ - [ ] Set Duck Player to always, and open in new tab - [ ] Open https://www.academycinemas.co.nz/ - [ ] Check Duck Player isn't opened _Feature 2_ - [ ] Set Duck Player to always, and open in new tab - [ ] Open a video in Duck Player, navigate to the end of it - [ ] Click on the recommendation for next video - [ ] Check a new tab is opened and Duck Player is loaded correctly _Feature 3_ - [ ] Set Duck Player to always, and open in new tab - [ ] Navigate to https://kottke.org/24/12/louis-armstrong-reads-twas-the-night-before-christmas - [ ] Click on the title of the embedded video - [ ] Check a new tab is opened and Duck Player is loaded correctly _Feature 4_ - [ ] Smoke test Duck Player --- .../app/browser/BrowserWebViewClientTest.kt | 65 +++++++++++++++++-- .../app/browser/BrowserWebViewClient.kt | 8 +-- .../duckplayer/impl/RealDuckPlayer.kt | 2 + .../duckplayer/impl/RealDuckPlayerTest.kt | 40 ++++++++++++ 4 files changed, 107 insertions(+), 8 deletions(-) diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt index 3061968b86f1..36f10ce114d4 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt @@ -114,7 +114,7 @@ class BrowserWebViewClientTest { var coroutinesTestRule = CoroutineTestRule() private lateinit var testee: BrowserWebViewClient - private lateinit var webView: WebView + private lateinit var webView: TestWebView private val context = InstrumentationRegistry.getInstrumentation().targetContext private val requestRewriter: RequestRewriter = mock() @@ -319,6 +319,7 @@ class BrowserWebViewClientTest { fun whenOnReceivedHttpAuthRequestThenListenerNotified() { val mockHandler = mock() val authenticationRequest = BasicAuthenticationRequest(mockHandler, "example.com", EXAMPLE_URL, EXAMPLE_URL) + webView.webViewUrl = EXAMPLE_URL testee.onReceivedHttpAuthRequest(webView, mockHandler, "example.com", EXAMPLE_URL) verify(listener).requiresAuthentication(authenticationRequest) } @@ -435,6 +436,26 @@ class BrowserWebViewClientTest { verify(mockDuckPlayer).setDuckPlayerOrigin(SERP_AUTO) } + @UiThreadTest + @Test + fun whenShouldOverrideWithShouldNavigateToDuckPlayerButNotMainFrameDoNothing() = runTest { + val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink("duck://player/1234".toUri()) + whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType) + whenever(webResourceRequest.isForMainFrame).thenReturn(false) + whenever(webResourceRequest.isRedirect).thenReturn(false) + whenever(webResourceRequest.url).thenReturn("www.youtube.com/watch?v=1234".toUri()) + whenever(mockDuckDuckGoUrlDetector.isDuckDuckGoUrl(any())).thenReturn(true) + val mockClientProvider: ClientBrandHintProvider = mock() + whenever(mockClientProvider.shouldChangeBranding(any())).thenReturn(false) + testee.clientProvider = mockClientProvider + doNothing().whenever(listener).willOverrideUrl(any()) + val mockWebView = getImmediatelyInvokedMockWebView() + whenever(mockWebView.url).thenReturn("www.duckduckgo.com") + openInNewTabFlow.emit(Off) + + assertFalse(testee.shouldOverrideUrlLoading(mockWebView, webResourceRequest)) + } + @Test fun whenShouldOverrideWithWebThenDoNotAddQueryParam() = runTest { val urlType = Web("www.youtube.com/watch?v=1234") @@ -506,13 +527,46 @@ class BrowserWebViewClientTest { val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink(EXAMPLE_URL.toUri()) whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType) whenever(webResourceRequest.url).thenReturn(EXAMPLE_URL.toUri()) + whenever(webResourceRequest.isForMainFrame).thenReturn(true) whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(On) + testee.clientProvider = mock() assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) verify(listener).onShouldOverride() verify(listener).openLinkInNewTab(EXAMPLE_URL.toUri()) } + @UiThreadTest + @Test + fun whenShouldLaunchDuckPlayerInNewTabButSameUrlThenDoNothing() = runTest { + openInNewTabFlow.emit(On) + val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink(EXAMPLE_URL.toUri()) + whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType) + whenever(webResourceRequest.url).thenReturn(EXAMPLE_URL.toUri()) + whenever(webResourceRequest.isForMainFrame).thenReturn(true) + whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(On) + testee.clientProvider = mock() + (webView as TestWebView).webViewUrl = EXAMPLE_URL + + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener, never()).openLinkInNewTab(EXAMPLE_URL.toUri()) + } + + @UiThreadTest + @Test + fun whenShouldLaunchDuckPlayerButNotMainframeThenDoNothing() = runTest { + openInNewTabFlow.emit(On) + val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink(EXAMPLE_URL.toUri()) + whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType) + whenever(webResourceRequest.url).thenReturn(EXAMPLE_URL.toUri()) + whenever(webResourceRequest.isForMainFrame).thenReturn(false) + whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(On) + + assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest)) + verify(listener).onShouldOverride() + verify(listener, never()).openLinkInNewTab(EXAMPLE_URL.toUri()) + } + @UiThreadTest @Test fun whenShouldLaunchDuckPlayerButNotOpenInNewTabThenReturnFalse() = runTest { @@ -1072,11 +1126,14 @@ class BrowserWebViewClientTest { } private class TestWebView(context: Context) : WebView(context) { - override fun getOriginalUrl(): String { - return EXAMPLE_URL + + var webViewUrl: String? = null + + override fun getUrl(): String? { + return webViewUrl } - override fun getUrl(): String { + override fun getOriginalUrl(): String { return EXAMPLE_URL } } diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt index a0cb6de7cda9..f8ba3fb39761 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt @@ -200,7 +200,7 @@ class BrowserWebViewClient @Inject constructor( false } is SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink -> { - if (isRedirect) { + if (isRedirect && isForMainFrame) { /* This forces shouldInterceptRequest to be called with the YouTube URL, otherwise that method is never executed and therefore the Duck Player page is never launched if YouTube comes from a redirect. @@ -214,8 +214,8 @@ class BrowserWebViewClient @Inject constructor( url, webView, isForMainFrame, - openInNewTab = shouldOpenDuckPlayerInNewTab, - willOpenDuckPlayer = true, + openInNewTab = shouldOpenDuckPlayerInNewTab && isForMainFrame && webView.url != url.toString(), + willOpenDuckPlayer = isForMainFrame, ) } } @@ -352,7 +352,7 @@ class BrowserWebViewClient @Inject constructor( return false } } else if (openInNewTab) { - webViewClientListener?.openLinkInNewTab(url) + listener.openLinkInNewTab(url) return true } else { val headers = androidFeaturesHeaderPlugin.getHeaders(url.toString()) 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 0fcda51138cd..607a918a7341 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 @@ -362,6 +362,8 @@ class RealDuckPlayer @Inject constructor( url: Uri, webView: WebView, ): WebResourceResponse? { + if (!request.isForMainFrame) return null + val currentUrl = withContext(dispatchers.main()) { webView.url } val videoIdQueryParam = duckPlayerFeatureRepository.getVideoIDQueryParam() 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 fceb894a79ae..2040672ccb70 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 @@ -519,6 +519,7 @@ class RealDuckPlayerTest { val request: WebResourceRequest = mock() val url: Uri = mock() val webView: WebView = mock() + whenever(request.isForMainFrame).thenReturn(true) setFeatureToggle(false) @@ -532,6 +533,7 @@ class RealDuckPlayerTest { val request: WebResourceRequest = mock() val url: Uri = Uri.parse("https://www.notmatching.com") val webView: WebView = mock() + whenever(request.isForMainFrame).thenReturn(true) setFeatureToggle(true) @@ -546,6 +548,7 @@ class RealDuckPlayerTest { val url: Uri = Uri.parse("duck://player/12345") val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -561,6 +564,7 @@ class RealDuckPlayerTest { val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) testee.setDuckPlayerOrigin(AUTO) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -575,6 +579,7 @@ class RealDuckPlayerTest { val url: Uri = Uri.parse("duck://player/openInYouTube?v=12345") val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -605,6 +610,7 @@ class RealDuckPlayerTest { val url: Uri = Uri.parse("duck://player/12345") val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Disabled)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -624,6 +630,7 @@ class RealDuckPlayerTest { whenever(context.assets).thenReturn(mockAssets) whenever(mockAssets.open(any())).thenReturn(mock()) whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -642,6 +649,7 @@ class RealDuckPlayerTest { val request: WebResourceRequest = mock() val url: Uri = Uri.parse("https://www.youtube-nocookie.com?videoID=12345") val webView: WebView = mock() + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -654,6 +662,7 @@ class RealDuckPlayerTest { val url: Uri = Uri.parse("https://www.youtube.com/watch?v=12345") val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -661,6 +670,19 @@ class RealDuckPlayerTest { assertNotNull(result) } + @Test + fun whenUriIsYoutubeWatchUrlButNotMainframe_interceptDoesNothing() = runTest { + val request: WebResourceRequest = mock() + val url: Uri = Uri.parse("https://www.youtube.com/watch?v=12345") + val webView: WebView = mock() + whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(request.isForMainFrame).thenReturn(false) + + val result = testee.intercept(request, url, webView) + + assertNull(result) + } + @Test fun whenUriIsYoutubeWatchUrlAndPreviousUrlIsDuckPlayer_interceptReturnsNull() = runTest { val request: WebResourceRequest = mock() @@ -668,6 +690,7 @@ class RealDuckPlayerTest { val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) whenever(webView.url).thenReturn("https://www.youtube-nocookie.com?videoID=12345") + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -683,6 +706,7 @@ class RealDuckPlayerTest { val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) whenever(webView.url).thenReturn("https://www.youtube-nocookie.com?videoID=12345") + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -696,6 +720,7 @@ class RealDuckPlayerTest { val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) whenever(webView.url).thenReturn("https://www.youtube-nocookie.com?videoID=12345") + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView) @@ -703,12 +728,27 @@ class RealDuckPlayerTest { assertNotNull(result) } + @Test + fun whenUriIsYoutubeWatchUrlAndPreviousUrlIsDuckPlayerWithDifferentIdAndNotMainFrame_interceptDoesNothing() = runTest { + val request: WebResourceRequest = mock() + val url: Uri = Uri.parse("https://www.youtube.com/watch?v=123456") + val webView: WebView = mock() + whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, Enabled)) + whenever(webView.url).thenReturn("https://www.youtube-nocookie.com?videoID=12345") + whenever(request.isForMainFrame).thenReturn(false) + + val result = testee.intercept(request, url, webView) + + assertNull(result) + } + @Test fun whenUriIsYoutubeWatchUrlAndSettingsAlwaysAsk_interceptProcessesYoutubeWatchUri() = runTest { val request: WebResourceRequest = mock() val url: Uri = Uri.parse("https://www.youtube.com/watch?v=12345") val webView: WebView = mock() whenever(mockDuckPlayerFeatureRepository.getUserPreferences()).thenReturn(UserPreferences(true, AlwaysAsk)) + whenever(request.isForMainFrame).thenReturn(true) val result = testee.intercept(request, url, webView)