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
duckduckgo · Jan 10, 2025 · 1eed968 · 1eed968
1 parent e632d53
commit 1eed968
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 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
@@ -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<HttpAuthHandler>()
         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
@@ -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
@@ -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/...ayer/duckplayer-impl/src/test/kotlin/com/duckduckgo/duckplayer/impl/RealDuckPlayerTest.kt b/...ayer/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,20 +662,35 @@ 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)
 
         verify(webView).loadUrl("duck://player/12345")
         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()
         val url: Uri = Uri.parse("https://www.youtube.com/watch?v=12345")
         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,19 +720,35 @@ 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)
 
         verify(webView).loadUrl("duck://player/123456")
         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)