From 4bac810c3d1a5ee01b9d72279a9a6cbf41bb386b Mon Sep 17 00:00:00 2001 From: David Perez Date: Mon, 13 Oct 2025 10:01:24 -0500 Subject: [PATCH] PM-26896: Fix Autofill ancestry --- .../autofill/parser/AutofillParserImpl.kt | 11 +-- .../data/autofill/util/ViewNodeExtensions.kt | 6 +- .../autofill/parser/AutofillParserTests.kt | 46 +++++------ .../autofill/util/ViewNodeExtensionsTest.kt | 77 +++++++------------ 4 files changed, 56 insertions(+), 84 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt index a35b1b9959c..5fea681f910 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt @@ -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 = .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 = mutableListOf() val mutableIgnoreAutofillIdList: MutableList = 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) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt index 5d80e86fc8f..23d9c14bb1d 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt @@ -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, diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt index cd214fb8cd2..247888a46b4 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt @@ -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. diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt index 2fec4139eba..35f7173aee5 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensionsTest.kt @@ -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().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().isPasswordInputType } returns false - every { any().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().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().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)