mirror of
https://github.com/bitwarden/android.git
synced 2026-10-22 02:53:19 -05:00
[PM-39612] fix: resolve autofill container-hint redirect to autofillable child (#7146)
This commit is contained in:
1 parent
05b7e27b96
commit
7a88c4bae6
4 files changed
+229
-7
No files matched your search
@@ -280,6 +280,9 @@ private fun AssistStructure.ViewNode.traverse(
|
||||
// Set up mutable lists for collecting valid AutofillViews and ignorable view ids.
|
||||
val mutableAutofillViewList: MutableList<AutofillView> = mutableListOf()
|
||||
val mutableIgnoreAutofillIdList: MutableList<AutofillId> = mutableListOf()
|
||||
// Tracks autofill IDs already claimed by a non-Unused view so that child nodes whose IDs
|
||||
// were redirected to by a container ancestor are not added a second time.
|
||||
val claimedAutofillIds: MutableSet<AutofillId> = mutableSetOf()
|
||||
// OS sometimes defaults node.idPackage to "android", which is not a valid
|
||||
// package name so it is ignored to prevent auto-filling unknown applications.
|
||||
var storedIdPackage: String? = this.idPackage?.takeUnless { it.isBlank() || it == "android" }
|
||||
@@ -293,7 +296,12 @@ private fun AssistStructure.ViewNode.traverse(
|
||||
// Try converting this `ViewNode` into an `AutofillView`. If a valid instance is returned, add
|
||||
// it to the list. Otherwise, ignore the `AutofillId` associated with this `ViewNode`.
|
||||
toAutofillView(parentWebsite = parentWebsite)
|
||||
?.run(mutableAutofillViewList::add)
|
||||
?.also { view ->
|
||||
if (view !is AutofillView.Unused) {
|
||||
claimedAutofillIds.add(view.data.autofillId)
|
||||
}
|
||||
mutableAutofillViewList.add(view)
|
||||
}
|
||||
?: autofillId?.run(mutableIgnoreAutofillIdList::add)
|
||||
|
||||
// Recursively traverse all of this view node's children.
|
||||
@@ -302,8 +310,23 @@ private fun AssistStructure.ViewNode.traverse(
|
||||
getChildAt(i)
|
||||
.traverse(parentWebsite = website)
|
||||
.let { viewNodeTraversalData ->
|
||||
viewNodeTraversalData.autofillViews.forEach(mutableAutofillViewList::add)
|
||||
viewNodeTraversalData.ignoreAutofillIds.forEach(mutableIgnoreAutofillIdList::add)
|
||||
viewNodeTraversalData.autofillViews
|
||||
// filter out existing AutofillIds to avoid duplicates
|
||||
.filter { view ->
|
||||
val id = view.data.autofillId
|
||||
if (id in claimedAutofillIds) {
|
||||
false
|
||||
} else if (view !is AutofillView.Unused) {
|
||||
claimedAutofillIds.add(id)
|
||||
true
|
||||
} else {
|
||||
true
|
||||
}
|
||||
}
|
||||
.forEach(mutableAutofillViewList::add)
|
||||
viewNodeTraversalData.ignoreAutofillIds
|
||||
.filter { it !in claimedAutofillIds }
|
||||
.forEach(mutableIgnoreAutofillIdList::add)
|
||||
|
||||
// Get the first non-null idPackage.
|
||||
if (storedIdPackage == null) {
|
||||
|
||||
@@ -2,6 +2,7 @@ package com.x8bit.bitwarden.data.autofill.util
|
||||
|
||||
import android.app.assist.AssistStructure
|
||||
import android.view.View
|
||||
import android.view.autofill.AutofillId
|
||||
import android.widget.EditText
|
||||
import androidx.annotation.VisibleForTesting
|
||||
import com.bitwarden.ui.platform.base.util.orNullIfBlank
|
||||
@@ -53,15 +54,32 @@ fun AssistStructure.ViewNode.toAutofillView(
|
||||
parentWebsite: String?,
|
||||
): AutofillView? {
|
||||
val nonNullAutofillId = this.autofillId ?: return null
|
||||
if (this.supportedAutofillHint == null && !this.isInputField) return null
|
||||
val hint = this.supportedAutofillHint
|
||||
val isInput = this.isInputField
|
||||
if (hint == null && !isInput) return null
|
||||
|
||||
// When a container (autofillType=NONE) is classified with a semantic hint, the container
|
||||
// itself cannot receive an autofill value. Redirect to the first autofillable child so the
|
||||
// fill reaches the inner EditText instead of silently failing on the wrapper.
|
||||
// The child's autofillType must also be used — the container's type=0 (NONE) would cause
|
||||
// buildFilledItemOrNull to return null and drop the field from the fill dataset entirely.
|
||||
val autofillableChild =
|
||||
if (hint != null && autofillType == View.AUTOFILL_TYPE_NONE) {
|
||||
findFirstAutofillableChild()
|
||||
} else {
|
||||
null
|
||||
}
|
||||
val effectiveAutofillId = autofillableChild?.autofillId ?: nonNullAutofillId
|
||||
val effectiveAutofillType = autofillableChild?.autofillType ?: autofillType
|
||||
|
||||
val autofillOptions = this
|
||||
.autofillOptions
|
||||
.orEmpty()
|
||||
.map { it.toString() }
|
||||
val autofillViewData = AutofillView.Data(
|
||||
autofillId = nonNullAutofillId,
|
||||
autofillId = effectiveAutofillId,
|
||||
autofillOptions = autofillOptions,
|
||||
autofillType = this.autofillType,
|
||||
autofillType = effectiveAutofillType,
|
||||
isFocused = this.isFocused,
|
||||
textValue = this.autofillValue?.extractTextValue(),
|
||||
hasPasswordTerms = this.hasPasswordTerms(),
|
||||
@@ -70,10 +88,24 @@ fun AssistStructure.ViewNode.toAutofillView(
|
||||
return buildAutofillView(
|
||||
autofillOptions = autofillOptions,
|
||||
autofillViewData = autofillViewData,
|
||||
autofillHint = this.supportedAutofillHint,
|
||||
autofillHint = hint,
|
||||
)
|
||||
}
|
||||
|
||||
private data class AutofillableChild(val autofillId: AutofillId, val autofillType: Int)
|
||||
|
||||
private fun AssistStructure.ViewNode.findFirstAutofillableChild(): AutofillableChild? {
|
||||
for (i in 0 until childCount) {
|
||||
val child = getChildAt(i)
|
||||
if (child.autofillType == View.AUTOFILL_TYPE_TEXT) {
|
||||
val id = child.autofillId ?: continue
|
||||
return AutofillableChild(autofillId = id, autofillType = child.autofillType)
|
||||
}
|
||||
child.findFirstAutofillableChild()?.let { return it }
|
||||
}
|
||||
return null
|
||||
}
|
||||
|
||||
/**
|
||||
* The first supported autofill hint for this view node, or null if none are found.
|
||||
*/
|
||||
|
||||
@@ -947,6 +947,73 @@ class AutofillParserTests {
|
||||
}
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `parse should deduplicate when a child node returns an AutofillView with the same autofillId already claimed by a non-Unused parent view`() {
|
||||
// Setup
|
||||
val sharedAutofillId: AutofillId = mockk()
|
||||
val childViewNode: AssistStructure.ViewNode = mockk {
|
||||
every { this@mockk.childCount } returns 0
|
||||
every { this@mockk.idPackage } returns null
|
||||
every { this@mockk.website } returns null
|
||||
}
|
||||
val parentViewNode: AssistStructure.ViewNode = mockk {
|
||||
every { this@mockk.childCount } returns 1
|
||||
every { this@mockk.getChildAt(0) } returns childViewNode
|
||||
every { this@mockk.idPackage } returns ID_PACKAGE
|
||||
every { this@mockk.website } returns WEBSITE
|
||||
}
|
||||
val windowNode: AssistStructure.WindowNode = mockk {
|
||||
every { this@mockk.rootViewNode } returns parentViewNode
|
||||
}
|
||||
every { assistStructure.windowNodeCount } returns 1
|
||||
every { assistStructure.getWindowNodeAt(0) } returns windowNode
|
||||
|
||||
val sharedAutofillView = AutofillView.Login.Password(
|
||||
data = AutofillView.Data(
|
||||
autofillId = sharedAutofillId,
|
||||
autofillOptions = emptyList(),
|
||||
autofillType = AUTOFILL_TYPE,
|
||||
isFocused = true,
|
||||
textValue = null,
|
||||
hasPasswordTerms = false,
|
||||
website = URI,
|
||||
),
|
||||
)
|
||||
every { parentViewNode.toAutofillView(parentWebsite = any()) } returns sharedAutofillView
|
||||
every { childViewNode.toAutofillView(parentWebsite = any()) } returns sharedAutofillView
|
||||
|
||||
val expected = AutofillRequest.Fillable(
|
||||
ignoreAutofillIds = emptyList(),
|
||||
inlinePresentationSpecs = inlinePresentationSpecs,
|
||||
maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT,
|
||||
packageName = PACKAGE_NAME,
|
||||
partition = AutofillPartition.Login(views = listOf(sharedAutofillView)),
|
||||
uri = URI,
|
||||
)
|
||||
|
||||
// Test
|
||||
val actual = parser.parse(
|
||||
autofillAppInfo = autofillAppInfo,
|
||||
fillRequest = fillRequest,
|
||||
)
|
||||
|
||||
// Verify
|
||||
assertEquals(expected, actual)
|
||||
verify(exactly = 1) {
|
||||
fillRequest.getInlinePresentationSpecs(
|
||||
autofillAppInfo = autofillAppInfo,
|
||||
isInlineAutofillEnabled = true,
|
||||
)
|
||||
fillRequest.getMaxInlineSuggestionsCount(
|
||||
autofillAppInfo = autofillAppInfo,
|
||||
isInlineAutofillEnabled = true,
|
||||
)
|
||||
any<List<ViewNodeTraversalData>>().buildPackageNameOrNull(assistStructure)
|
||||
any<AutofillView>().buildUriOrNull(PACKAGE_NAME)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Setup [assistStructure] to return window nodes with each [AutofillView] type (card and login)
|
||||
* so we can test how different window node configurations produce different partitions.
|
||||
|
||||
@@ -34,10 +34,12 @@ class ViewNodeExtensionsTest {
|
||||
private val testAutofillValue: AutofillValue = mockk()
|
||||
private val mockHtmlInfo: HtmlInfo = mockk {
|
||||
every { attributes } returns emptyList()
|
||||
every { tag } returns ""
|
||||
}
|
||||
|
||||
private val viewNode: AssistStructure.ViewNode = mockk {
|
||||
every { autofillId } returns expectedAutofillId
|
||||
every { className } returns null
|
||||
every { idEntry } returns null
|
||||
every { hint } returns null
|
||||
every { autofillOptions } returns AUTOFILL_OPTIONS_ARRAY
|
||||
@@ -1236,6 +1238,104 @@ class ViewNodeExtensionsTest {
|
||||
assertNull(actual)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `toAutofillView should redirect autofillId and autofillType to first autofillable child when container has AUTOFILL_TYPE_NONE and a supported hint`() {
|
||||
// Setup
|
||||
val childAutofillId: AutofillId = mockk()
|
||||
val childAutofillType = View.AUTOFILL_TYPE_TEXT
|
||||
val childNode: AssistStructure.ViewNode = mockk {
|
||||
every { autofillId } returns childAutofillId
|
||||
every { autofillType } returns childAutofillType
|
||||
every { childCount } returns 0
|
||||
every { className } returns null
|
||||
every { htmlInfo } returns mockHtmlInfo
|
||||
}
|
||||
every { viewNode.autofillHints } returns arrayOf(View.AUTOFILL_HINT_PASSWORD)
|
||||
every { viewNode.autofillType } returns View.AUTOFILL_TYPE_NONE
|
||||
every { viewNode.childCount } returns 1
|
||||
every { viewNode.getChildAt(0) } returns childNode
|
||||
val expected = AutofillView.Login.Password(
|
||||
data = autofillViewData.copy(
|
||||
autofillId = childAutofillId,
|
||||
autofillType = childAutofillType,
|
||||
),
|
||||
)
|
||||
|
||||
// Test
|
||||
val actual = viewNode.toAutofillView(parentWebsite = null)
|
||||
|
||||
// Verify
|
||||
assertEquals(expected, actual)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `toAutofillView should redirect to nested grandchild when direct child has AUTOFILL_TYPE_NONE`() {
|
||||
// Setup
|
||||
val grandchildAutofillId: AutofillId = mockk()
|
||||
val grandchildAutofillType = View.AUTOFILL_TYPE_TEXT
|
||||
val grandchildNode: AssistStructure.ViewNode = mockk {
|
||||
every { autofillId } returns grandchildAutofillId
|
||||
every { autofillType } returns grandchildAutofillType
|
||||
every { childCount } returns 0
|
||||
every { className } returns null
|
||||
every { htmlInfo } returns mockHtmlInfo
|
||||
}
|
||||
val intermediateChildNode: AssistStructure.ViewNode = mockk {
|
||||
every { autofillId } returns mockk()
|
||||
every { autofillType } returns View.AUTOFILL_TYPE_NONE
|
||||
every { childCount } returns 1
|
||||
every { getChildAt(0) } returns grandchildNode
|
||||
every { className } returns null
|
||||
every { htmlInfo } returns mockHtmlInfo
|
||||
}
|
||||
every { viewNode.autofillHints } returns arrayOf(View.AUTOFILL_HINT_PASSWORD)
|
||||
every { viewNode.autofillType } returns View.AUTOFILL_TYPE_NONE
|
||||
every { viewNode.childCount } returns 1
|
||||
every { viewNode.getChildAt(0) } returns intermediateChildNode
|
||||
val expected = AutofillView.Login.Password(
|
||||
data = autofillViewData.copy(
|
||||
autofillId = grandchildAutofillId,
|
||||
autofillType = grandchildAutofillType,
|
||||
),
|
||||
)
|
||||
|
||||
// Test
|
||||
val actual = viewNode.toAutofillView(parentWebsite = null)
|
||||
|
||||
// Verify
|
||||
assertEquals(expected, actual)
|
||||
}
|
||||
|
||||
@Suppress("MaxLineLength")
|
||||
@Test
|
||||
fun `toAutofillView should fall back to container own autofillId when AUTOFILL_TYPE_NONE and hint present but no autofillable child exists`() {
|
||||
// Setup
|
||||
val nonAutofillableChildNode: AssistStructure.ViewNode = mockk {
|
||||
every { autofillId } returns mockk()
|
||||
every { autofillType } returns View.AUTOFILL_TYPE_NONE
|
||||
every { childCount } returns 0
|
||||
every { className } returns null
|
||||
every { htmlInfo } returns mockHtmlInfo
|
||||
}
|
||||
every { viewNode.autofillHints } returns arrayOf(View.AUTOFILL_HINT_PASSWORD)
|
||||
every { viewNode.autofillType } returns View.AUTOFILL_TYPE_NONE
|
||||
every { viewNode.childCount } returns 1
|
||||
every { viewNode.getChildAt(0) } returns nonAutofillableChildNode
|
||||
val expected = AutofillView.Login.Password(
|
||||
data = autofillViewData.copy(
|
||||
autofillType = View.AUTOFILL_TYPE_NONE,
|
||||
),
|
||||
)
|
||||
|
||||
// Test
|
||||
val actual = viewNode.toAutofillView(parentWebsite = null)
|
||||
|
||||
// Verify
|
||||
assertEquals(expected, actual)
|
||||
}
|
||||
|
||||
/**
|
||||
* Set up [viewNode] to be an input field but not supported.
|
||||
*/
|
||||
|
||||
Reference in new issue
Block a user