PM-26896: Fix Autofill ancestry

This commit is contained in:
David Perez 2025-10-13 10:01:24 -05:00
parent 05d6fe1ba1
commit 4bac810c3d
No known key found for this signature in database
GPG Key ID: 3E29BD8B1BF090AC
4 changed files with 56 additions and 84 deletions

View File

@ -13,7 +13,6 @@ import com.x8bit.bitwarden.data.autofill.util.buildUriOrNull
import com.x8bit.bitwarden.data.autofill.util.getInlinePresentationSpecs
import com.x8bit.bitwarden.data.autofill.util.getMaxInlineSuggestionsCount
import com.x8bit.bitwarden.data.autofill.util.toAutofillView
import com.x8bit.bitwarden.data.autofill.util.website
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
import timber.log.Timber
@ -170,7 +169,7 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> =
.mapNotNull { windowNode ->
windowNode
.rootViewNode
?.traverse(parentWebsite = null)
?.traverse()
?.updateForMissingPasswordFields()
?.updateForMissingUsernameFields()
}
@ -248,9 +247,7 @@ private fun ViewNodeTraversalData.copyAndMapAutofillViews(
* Recursively traverse this [AssistStructure.ViewNode] and all of its descendants. Convert the
* data into [ViewNodeTraversalData].
*/
private fun AssistStructure.ViewNode.traverse(
parentWebsite: String?,
): ViewNodeTraversalData {
private fun AssistStructure.ViewNode.traverse(): ViewNodeTraversalData {
// Set up mutable lists for collecting valid AutofillViews and ignorable view ids.
val mutableAutofillViewList: MutableList<AutofillView> = mutableListOf()
val mutableIgnoreAutofillIdList: MutableList<AutofillId> = mutableListOf()
@ -260,7 +257,7 @@ 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)
toAutofillView()
?.run(mutableAutofillViewList::add)
?: autofillId?.run(mutableIgnoreAutofillIdList::add)
@ -268,7 +265,7 @@ private fun AssistStructure.ViewNode.traverse(
for (i in 0 until childCount) {
// Extract the traversal data from each child view node and add it to the lists.
getChildAt(i)
.traverse(parentWebsite = website)
.traverse()
.let { viewNodeTraversalData ->
viewNodeTraversalData.autofillViews.forEach(mutableAutofillViewList::add)
viewNodeTraversalData.ignoreAutofillIds.forEach(mutableIgnoreAutofillIdList::add)

View File

@ -49,9 +49,7 @@ private val AssistStructure.ViewNode.isInputField: Boolean
* doesn't contain a valid autofillId, it isn't an a view setup for autofill, so we return null. If
* it doesn't have a supported hint and isn't an input field, we also return null.
*/
fun AssistStructure.ViewNode.toAutofillView(
parentWebsite: String?,
): AutofillView? {
fun AssistStructure.ViewNode.toAutofillView(): AutofillView? {
val nonNullAutofillId = this.autofillId ?: return null
if (this.supportedAutofillHint == null && !this.isInputField) return null
val autofillOptions = this
@ -65,7 +63,7 @@ fun AssistStructure.ViewNode.toAutofillView(
isFocused = this.isFocused,
textValue = this.autofillValue?.extractTextValue(),
hasPasswordTerms = this.hasPasswordTerms(),
website = this.website ?: parentWebsite,
website = this.website,
)
return buildAutofillView(
autofillOptions = autofillOptions,

View File

@ -183,7 +183,7 @@ class AutofillParserTests {
every { this@mockk.childCount } returns 0
every { this@mockk.idPackage } returns null
every { this@mockk.isFocused } returns false
every { this@mockk.toAutofillView(parentWebsite = any()) } returns null
every { this@mockk.toAutofillView() } returns null
every { this@mockk.website } returns null
}
// `invalidChildViewNode` simulates the OS assigning a node's idPackage to "android", which
@ -196,7 +196,7 @@ class AutofillParserTests {
every { this@mockk.childCount } returns 0
every { this@mockk.idPackage } returns ID_PACKAGE_ANDROID
every { this@mockk.isFocused } returns false
every { this@mockk.toAutofillView(parentWebsite = any()) } returns null
every { this@mockk.toAutofillView() } returns null
every { this@mockk.website } returns null
}
val parentAutofillHint = View.AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_YEAR
@ -217,7 +217,7 @@ class AutofillParserTests {
every { this@mockk.autofillHints } returns arrayOf(parentAutofillHint)
every { this@mockk.autofillId } returns parentAutofillId
every { this@mockk.idPackage } returns null
every { this@mockk.toAutofillView(parentWebsite = any()) } returns parentAutofillView
every { this@mockk.toAutofillView() } returns parentAutofillView
every { this@mockk.childCount } returns 2
every { this@mockk.getChildAt(0) } returns childViewNode
every { this@mockk.getChildAt(1) } returns invalidChildViewNode
@ -303,8 +303,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -366,8 +366,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -422,7 +422,7 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedAutofillView
every { loginViewNode.toAutofillView() } returns unusedAutofillView
// Test
val actual = parser.parse(
@ -525,13 +525,9 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { rootViewNode.toAutofillView(parentWebsite = any()) } returns null
every {
hiddenUserNameViewNode.toAutofillView(parentWebsite = any())
} returns unusedAutofillView
every {
passwordViewNode.toAutofillView(parentWebsite = any())
} returns loginPasswordAutofillView
every { rootViewNode.toAutofillView() } returns null
every { hiddenUserNameViewNode.toAutofillView() } returns unusedAutofillView
every { passwordViewNode.toAutofillView() } returns loginPasswordAutofillView
// Test
val actual = parser.parse(
@ -593,8 +589,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -657,8 +653,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -721,8 +717,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -785,8 +781,8 @@ class AutofillParserTests {
partition = autofillPartition,
uri = URI,
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
// Test
val actual = parser.parse(
@ -841,8 +837,8 @@ class AutofillParserTests {
"blockListedUri.com",
"blockListedAgainUri.com",
)
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
every { cardViewNode.toAutofillView() } returns cardAutofillView
every { loginViewNode.toAutofillView() } returns loginAutofillView
every { settingsRepository.blockedAutofillUris } returns remoteBlockList
// A function for asserting that a block listed URI results in an unfillable request.

View File

@ -97,7 +97,7 @@ class ViewNodeExtensionsTest {
every { viewNode.autofillHints } returns arrayOf(autofillHint)
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -125,7 +125,7 @@ class ViewNodeExtensionsTest {
} returns monthValue
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -141,7 +141,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARD_EXP_MONTH_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -156,7 +156,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARD_EXP_MONTH_HINTS.forEach { idEntry ->
every { viewNode.idEntry } returns idEntry
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for idEntry: $idEntry")
}
@ -171,7 +171,7 @@ class ViewNodeExtensionsTest {
)
SUPPORTED_RAW_CARD_EXP_MONTH_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
}
@ -186,7 +186,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.autofillHints } returns arrayOf(autofillHint)
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -201,7 +201,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARD_EXP_YEAR_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -217,7 +217,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARD_EXP_YEAR_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -231,7 +231,7 @@ class ViewNodeExtensionsTest {
every { viewNode.autofillHints } returns arrayOf(autofillHint)
every { mockHtmlInfo.isInputField } returns true
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -245,7 +245,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARD_EXP_DATE_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -260,7 +260,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARD_EXP_DATE_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -275,7 +275,7 @@ class ViewNodeExtensionsTest {
every { viewNode.autofillHints } returns arrayOf(autofillHint)
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -290,7 +290,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARD_NUMBER_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -304,7 +304,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARD_NUMBER_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -319,7 +319,7 @@ class ViewNodeExtensionsTest {
every { viewNode.autofillHints } returns arrayOf(autofillHint)
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -334,7 +334,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARD_SECURITY_CODE_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -348,7 +348,7 @@ class ViewNodeExtensionsTest {
data = autofillViewData,
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARD_SECURITY_CODE_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -361,7 +361,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARDHOLDER_NAME_HINTS.forEach { idEntry ->
every { viewNode.idEntry } returns idEntry
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for idEntry: $idEntry")
}
@ -376,7 +376,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_CARDHOLDER_NAME_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -390,7 +390,7 @@ class ViewNodeExtensionsTest {
data = autofillViewData,
)
every { viewNode.htmlInfo.hints() } returns SUPPORTED_RAW_CARDHOLDER_NAME_HINTS
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -403,7 +403,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.autofillHints } returns arrayOf(autofillHint)
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -417,7 +417,7 @@ class ViewNodeExtensionsTest {
SUPPORTED_RAW_PASSWORD_HINTS.forEach { hint ->
every { viewNode.hint } returns hint
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual, "Failed for hint: $hint")
}
@ -432,7 +432,7 @@ class ViewNodeExtensionsTest {
)
every { viewNode.htmlInfo.isPasswordField() } returns true
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -451,26 +451,7 @@ class ViewNodeExtensionsTest {
every { any<Int>().isUsernameInputType } returns true
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
// Verify
assertEquals(expected, actual)
}
@Test
fun `toAutofillView should return AutofillView Login Username with external website`() {
// Setup
val website = "website"
val expected = AutofillView.Login.Username(
data = autofillViewData.copy(website = website),
)
setupUnsupportedInputFieldViewNode()
every { viewNode.className } returns "android.widget.EditText"
every { any<Int>().isPasswordInputType } returns false
every { any<Int>().isUsernameInputType } returns true
// Test
val actual = viewNode.toAutofillView(parentWebsite = website)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -489,7 +470,7 @@ class ViewNodeExtensionsTest {
every { any<Int>().isUsernameInputType } returns true
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -508,7 +489,7 @@ class ViewNodeExtensionsTest {
every { any<Int>().isUsernameInputType } returns true
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)
@ -523,7 +504,7 @@ class ViewNodeExtensionsTest {
every { viewNode.htmlInfo.isInputField } returns false
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertNull(actual)
@ -537,7 +518,7 @@ class ViewNodeExtensionsTest {
data = autofillViewData,
)
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
assertEquals(expected, actual)
}
@ -554,7 +535,7 @@ class ViewNodeExtensionsTest {
every { viewNode.autofillHints } returns arrayOf(autofillHintOne, autofillHintTwo)
// Test
val actual = viewNode.toAutofillView(parentWebsite = null)
val actual = viewNode.toAutofillView()
// Verify
assertEquals(expected, actual)