Shared queries into the app containing apostrophes better handled (#4966

)

Task/Issue URL:
https://app.asana.com/0/1208134428464537/1207998553475892/f

### Description
Currently any text shared into the app (e.g., using `Search DuckDuckGo`
from elsewhere) doesn't handle the case where apostrophes are in it
(might also do it with some other special characters).

This fix addresses that. In case of any unintended side-effects,
`androidSkipUrlConversionOnNewTab` remote feature flag could be created
and set to `false` to revert back to the old behaviour.

### Steps to test this PR

- [x] Follow the testing steps in the linked task
- [x] Plus as much manual testing as you can, especially for different
types being shared into the app (plain text, plain text + special
characters, links)

app/src/main/java/com/duckduckgo/app/browser/BrowserViewModel.kt

@@ -20,6 +20,7 @@ import androidx.lifecycle.LiveData
 import androidx.lifecycle.MutableLiveData
 import androidx.lifecycle.Observer
 import androidx.lifecycle.ViewModel
+import com.duckduckgo.anvil.annotations.ContributesRemoteFeature
 import com.duckduckgo.anvil.annotations.ContributesViewModel
 import com.duckduckgo.app.browser.defaultbrowsing.DefaultBrowserDetector
 import com.duckduckgo.app.browser.omnibar.OmnibarEntryConverter
@@ -49,6 +50,8 @@ import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.common.utils.DispatcherProvider
 import com.duckduckgo.common.utils.SingleLiveEvent
 import com.duckduckgo.di.scopes.ActivityScope
+import com.duckduckgo.di.scopes.AppScope
+import com.duckduckgo.feature.toggles.api.Toggle
 import javax.inject.Inject
 import kotlin.coroutines.CoroutineContext
 import kotlinx.coroutines.CoroutineScope
@@ -65,6 +68,7 @@ class BrowserViewModel @Inject constructor(
     private val defaultBrowserDetector: DefaultBrowserDetector,
     private val dispatchers: DispatcherProvider,
     private val pixel: Pixel,
+    private val skipUrlConversionOnNewTabFeature: SkipUrlConversionOnNewTabFeature,
 ) : ViewModel(),
     CoroutineScope {
 
@@ -145,15 +149,21 @@ class BrowserViewModel @Inject constructor(
         sourceTabId: String? = null,
         skipHome: Boolean = false,
     ): String {
+        val url = if (skipUrlConversionOnNewTabFeature.self().isEnabled()) {
+            query
+        } else {
+            queryUrlConverter.convertQueryToUrl(query)
+        }
+
         return if (sourceTabId != null) {
             tabRepository.addFromSourceTab(
-                url = queryUrlConverter.convertQueryToUrl(query),
+                url = url,
                 skipHome = skipHome,
                 sourceTabId = sourceTabId,
             )
         } else {
             tabRepository.add(
-                url = queryUrlConverter.convertQueryToUrl(query),
+                url = url,
                 skipHome = skipHome,
             )
         }
@@ -275,3 +285,18 @@ class BrowserViewModel @Inject constructor(
         command.value = Command.OpenSavedSite(url)
     }
 }
+
+/**
+ * Feature flag to skip converting the query to a URL when opening a new tab
+ * This is for fixing https://app.asana.com/0/1208134428464537/1207998553475892/f
+ *
+ * In case of unexpected side-effects, the old behaviour can be reverted by disabling this remote feature flag
+ */
+@ContributesRemoteFeature(
+    scope = AppScope::class,
+    featureName = "androidSkipUrlConversionOnNewTab",
+)
+interface SkipUrlConversionOnNewTabFeature {
+    @Toggle.DefaultValue(true)
+    fun self(): Toggle
+}

app/src/test/java/com/duckduckgo/app/browser/BrowserViewModelTest.kt

@@ -33,6 +33,8 @@ import com.duckduckgo.app.statistics.pixels.Pixel.PixelParameter
 import com.duckduckgo.app.tabs.model.TabEntity
 import com.duckduckgo.app.tabs.model.TabRepository
 import com.duckduckgo.common.test.CoroutineTestRule
+import com.duckduckgo.feature.toggles.api.FakeFeatureToggleFactory
+import com.duckduckgo.feature.toggles.api.Toggle.State
 import kotlinx.coroutines.test.runTest
 import org.junit.After
 import org.junit.Assert.assertEquals
@@ -81,12 +83,16 @@ class BrowserViewModelTest {
 
     private lateinit var testee: BrowserViewModel
 
+    private val skipUrlConversionOnNewTabFeature = FakeFeatureToggleFactory.create(SkipUrlConversionOnNewTabFeature::class.java)
+
     @Before
     fun before() {
         MockitoAnnotations.openMocks(this)
 
         doReturn(MutableLiveData<AppEnjoymentPromptOptions>()).whenever(mockAppEnjoymentPromptEmitter).promptType
 
+        configureSkipUrlConversionInNewTabState(enabled = true)
+
         testee = BrowserViewModel(
             tabRepository = mockTabRepository,
             queryUrlConverter = mockOmnibarEntryConverter,
@@ -96,6 +102,7 @@ class BrowserViewModelTest {
             defaultBrowserDetector = mockDefaultBrowserDetector,
             dispatchers = coroutinesTestRule.testDispatcherProvider,
             pixel = mockPixel,
+            skipUrlConversionOnNewTabFeature = skipUrlConversionOnNewTabFeature,
         )
 
         testee.command.observeForever(mockCommandObserver)
@@ -246,6 +253,24 @@ class BrowserViewModelTest {
         assertEquals(Command.OpenSavedSite(bookmarkUrl), commandCaptor.lastValue)
     }
 
+    @Test
+    fun whenOpenInNewTabWithSkipUrlConversionEnabledThenQueryNotConverted() = runTest {
+        configureSkipUrlConversionInNewTabState(enabled = true)
+        testee.onOpenInNewTabRequested(query = "query")
+        verify(mockOmnibarEntryConverter, never()).convertQueryToUrl("query")
+    }
+
+    @Test
+    fun whenOpenInNewTabWithSkipUrlConversionDisabledThenQueryConverted() = runTest {
+        configureSkipUrlConversionInNewTabState(enabled = false)
+        testee.onOpenInNewTabRequested(query = "query")
+        verify(mockOmnibarEntryConverter).convertQueryToUrl("query")
+    }
+
+    private fun configureSkipUrlConversionInNewTabState(enabled: Boolean) {
+        skipUrlConversionOnNewTabFeature.self().setEnabled(State(enable = enabled))
+    }
+
     companion object {
         const val TAB_ID = "TAB_ID"
     }