Skip to content

Commit

Permalink
Suppress system AutofillService save prompts to prevent it overlappin…
Browse files Browse the repository at this point in the history
…g in-browser save prompts (#3840)

<!--
Note: This checklist is a reminder of our shared engineering
expectations.
The items in Bold are required
If your PR involves UI changes:
1. Upload screenshots or screencasts that illustrate the changes before
/ after
2. Add them under the UI changes section (feel free to add more columns
if needed)
If your PR does not involve UI changes, you can remove the **UI
changes** section

At a minimum, make sure your changes are tested in API 23 and one of the
more recent API levels available.
-->

Task/Issue URL:
https://app.asana.com/0/608920331025315/1205657473475434/f

### Description
Prevent an overlap occurring between the system AutofillService save
prompt and the in-browser save prompt. Note, this isn't fool proof and
there is at least one known site where it doesn't prevent both showing,
but it's a UX improvement on majority of sites.

### Steps to test this PR

To test this properly, ensure your Android version is >= Android 8.0
(API 26) and that you have a system-level Autofill provider set up
(otherwise you would never have seen the system prompt anyway, so the
suppression isn't being tested). You can verify this here:
- Visit settings, languages & input, autofill service (or you might have
to search for it in settings if OEM has put it somewhere else)

<img
src="https://github.com/duckduckgo/Android/assets/1336281/75ca068b-549d-4cca-a79a-f029af76b86e"
width="30%" />

<hr />

**Steps**
- [x] Install `internal` app build
- [x] Visit https://fill.dev/form/login-simple; enter username, password
and tap login button
- [x] Verify you only see our save prompt, and not the system (e.g.,
Google Password Manager) save prompt

### UI changes


![combined](https://github.com/duckduckgo/Android/assets/1336281/645e6943-7c64-47fd-88b1-f6dfedd64e5c)
  • Loading branch information
CDRussell committed Nov 10, 2023
1 parent 8c80da7 commit 5354e7f
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import com.duckduckgo.autofill.impl.jsbridge.request.SupportedAutofillTriggerTyp
import com.duckduckgo.autofill.impl.jsbridge.request.SupportedAutofillTriggerType.USER_INITIATED
import com.duckduckgo.autofill.impl.jsbridge.response.AutofillResponseWriter
import com.duckduckgo.autofill.impl.sharedcreds.ShareableCredentials
import com.duckduckgo.autofill.impl.systemautofill.SystemAutofillServiceSuppressor
import com.duckduckgo.autofill.impl.ui.credential.passwordgeneration.Actions
import com.duckduckgo.autofill.impl.ui.credential.passwordgeneration.Actions.DeleteAutoLogin
import com.duckduckgo.autofill.impl.ui.credential.passwordgeneration.Actions.DiscardAutoLoginId
Expand Down Expand Up @@ -110,6 +111,7 @@ class AutofillStoredBackJavascriptInterface @Inject constructor(
private val inContextDataStore: EmailProtectionInContextDataStore,
private val recentInstallChecker: EmailProtectionInContextRecentInstallChecker,
private val loginDeduplicator: AutofillLoginDeduplicator,
private val systemAutofillServiceSuppressor: SystemAutofillServiceSuppressor,
) : AutofillJavascriptInterface {

override var callback: Callback? = null
Expand Down Expand Up @@ -248,6 +250,9 @@ class AutofillStoredBackJavascriptInterface @Inject constructor(

@JavascriptInterface
fun storeFormData(data: String) {
// important to call suppressor as soon as possible
systemAutofillServiceSuppressor.suppressAutofill(webView)

Timber.i("storeFormData called, credentials provided to be persisted")

storeFormDataJob += coroutineScope.launch(dispatcherProvider.default()) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/*
* Copyright (c) 2023 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.autofill.impl.systemautofill

import android.annotation.SuppressLint
import android.os.Build
import android.view.autofill.AutofillManager
import android.webkit.WebView
import com.duckduckgo.appbuildconfig.api.AppBuildConfig
import com.duckduckgo.di.scopes.AppScope
import com.squareup.anvil.annotations.ContributesBinding
import javax.inject.Inject

interface SystemAutofillServiceSuppressor {
fun suppressAutofill(webView: WebView?)
}

@ContributesBinding(AppScope::class)
class RealSystemAutofillServiceSuppressor @Inject constructor(
private val appBuildConfig: AppBuildConfig,
) : SystemAutofillServiceSuppressor {

@SuppressLint("NewApi")
override fun suppressAutofill(webView: WebView?) {
if (appBuildConfig.versionCode >= Build.VERSION_CODES.O) {
webView?.context?.getSystemService(AutofillManager::class.java)?.cancel()
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import com.duckduckgo.autofill.impl.jsbridge.request.SupportedAutofillInputSubTy
import com.duckduckgo.autofill.impl.jsbridge.request.SupportedAutofillTriggerType.USER_INITIATED
import com.duckduckgo.autofill.impl.jsbridge.response.AutofillResponseWriter
import com.duckduckgo.autofill.impl.sharedcreds.ShareableCredentials
import com.duckduckgo.autofill.impl.systemautofill.SystemAutofillServiceSuppressor
import com.duckduckgo.autofill.impl.ui.credential.passwordgeneration.Actions
import com.duckduckgo.autofill.impl.ui.credential.passwordgeneration.AutogeneratedPasswordEventResolver
import kotlinx.coroutines.CoroutineScope
Expand Down Expand Up @@ -77,6 +78,7 @@ class AutofillStoredBackJavascriptInterfaceTest {
private val recentInstallChecker: EmailProtectionInContextRecentInstallChecker = mock()
private val testWebView = WebView(getApplicationContext())
private val loginDeduplicator: AutofillLoginDeduplicator = NoopDeduplicator()
private val systemAutofillServiceSuppressor: SystemAutofillServiceSuppressor = mock()
private lateinit var testee: AutofillStoredBackJavascriptInterface

private val testCallback = TestCallback()
Expand All @@ -102,6 +104,7 @@ class AutofillStoredBackJavascriptInterfaceTest {
inContextDataStore = inContextDataStore,
recentInstallChecker = recentInstallChecker,
loginDeduplicator = loginDeduplicator,
systemAutofillServiceSuppressor = systemAutofillServiceSuppressor,
)
testee.callback = testCallback
testee.webView = testWebView
Expand Down

0 comments on commit 5354e7f

Please sign in to comment.