Set open in new tab to true by default

duckduckgo · Oct 14, 2024 · 1912aef · 1912aef
1 parent 98f68f3
commit 1912aef
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 21 deletions.
diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt
@@ -191,6 +191,9 @@ import com.duckduckgo.downloads.api.FileDownloader.PendingFileDownload
 import com.duckduckgo.duckplayer.api.DuckPlayer
 import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.DISABLED
 import com.duckduckgo.duckplayer.api.DuckPlayer.DuckPlayerState.ENABLED
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Off
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Unavailable
 import com.duckduckgo.duckplayer.api.DuckPlayer.UserPreferences
 import com.duckduckgo.duckplayer.api.PrivatePlayerMode.AlwaysAsk
 import com.duckduckgo.duckplayer.api.PrivatePlayerMode.Disabled
@@ -5050,10 +5053,10 @@ class BrowserTabViewModelTest {
     }
 
     @Test
-    fun whenProcessJsCallbackMessageOpenDuckPlayerWithUrlAndOpenInNewTabFalseThenNavigate() = runTest {
+    fun whenProcessJsCallbackMessageOpenDuckPlayerWithUrlAndOpenInNewTabOffThenNavigate() = runTest {
         whenever(mockEnabledToggle.isEnabled()).thenReturn(true)
         whenever(mockDuckPlayer.getUserPreferences()).thenReturn(UserPreferences(overlayInteracted = true, privatePlayerMode = AlwaysAsk))
-        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(false)
+        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(Off)
         testee.processJsCallbackMessage(
             DUCK_PLAYER_FEATURE_NAME,
             "openDuckPlayer",
@@ -5065,10 +5068,25 @@ class BrowserTabViewModelTest {
     }
 
     @Test
-    fun whenProcessJsCallbackMessageOpenDuckPlayerWithUrlAndOpenInNewTabTrueThenOpenInNewTab() = runTest {
+    fun whenProcessJsCallbackMessageOpenDuckPlayerWithUrlAndOpenInNewTabUnavailableThenNavigate() = runTest {
         whenever(mockEnabledToggle.isEnabled()).thenReturn(true)
         whenever(mockDuckPlayer.getUserPreferences()).thenReturn(UserPreferences(overlayInteracted = true, privatePlayerMode = AlwaysAsk))
-        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(true)
+        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(Unavailable)
+        testee.processJsCallbackMessage(
+            DUCK_PLAYER_FEATURE_NAME,
+            "openDuckPlayer",
+            "id",
+            JSONObject("""{ href: "duck://player/1234" }"""),
+            { "someUrl" },
+        )
+        assertCommandIssued<Navigate>()
+    }
+
+    @Test
+    fun whenProcessJsCallbackMessageOpenDuckPlayerWithUrlAndOpenInNewTabOnThenOpenInNewTab() = runTest {
+        whenever(mockEnabledToggle.isEnabled()).thenReturn(true)
+        whenever(mockDuckPlayer.getUserPreferences()).thenReturn(UserPreferences(overlayInteracted = true, privatePlayerMode = AlwaysAsk))
+        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(On)
         testee.processJsCallbackMessage(
             DUCK_PLAYER_FEATURE_NAME,
             "openDuckPlayer",

diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserWebViewClientTest.kt
@@ -71,6 +71,10 @@ import com.duckduckgo.common.utils.device.DeviceInfo
 import com.duckduckgo.common.utils.plugins.PluginPoint
 import com.duckduckgo.cookies.api.CookieManagerProvider
 import com.duckduckgo.duckplayer.api.DuckPlayer
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Off
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Unavailable
 import com.duckduckgo.experiments.api.loadingbarexperiment.LoadingBarExperimentManager
 import com.duckduckgo.history.api.NavigationHistory
 import com.duckduckgo.privacy.config.api.AmpLinks
@@ -143,7 +147,7 @@ class BrowserWebViewClientTest {
     private val navigationHistory: NavigationHistory = mock()
     private val loadingBarExperimentManager: LoadingBarExperimentManager = mock()
     private val mockDuckDuckGoUrlDetector: DuckDuckGoUrlDetector = mock()
-    private val openInNewTabFlow: MutableSharedFlow<Boolean> = MutableSharedFlow()
+    private val openInNewTabFlow: MutableSharedFlow<OpenDuckPlayerInNewTab> = MutableSharedFlow()
 
     @UiThreadTest
     @Before
@@ -404,7 +408,7 @@ class BrowserWebViewClientTest {
 
     @UiThreadTest
     @Test
-    fun whenShouldOverrideWithShouldNavigateToDuckPlayerFromSerpThenAddQueryParam() {
+    fun whenShouldOverrideWithShouldNavigateToDuckPlayerFromSerpThenAddQueryParam() = runTest {
         val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink("duck://player/1234".toUri())
         whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType)
         whenever(webResourceRequest.isForMainFrame).thenReturn(true)
@@ -417,6 +421,7 @@ class BrowserWebViewClientTest {
         doNothing().whenever(listener).willOverrideUrl(any())
         val mockWebView = getImmediatelyInvokedMockWebView()
         whenever(mockWebView.url).thenReturn("www.duckduckgo.com")
+        openInNewTabFlow.emit(Off)
 
         assertTrue(testee.shouldOverrideUrlLoading(mockWebView, webResourceRequest))
         verify(mockWebView).loadUrl("www.youtube.com/watch?v=1234&origin=serp_auto")
@@ -470,11 +475,11 @@ class BrowserWebViewClientTest {
     @UiThreadTest
     @Test
     fun whenShouldLaunchDuckPlayerThenOpenInNewTabAndReturnTrue() = runTest {
-        openInNewTabFlow.emit(true)
+        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(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(true)
+        whenever(mockDuckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(On)
 
         assertTrue(testee.shouldOverrideUrlLoading(webView, webResourceRequest))
         verify(listener).onShouldOverride()
@@ -484,7 +489,20 @@ class BrowserWebViewClientTest {
     @UiThreadTest
     @Test
     fun whenShouldLaunchDuckPlayerButNotOpenInNewTabThenReturnFalse() = runTest {
-        openInNewTabFlow.emit(false)
+        openInNewTabFlow.emit(Off)
+        val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink(EXAMPLE_URL.toUri())
+        whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType)
+        whenever(webResourceRequest.url).thenReturn(EXAMPLE_URL.toUri())
+
+        assertFalse(testee.shouldOverrideUrlLoading(webView, webResourceRequest))
+        verify(listener).onShouldOverride()
+        verify(listener, never()).openLinkInNewTab(EXAMPLE_URL.toUri())
+    }
+
+    @UiThreadTest
+    @Test
+    fun whenShouldLaunchDuckPlayerButOpenInNewTabUnavailableThenReturnFalse() = runTest {
+        openInNewTabFlow.emit(Unavailable)
         val urlType = SpecialUrlDetector.UrlType.ShouldLaunchDuckPlayerLink(EXAMPLE_URL.toUri())
         whenever(specialUrlDetector.determineType(initiatingUrl = any(), uri = any())).thenReturn(urlType)
         whenever(webResourceRequest.url).thenReturn(EXAMPLE_URL.toUri())

diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserWebViewClient.kt
@@ -127,7 +127,7 @@ class BrowserWebViewClient @Inject constructor(
     private var lastPageStarted: String? = null
     private var start: Long? = null
 
-    private var shouldOpenDuckPlayerInNewTab: Boolean = false
+    private var shouldOpenDuckPlayerInNewTab: Boolean = true
 
     init {
         appCoroutineScope.launch {
@@ -205,11 +205,7 @@ class BrowserWebViewClient @Inject constructor(
                         }
                         return true
                     } else {
-                        if (shouldOpenDuckPlayerInNewTab) {
-                            shouldOverrideWebRequest(url, webView, isForMainFrame, openInNewTab = true)
-                        } else {
-                            shouldOverrideWebRequest(url, webView, isForMainFrame, openInNewTab = false)
-                        }
+                        shouldOverrideWebRequest(url, webView, isForMainFrame, openInNewTab = shouldOpenDuckPlayerInNewTab)
                     }
                 }
                 is SpecialUrlDetector.UrlType.NonHttpAppLink -> {

diff --git a/app/src/test/java/com/duckduckgo/duckplayer/impl/DuckPlayerSettingsViewModelTest.kt b/app/src/test/java/com/duckduckgo/duckplayer/impl/DuckPlayerSettingsViewModelTest.kt
@@ -3,6 +3,8 @@ package com.duckduckgo.duckplayer.impl
 import app.cash.turbine.test
 import com.duckduckgo.app.statistics.pixels.Pixel
 import com.duckduckgo.common.test.CoroutineTestRule
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.Off
+import com.duckduckgo.duckplayer.api.DuckPlayer.OpenDuckPlayerInNewTab.On
 import com.duckduckgo.duckplayer.api.DuckPlayer.UserPreferences
 import com.duckduckgo.duckplayer.api.PrivatePlayerMode.AlwaysAsk
 import com.duckduckgo.duckplayer.api.PrivatePlayerMode.Disabled
@@ -16,7 +18,6 @@ import kotlinx.coroutines.flow.MutableSharedFlow
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.test.runTest
 import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
@@ -43,7 +44,7 @@ class DuckPlayerSettingsViewModelTest {
             whenever(duckPlayer.observeUserPreferences()).thenReturn(userPreferencesFlow)
             userPreferencesFlow.emit(UserPreferences(overlayInteracted = true, privatePlayerMode = AlwaysAsk))
             whenever(duckPlayer.getUserPreferences()).thenReturn(UserPreferences(overlayInteracted = true, privatePlayerMode = AlwaysAsk))
-            whenever(duckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(false)
+            whenever(duckPlayer.shouldOpenDuckPlayerInNewTab()).thenReturn(Off)
             viewModel = DuckPlayerSettingsViewModel(duckPlayer, duckPlayerFeatureRepository, pixel)
         }
     }
@@ -84,7 +85,7 @@ class DuckPlayerSettingsViewModelTest {
     @Test
     fun whenViewModelIsCreatedAndPrivatePlayerModeIsDisabledThenEmitDisabled() = runTest {
         whenever(duckPlayer.observeUserPreferences()).thenReturn(flowOf(UserPreferences(overlayInteracted = true, privatePlayerMode = Disabled)))
-        whenever(duckPlayer.observeShouldOpenInNewTab()).thenReturn(flowOf(true))
+        whenever(duckPlayer.observeShouldOpenInNewTab()).thenReturn(flowOf(On))
         userPreferencesFlow.emit(UserPreferences(overlayInteracted = true, privatePlayerMode = Disabled))
         viewModel = DuckPlayerSettingsViewModel(duckPlayer, duckPlayerFeatureRepository, pixel)
 
@@ -96,14 +97,14 @@ class DuckPlayerSettingsViewModelTest {
     @Test
     fun whenViewModelIsCreatedAndPrivatePlayerModeIsAlwaysAndOpenInNewTabThenEmitAlwaysAndOpenInNewTab() = runTest {
         whenever(duckPlayer.observeUserPreferences()).thenReturn(flowOf(UserPreferences(overlayInteracted = true, privatePlayerMode = Enabled)))
-        whenever(duckPlayer.observeShouldOpenInNewTab()).thenReturn(flowOf(true))
+        whenever(duckPlayer.observeShouldOpenInNewTab()).thenReturn(flowOf(On))
         userPreferencesFlow.emit(UserPreferences(overlayInteracted = true, privatePlayerMode = Enabled))
         viewModel = DuckPlayerSettingsViewModel(duckPlayer, duckPlayerFeatureRepository, pixel)
 
         viewModel.viewState.test {
             awaitItem().let {
                 assertEquals(Enabled, it.privatePlayerMode)
-                assertTrue(it.openDuckPlayerInNewTab)
+                assertEquals(On, it.openDuckPlayerInNewTab)
             }
         }
     }

diff --git a/...layer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt b/...layer/duckplayer-impl/src/main/java/com/duckduckgo/duckplayer/impl/DuckPlayerDataStore.kt
@@ -194,7 +194,7 @@ class SharedPreferencesDuckPlayerDataStore @Inject constructor(
     private val duckPlayerOpenInNewTab: Flow<Boolean>
         get() = store.data
             .map { prefs ->
-                prefs[Keys.DUCK_PLAYER_OPEN_IN_NEW_TAB] ?: false
+                prefs[Keys.DUCK_PLAYER_OPEN_IN_NEW_TAB] ?: true
             }
             .distinctUntilChanged()