diff --git a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt index 4b181fc67087..5ce7625ce68c 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/browser/BrowserTabViewModelTest.kt @@ -1218,6 +1218,46 @@ class BrowserTabViewModelTest { assertCommandIssued() } + @Test + fun whenRefreshRequestedWithQuerySearchThenFireQueryChangePixelZero() { + loadUrl("query") + + testee.onRefreshRequested() + + verify(mockPixel).fire("rq_0") + } + + @Test + fun whenRefreshRequestedWithUrlThenDoNotFireQueryChangePixel() { + loadUrl("https://example.com") + + testee.onRefreshRequested() + + verify(mockPixel, never()).fire("rq_0") + } + + @Test + fun whenUserSubmittedQueryWithPreviousBlankQueryThenDoNotSendQueryChangePixel() { + whenever(mockOmnibarConverter.convertQueryToUrl("another query", null)).thenReturn("another query") + loadUrl("") + + testee.onUserSubmittedQuery("another query") + + verify(mockPixel, never()).fire("rq_0") + verify(mockPixel, never()).fire("rq_1") + } + + @Test + fun whenUserSubmittedQueryWithDifferentPreviousQueryThenSendQueryChangePixel() { + whenever(mockOmnibarConverter.convertQueryToUrl("another query", null)).thenReturn("another query") + loadUrl("query") + + testee.onUserSubmittedQuery("another query") + + verify(mockPixel, never()).fire("rq_0") + verify(mockPixel).fire("rq_1") + } + @Test fun whenUserBrowsingPressesBackAndBrowserCanGoBackThenNavigatesToPreviousPageAndHandledTrue() { setupNavigation(isBrowsing = true, canGoBack = true, stepsToPreviousPage = 2) diff --git a/app/src/androidTest/java/com/duckduckgo/app/global/api/ApiRequestInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/global/api/ApiRequestInterceptorTest.kt index a57fdb27f81b..80bbee7a3bdf 100644 --- a/app/src/androidTest/java/com/duckduckgo/app/global/api/ApiRequestInterceptorTest.kt +++ b/app/src/androidTest/java/com/duckduckgo/app/global/api/ApiRequestInterceptorTest.kt @@ -16,75 +16,51 @@ package com.duckduckgo.app.global.api +import android.webkit.WebSettings import androidx.test.platform.app.InstrumentationRegistry -import okhttp3.* +import com.duckduckgo.app.browser.useragent.UserAgentProvider +import com.duckduckgo.app.global.device.ContextDeviceInfo import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test -import java.util.concurrent.TimeUnit class ApiRequestInterceptorTest { private lateinit var testee: ApiRequestInterceptor - private val fakeChain: Interceptor.Chain = FakeChain() + private lateinit var userAgentProvider: UserAgentProvider @Before fun before() { - testee = ApiRequestInterceptor(InstrumentationRegistry.getInstrumentation().context) + userAgentProvider = UserAgentProvider( + WebSettings.getDefaultUserAgent(InstrumentationRegistry.getInstrumentation().context), + ContextDeviceInfo(InstrumentationRegistry.getInstrumentation().context) + ) + + testee = ApiRequestInterceptor( + InstrumentationRegistry.getInstrumentation().context, + userAgentProvider + ) } @Test fun whenAPIRequestIsMadeThenUserAgentIsAdded() { val packageName = InstrumentationRegistry.getInstrumentation().context.applicationInfo.packageName - val response = testee.intercept(fakeChain) + val response = testee.intercept(FakeChain("http://example.com")) val regex = "ddg_android/.*\\($packageName; Android API .*\\)".toRegex() val result = response.request.header(Header.USER_AGENT)!! assertTrue(result.matches(regex)) } - // implement just request and proceed methods - private class FakeChain : Interceptor.Chain { - override fun call(): Call { - TODO("Not yet implemented") - } - - override fun connectTimeoutMillis(): Int { - TODO("Not yet implemented") - } - - override fun connection(): Connection? { - TODO("Not yet implemented") - } - - override fun proceed(request: Request): Response { - return Response.Builder().request(request).protocol(Protocol.HTTP_2).code(200).message("").build() - } - - override fun readTimeoutMillis(): Int { - TODO("Not yet implemented") - } - - override fun request(): Request { - return Request.Builder().url("http://example.com").build() - } - - override fun withConnectTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { - TODO("Not yet implemented") - } - - override fun withReadTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { - TODO("Not yet implemented") - } - - override fun withWriteTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { - TODO("Not yet implemented") - } + @Test + fun whenAPIRequestIsRqPixelThenOverrideHeader() { + val fakeChain = FakeChain("https://improving.duckduckgo.com/t/rq_0") - override fun writeTimeoutMillis(): Int { - TODO("Not yet implemented") - } + val response = testee.intercept(fakeChain) + val header = response.request.header(Header.USER_AGENT)!! + val regex = "Mozilla/.* \\(Linux; Android.*\\) AppleWebKit/.* \\(KHTML, like Gecko\\) Version/.* Chrome/.* Mobile DuckDuckGo/.* Safari/.*".toRegex() + assertTrue(header.matches(regex)) } } diff --git a/app/src/androidTest/java/com/duckduckgo/app/global/api/FakeChain.kt b/app/src/androidTest/java/com/duckduckgo/app/global/api/FakeChain.kt new file mode 100644 index 000000000000..92837736b429 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/global/api/FakeChain.kt @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2021 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.global.api + +import okhttp3.* +import java.util.concurrent.TimeUnit + +class FakeChain(private val url: String) : Interceptor.Chain { + override fun call(): Call { + TODO("Not yet implemented") + } + + override fun connectTimeoutMillis(): Int { + TODO("Not yet implemented") + } + + override fun connection(): Connection? { + TODO("Not yet implemented") + } + + override fun proceed(request: Request): Response { + return Response.Builder().request(request).protocol(Protocol.HTTP_2).code(200).message("").build() + } + + override fun readTimeoutMillis(): Int { + TODO("Not yet implemented") + } + + override fun request(): Request { + return Request.Builder().url(url).build() + } + + override fun withConnectTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { + TODO("Not yet implemented") + } + + override fun withReadTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { + TODO("Not yet implemented") + } + + override fun withWriteTimeout(timeout: Int, unit: TimeUnit): Interceptor.Chain { + TODO("Not yet implemented") + } + + override fun writeTimeoutMillis(): Int { + TODO("Not yet implemented") + } +} diff --git a/app/src/androidTest/java/com/duckduckgo/app/global/api/PixelReQueryInterceptorTest.kt b/app/src/androidTest/java/com/duckduckgo/app/global/api/PixelReQueryInterceptorTest.kt new file mode 100644 index 000000000000..cd60c40bff56 --- /dev/null +++ b/app/src/androidTest/java/com/duckduckgo/app/global/api/PixelReQueryInterceptorTest.kt @@ -0,0 +1,86 @@ +/* + * Copyright (c) 2021 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.global.api + +import okhttp3.HttpUrl.Companion.toHttpUrl +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test + +class PixelReQueryInterceptorTest { + + private lateinit var pixelReQueryInterceptor: PixelReQueryInterceptor + + @Before + fun setup() { + pixelReQueryInterceptor = PixelReQueryInterceptor() + } + + @Test + fun whenRq0PixelIsSendThenRemoveDeviceAndFormFactor() { + assertEquals( + EXPECTED_RQ_0_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(RQ_0_PHONE_URL)).request.url + ) + + assertEquals( + EXPECTED_RQ_0_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(RQ_0_TABLET_URL)).request.url + ) + } + + @Test + fun whenRq1PixelIsSendThenRemoveDeviceAndFormFactor() { + assertEquals( + EXPECTED_RQ_1_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(RQ_1_PHONE_URL)).request.url + ) + + assertEquals( + EXPECTED_RQ_1_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(RQ_1_TABLET_URL)).request.url + ) + } + + @Test + fun whenPixelOtherThanRqIsSendThenDoNotModify() { + assertEquals( + EXPECTED_OTHER_PIXEL_PHONE_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(OTHER_PIXEL_PHONE_URL)).request.url + ) + + assertEquals( + EXPECTED_OTHER_PIXEL_TABLET_URL.toHttpUrl(), + pixelReQueryInterceptor.intercept(FakeChain(OTHER_PIXEL_TABLET_URL)).request.url + ) + + } + + private companion object { + private const val RQ_0_PHONE_URL = "https://improving.duckduckgo.com/t/rq_0_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val RQ_0_TABLET_URL = "https://improving.duckduckgo.com/t/rq_0_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val RQ_1_PHONE_URL = "https://improving.duckduckgo.com/t/rq_1_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val RQ_1_TABLET_URL = "https://improving.duckduckgo.com/t/rq_1_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val OTHER_PIXEL_PHONE_URL = "https://improving.duckduckgo.com/t/my_pixel_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val OTHER_PIXEL_TABLET_URL = "https://improving.duckduckgo.com/t/my_pixel_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1" + + private const val EXPECTED_RQ_0_URL = "https://improving.duckduckgo.com/t/rq_0?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val EXPECTED_RQ_1_URL = "https://improving.duckduckgo.com/t/rq_1?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val EXPECTED_OTHER_PIXEL_PHONE_URL = "https://improving.duckduckgo.com/t/my_pixel_android_phone?atb=v255-7zu&appVersion=5.74.0&test=1" + private const val EXPECTED_OTHER_PIXEL_TABLET_URL = "https://improving.duckduckgo.com/t/my_pixel_android_tablet?atb=v255-7zu&appVersion=5.74.0&test=1" + } +} diff --git a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt index e41c4c8e9b69..004219282cb4 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/BrowserTabViewModel.kt @@ -20,6 +20,7 @@ import android.annotation.SuppressLint import android.graphics.Bitmap import android.net.Uri import android.os.Message +import android.util.Patterns import android.view.ContextMenu import android.view.MenuItem import android.view.View @@ -548,7 +549,7 @@ class BrowserTabViewModel( if (oldQuery == newQuery) { pixel.fire(String.format(Locale.US, PixelName.SERP_REQUERY.pixelName, PixelParameter.SERP_QUERY_NOT_CHANGED)) - } else { + } else if (oldQuery.toString().isNotBlank()) { // blank means no previous search, don't send pixel pixel.fire(String.format(Locale.US, PixelName.SERP_REQUERY.pixelName, PixelParameter.SERP_QUERY_CHANGED)) } } @@ -626,6 +627,10 @@ class BrowserTabViewModel( } fun onRefreshRequested() { + val omnibarContent = currentOmnibarViewState().omnibarText + if (!Patterns.WEB_URL.matcher(omnibarContent).matches()) { + fireQueryChangedPixel(currentOmnibarViewState().omnibarText) + } navigationAwareLoginDetector.onEvent(NavigationEvent.UserAction.Refresh) if (currentGlobalLayoutState() is Invalidated) { recoverTabWithQuery(url.orEmpty()) diff --git a/app/src/main/java/com/duckduckgo/app/browser/omnibar/KeyboardAwareEditText.kt b/app/src/main/java/com/duckduckgo/app/browser/omnibar/KeyboardAwareEditText.kt index 7acf108cba03..64312e207a69 100644 --- a/app/src/main/java/com/duckduckgo/app/browser/omnibar/KeyboardAwareEditText.kt +++ b/app/src/main/java/com/duckduckgo/app/browser/omnibar/KeyboardAwareEditText.kt @@ -18,7 +18,9 @@ package com.duckduckgo.app.browser.omnibar import android.content.Context import android.graphics.Rect +import android.text.Editable import android.util.AttributeSet +import android.util.Patterns import android.view.KeyEvent import androidx.appcompat.widget.AppCompatEditText import com.duckduckgo.app.global.view.showKeyboard @@ -33,10 +35,24 @@ class KeyboardAwareEditText : AppCompatEditText { constructor(context: Context, attrs: AttributeSet?) : this(context, attrs, 0) constructor(context: Context, attrs: AttributeSet?, defStyleAttr: Int) : super(context, attrs, defStyleAttr) + private var didFocusAlready = false + + private fun Editable.isWebUrl(): Boolean { + return Patterns.WEB_URL.matcher(this.toString()).matches() + } + override fun onFocusChanged(focused: Boolean, direction: Int, previouslyFocusedRect: Rect?) { super.onFocusChanged(focused, direction, previouslyFocusedRect) + setSelectAllOnFocus(!didFocusAlready) if (focused) { + if (didFocusAlready && text != null && text?.isWebUrl() == false) { + // trigger the text change listener so that we can show autocomplete + text = text + // cursor at the end of the word + setSelection(text!!.length) + } showKeyboard() + didFocusAlready = true } } diff --git a/app/src/main/java/com/duckduckgo/app/di/NetworkModule.kt b/app/src/main/java/com/duckduckgo/app/di/NetworkModule.kt index c5980418f834..f5fdf35d523b 100644 --- a/app/src/main/java/com/duckduckgo/app/di/NetworkModule.kt +++ b/app/src/main/java/com/duckduckgo/app/di/NetworkModule.kt @@ -21,13 +21,13 @@ import androidx.work.WorkManager import com.duckduckgo.app.autocomplete.api.AutoCompleteService import com.duckduckgo.app.brokensite.api.BrokenSiteSender import com.duckduckgo.app.brokensite.api.BrokenSiteSubmitter +import com.duckduckgo.app.browser.useragent.UserAgentProvider import com.duckduckgo.app.feedback.api.FeedbackService import com.duckduckgo.app.feedback.api.FeedbackSubmitter import com.duckduckgo.app.feedback.api.FireAndForgetFeedbackSubmitter import com.duckduckgo.app.feedback.api.SubReasonApiMapper import com.duckduckgo.app.global.AppUrl.Url -import com.duckduckgo.app.global.api.ApiRequestInterceptor -import com.duckduckgo.app.global.api.NetworkApiCache +import com.duckduckgo.app.global.api.* import com.duckduckgo.app.global.job.AppConfigurationSyncWorkRequestBuilder import com.duckduckgo.app.globalprivacycontrol.GlobalPrivacyControl import com.duckduckgo.app.httpsupgrade.api.HttpsUpgradeService @@ -72,9 +72,13 @@ class NetworkModule { @Provides @Singleton @Named("nonCaching") - fun pixelOkHttpClient(apiRequestInterceptor: ApiRequestInterceptor): OkHttpClient { + fun pixelOkHttpClient( + apiRequestInterceptor: ApiRequestInterceptor, + pixelReQueryInterceptor: PixelReQueryInterceptor, + ): OkHttpClient { return OkHttpClient.Builder() .addInterceptor(apiRequestInterceptor) + .addInterceptor(pixelReQueryInterceptor) .build() } @@ -105,8 +109,16 @@ class NetworkModule { } @Provides - fun apiRequestInterceptor(context: Context): ApiRequestInterceptor { - return ApiRequestInterceptor(context) + fun apiRequestInterceptor( + context: Context, + userAgentProvider: UserAgentProvider + ): ApiRequestInterceptor { + return ApiRequestInterceptor(context, userAgentProvider) + } + + @Provides + fun pixelReQueryInterceptor(): PixelReQueryInterceptor { + return PixelReQueryInterceptor() } @Provides diff --git a/app/src/main/java/com/duckduckgo/app/global/api/ApiRequestInterceptor.kt b/app/src/main/java/com/duckduckgo/app/global/api/ApiRequestInterceptor.kt index acc9313363ca..70b87319c557 100644 --- a/app/src/main/java/com/duckduckgo/app/global/api/ApiRequestInterceptor.kt +++ b/app/src/main/java/com/duckduckgo/app/global/api/ApiRequestInterceptor.kt @@ -18,21 +18,31 @@ package com.duckduckgo.app.global.api import android.content.Context import com.duckduckgo.app.browser.BuildConfig +import com.duckduckgo.app.browser.useragent.UserAgentProvider +import com.duckduckgo.app.global.AppUrl import okhttp3.Interceptor import okhttp3.Response -class ApiRequestInterceptor(context: Context) : Interceptor { +class ApiRequestInterceptor( + context: Context, + private val userAgentProvider: UserAgentProvider +) : Interceptor { private val userAgent: String by lazy { "ddg_android/${BuildConfig.VERSION_NAME} (${context.applicationInfo.packageName}; Android API ${android.os.Build.VERSION.SDK_INT})" } override fun intercept(chain: Interceptor.Chain): Response { - val request = chain.request() - .newBuilder() - .addHeader(Header.USER_AGENT, userAgent) - .build() + val request = chain.request().newBuilder() - return chain.proceed(request) + val url = chain.request().url + if (url.toString().startsWith("${AppUrl.Url.PIXEL}/t/rq_")) { + // user agent for re-query pixels needs to be the same for the webview + request.addHeader(Header.USER_AGENT, userAgentProvider.userAgent()) + } else { + request.addHeader(Header.USER_AGENT, userAgent) + } + + return chain.proceed(request.build()) } } diff --git a/app/src/main/java/com/duckduckgo/app/global/api/PixelReQueryInterceptor.kt b/app/src/main/java/com/duckduckgo/app/global/api/PixelReQueryInterceptor.kt new file mode 100644 index 000000000000..12426551bc77 --- /dev/null +++ b/app/src/main/java/com/duckduckgo/app/global/api/PixelReQueryInterceptor.kt @@ -0,0 +1,47 @@ +/* + * Copyright (c) 2021 DuckDuckGo + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.duckduckgo.app.global.api + +import com.duckduckgo.app.global.device.DeviceInfo +import okhttp3.HttpUrl.Companion.toHttpUrl +import okhttp3.Interceptor +import okhttp3.Response +import timber.log.Timber + +/** + * This interceptor fixes the re-query pixels without being invasive to the Pixel APIs. + * When the search box was removed, the rq pixels were moved into the mobile app, however ALL pixels sent + * from the mobile app have _android_{formFactor} appended to the pixel name, and this broke all existing + * monitoring and dashboards. + * + * This interceptor removes the _android_{formFactor} appended suffix + */ +class PixelReQueryInterceptor : Interceptor { + override fun intercept(chain: Interceptor.Chain): Response { + var url = chain.request().url + val request = chain.request().newBuilder() + + url = url.toUrl().toString().replace("rq_0_android_${DeviceInfo.FormFactor.PHONE.description}", "rq_0").toHttpUrl() + url = url.toUrl().toString().replace("rq_0_android_${DeviceInfo.FormFactor.TABLET.description}", "rq_0").toHttpUrl() + url = url.toUrl().toString().replace("rq_1_android_${DeviceInfo.FormFactor.PHONE.description}", "rq_1").toHttpUrl() + url = url.toUrl().toString().replace("rq_1_android_${DeviceInfo.FormFactor.TABLET.description}", "rq_1").toHttpUrl() + + Timber.d("Pixel interceptor: $url") + + return chain.proceed(request.url(url).build()) + } +}