From f15821ec179becf50f3eecdc0d0a37766c162255 Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Thu, 23 Jul 2026 11:36:25 +0100 Subject: [PATCH 1/6] Add model and data layer for Identity Autofill --- .../bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt | 2 +- .../bitwarden/data/autofill/parser/AutofillParserImpl.kt | 5 +---- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index da12e597298..e7a23226d32 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -94,7 +94,7 @@ class FilledDataBuilderImpl( is AutofillPartition.Identity -> { // Filling an identity partition is wired up in a later phase; this is a no-op - // today since an identity partition is never constructed yet. + // today since nothing yet classifies a view as Identity. emptyList() } } 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 aa857474820..ffb7226b1cd 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 @@ -551,10 +551,7 @@ private fun AutofillView.updateWebsiteIfNecessary(website: String?): AutofillVie } is AutofillView.Identity.PhoneFull -> this.copy(data = this.data.copy(website = site)) - is AutofillView.Identity.PostalAddressFull -> { - this.copy(data = this.data.copy(website = site)) - } - + is AutofillView.Identity.PostalAddressFull -> this.copy(data = this.data.copy(website = site)) is AutofillView.Identity.PostalCode -> this.copy(data = this.data.copy(website = site)) is AutofillView.Identity.Ssn -> this.copy(data = this.data.copy(website = site)) is AutofillView.Unused -> this.copy(data = this.data.copy(website = site)) From 1b553285106851e443b54050508b562db2e621de Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Thu, 23 Jul 2026 17:41:24 +0100 Subject: [PATCH 2/6] Add heuristic detection for identity autofill fields --- .../bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index e7a23226d32..da12e597298 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -94,7 +94,7 @@ class FilledDataBuilderImpl( is AutofillPartition.Identity -> { // Filling an identity partition is wired up in a later phase; this is a no-op - // today since nothing yet classifies a view as Identity. + // today since an identity partition is never constructed yet. emptyList() } } From d972e7a9c218eec8d53d89aa01bfe6ebbff7f8b1 Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Tue, 28 Jul 2026 13:42:54 +0100 Subject: [PATCH 3/6] Build and fill identity autofill partitions --- .../autofill/builder/FilledDataBuilderImpl.kt | 151 +++-- .../autofill/parser/AutofillParserImpl.kt | 61 +- .../autofill/builder/FilledDataBuilderTest.kt | 101 ++++ .../autofill/parser/AutofillParserTests.kt | 554 ++++++++++++++---- 4 files changed, 691 insertions(+), 176 deletions(-) diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt index da12e597298..7f6de559581 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderImpl.kt @@ -59,45 +59,10 @@ class FilledDataBuilderImpl( } ?.also { inlineSuggestionsAdded += 1 } - val filledPartitions = when (autofillRequest.partition) { - is AutofillPartition.Card -> { - autofillCipherProvider - .getCardAutofillCiphers() - .map { autofillCipher -> - fillCardPartition( - autofillCipher = autofillCipher, - autofillViews = autofillRequest.partition.views, - inlinePresentationSpec = getCipherInlinePresentationOrNull(), - ) - } - } - - is AutofillPartition.Login -> { - autofillRequest - .uri - ?.let { nonNullUri -> - autofillCipherProvider - .getLoginAutofillCiphers( - uri = nonNullUri, - ) - .map { autofillCipher -> - fillLoginPartition( - autofillCipher = autofillCipher, - autofillViews = autofillRequest.partition.views, - inlinePresentationSpec = getCipherInlinePresentationOrNull(), - packageName = autofillRequest.packageName, - ) - } - } - .orEmpty() - } - - is AutofillPartition.Identity -> { - // Filling an identity partition is wired up in a later phase; this is a no-op - // today since an identity partition is never constructed yet. - emptyList() - } - } + val filledPartitions = buildFilledPartitions( + autofillRequest = autofillRequest, + getCipherInlinePresentationOrNull = ::getCipherInlinePresentationOrNull, + ) // Use getOrLastOrNull so if the list has run dry take the last spec. val vaultItemInlinePresentationSpec = autofillRequest @@ -116,6 +81,59 @@ class FilledDataBuilderImpl( ) } + /** + * Build the [FilledPartition]s for the [autofillRequest]'s partition by fetching the matching + * ciphers and fulfilling the partition's views with each cipher's data. + */ + private suspend fun buildFilledPartitions( + autofillRequest: AutofillRequest.Fillable, + getCipherInlinePresentationOrNull: () -> InlinePresentationSpec?, + ): List = when (autofillRequest.partition) { + is AutofillPartition.Card -> { + autofillCipherProvider + .getCardAutofillCiphers() + .map { autofillCipher -> + fillCardPartition( + autofillCipher = autofillCipher, + autofillViews = autofillRequest.partition.views, + inlinePresentationSpec = getCipherInlinePresentationOrNull(), + ) + } + } + + is AutofillPartition.Login -> { + autofillRequest + .uri + ?.let { nonNullUri -> + autofillCipherProvider + .getLoginAutofillCiphers( + uri = nonNullUri, + ) + .map { autofillCipher -> + fillLoginPartition( + autofillCipher = autofillCipher, + autofillViews = autofillRequest.partition.views, + inlinePresentationSpec = getCipherInlinePresentationOrNull(), + packageName = autofillRequest.packageName, + ) + } + } + .orEmpty() + } + + is AutofillPartition.Identity -> { + autofillCipherProvider + .getIdentityAutofillCiphers() + .map { autofillCipher -> + fillIdentityPartition( + autofillCipher = autofillCipher, + autofillViews = autofillRequest.partition.views, + inlinePresentationSpec = getCipherInlinePresentationOrNull(), + ) + } + } + } + /** * Construct a [FilledPartition] by fulfilling the card [autofillViews] with data from the * card [autofillCipher]. @@ -181,6 +199,33 @@ class FilledDataBuilderImpl( inlinePresentationSpec = inlinePresentationSpec, ) } + + /** + * Construct a [FilledPartition] by fulfilling the identity [autofillViews] with data from the + * identity [autofillCipher]. + */ + private fun fillIdentityPartition( + autofillCipher: AutofillCipher.Identity, + autofillViews: List, + inlinePresentationSpec: InlinePresentationSpec?, + ): FilledPartition { + val filledItems = autofillViews + .mapNotNull { autofillView -> + autofillCipher + .getAutofillValueOrNull(autofillView) + ?.let { value -> + autofillView.buildFilledItemOrNull( + value = value, + ) + } + } + + return FilledPartition( + autofillCipher = autofillCipher, + filledItems = filledItems, + inlinePresentationSpec = inlinePresentationSpec, + ) + } } /** @@ -225,6 +270,34 @@ private fun AutofillCipher.Card.getAutofillValueOrNull(autofillView: AutofillVie } } +/** + * Get the autofill value for the given [autofillView], or null if no value is available. + */ +@Suppress("CyclomaticComplexMethod") +private fun AutofillCipher.Identity.getAutofillValueOrNull( + autofillView: AutofillView.Identity, +): String? = + when (autofillView) { + is AutofillView.Identity.PersonNameFull -> fullName + is AutofillView.Identity.PersonNamePrefix -> title + is AutofillView.Identity.PersonNameGiven -> firstName + is AutofillView.Identity.PersonNameMiddle -> middleName + is AutofillView.Identity.PersonNameFamily -> lastName + is AutofillView.Identity.PostalAddressFull -> fullAddress + is AutofillView.Identity.AddressStreet -> address1 + is AutofillView.Identity.AddressLocality -> city + is AutofillView.Identity.AddressRegion -> state + is AutofillView.Identity.AddressCountry -> country + is AutofillView.Identity.PostalCode -> postalCode + is AutofillView.Identity.PhoneFull -> phone + is AutofillView.Identity.Company -> company + is AutofillView.Identity.Email -> email + is AutofillView.Identity.Ssn -> ssn + is AutofillView.Identity.PassportNumber -> passportNumber + is AutofillView.Identity.LicenseNumber -> licenseNumber + } + ?.takeIf { it.isNotEmpty() } + /** * Get the item at the [index]. If that fails, return the last item in the list. If that also fails, * return null. 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 ffb7226b1cd..91f33e48487 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 @@ -62,8 +62,6 @@ private val URL_BARS: Map = mapOf( */ private val LOGIN_FILL_ASSIST_CATEGORIES: List = listOf( "account-login", - "account-creation", - "account-update", ) /** @@ -73,6 +71,17 @@ private val CARD_FILL_ASSIST_CATEGORIES: List = listOf( "payment-card", ) +/** + * A list of categories from Fill Assist that are used for [AutofillView.Identity]. + * + * Account creation and account update flows are where identity fields (name, address, phone, + * etc.) are curated alongside credential fields. + */ +private val IDENTITY_FILL_ASSIST_CATEGORIES: List = listOf( + "account-creation", + "account-update", +) + /** * The default [AutofillParser] implementation for the app. This is a tool for parsing autofill data * from the OS into domain models. @@ -140,7 +149,7 @@ class AutofillParserImpl( // request to Unfillable. val autofillViews = traversalDataList .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite) { - it !is AutofillView.Unused && it !is AutofillView.Identity + it !is AutofillView.Unused } val isFillAssistEnabled = featureFlagManager @@ -203,8 +212,12 @@ class AutofillParserImpl( } is AutofillView.Identity -> { - // Identity partition construction lands in Phase D. Unfillable until then. - return AutofillRequest.Unfillable + // Gated behind FlagKey.IdentityAutofill until the feature is ready for + // production; disabled matches this partition's pre-feature behavior. + if (!isIdentityAutofillEnabled) return AutofillRequest.Unfillable + AutofillPartition.Identity( + views = effectiveViews.filterIsInstance(), + ) } is AutofillView.Unused -> { @@ -262,16 +275,23 @@ class AutofillParserImpl( } ?: return this + // Identity categories were Login categories before identity autofill, so with the flag + // off they must stay Login's to keep fill-assist coverage unchanged on those hosts. + val loginCategories = if (isIdentityAutofillEnabled) { + LOGIN_FILL_ASSIST_CATEGORIES + } else { + LOGIN_FILL_ASSIST_CATEGORIES + IDENTITY_FILL_ASSIST_CATEGORIES + } + val coversCurrentPartition = hostRules.any { rule -> when (focusedView) { is AutofillView.Card -> rule.category in CARD_FILL_ASSIST_CATEGORIES - is AutofillView.Login -> rule.category in LOGIN_FILL_ASSIST_CATEGORIES + is AutofillView.Login -> rule.category in loginCategories + is AutofillView.Identity -> rule.category in IDENTITY_FILL_ASSIST_CATEGORIES is AutofillView.Unused -> { - rule.category in LOGIN_FILL_ASSIST_CATEGORIES || + rule.category in loginCategories || rule.category in CARD_FILL_ASSIST_CATEGORIES } - // Identity fill-assist categories land in a later phase. - is AutofillView.Identity -> false } } if (!coversCurrentPartition) return this @@ -471,17 +491,24 @@ private fun AssistStructure.ViewNode.traverse( isIdentityAutofillEnabled = isIdentityAutofillEnabled, ) .let { viewNodeTraversalData -> + // Flatten child views into this node, keeping the first view seen for each autofill + // id and dropping later duplicates (e.g. a container-redirect leftover). 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 + when (view) { + // Never claims an id, so a real view for that id can still be kept. + is AutofillView.Unused -> id !in claimedAutofillIds + // Always kept: a primary field, or the email/phone dual-classification + // sibling that intentionally shares a Login primary's already-claimed + // id. Claims the id when it is the primary. Container redirect never + // produces an Identity view, so keeping a claimed id is always safe. + is AutofillView.Identity -> { + claimedAutofillIds.add(id) + true + } + // Kept only the first time its id is seen (add returns false if known). + else -> claimedAutofillIds.add(id) } } .forEach(mutableAutofillViewList::add) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt index 21a7a3c1bdd..1d118cabcad 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/builder/FilledDataBuilderTest.kt @@ -728,6 +728,107 @@ class FilledDataBuilderTest { } } + @Suppress("MaxLineLength") + @Test + fun `build should fill each identity view from the identity cipher and skip empty values when Identity`() = + runTest { + // Setup + val firstName = "John" + val lastName = "Doe" + val city = "Springfield" + val autofillCipher = AutofillCipher.Identity( + cipherId = null, + name = "Cipher One", + subtitle = "Subtitle", + fullName = "John Doe", + fullAddress = "", + title = "", + firstName = firstName, + middleName = "", + lastName = lastName, + address1 = "", + address2 = "", + address3 = "", + city = city, + state = "", + postalCode = "", + country = "", + company = "", + email = "", + phone = "", + ssn = "", + passportNumber = "", + licenseNumber = "", + ) + val filledItemFirstName: FilledItem = mockk() + val filledItemLastName: FilledItem = mockk() + val filledItemCity: FilledItem = mockk() + val autofillViewFirstName: AutofillView.Identity.PersonNameGiven = mockk { + every { buildFilledItemOrNull(firstName) } returns filledItemFirstName + } + val autofillViewLastName: AutofillView.Identity.PersonNameFamily = mockk { + every { buildFilledItemOrNull(lastName) } returns filledItemLastName + } + val autofillViewCity: AutofillView.Identity.AddressLocality = mockk { + every { buildFilledItemOrNull(city) } returns filledItemCity + } + // Empty cipher value (state is blank) -> skipped, never builds a filled item. + val autofillViewState: AutofillView.Identity.AddressRegion = mockk() + val autofillPartition = AutofillPartition.Identity( + views = listOf( + autofillViewFirstName, + autofillViewLastName, + autofillViewCity, + autofillViewState, + ), + ) + val ignoreAutofillIds: List = mockk() + val autofillRequest = AutofillRequest.Fillable( + ignoreAutofillIds = ignoreAutofillIds, + inlinePresentationSpecs = emptyList(), + maxInlineSuggestionsCount = 0, + packageName = null, + partition = autofillPartition, + uri = URI, + ) + val filledPartition = FilledPartition( + autofillCipher = autofillCipher, + filledItems = listOf( + filledItemFirstName, + filledItemLastName, + filledItemCity, + ), + inlinePresentationSpec = null, + ) + val expected = FilledData( + filledPartitions = listOf( + filledPartition, + ), + ignoreAutofillIds = ignoreAutofillIds, + originalPartition = autofillPartition, + uri = URI, + vaultItemInlinePresentationSpec = null, + isVaultLocked = false, + ) + coEvery { + autofillCipherProvider.getIdentityAutofillCiphers() + } returns listOf(autofillCipher) + + // Test + val actual = filledDataBuilder.build( + autofillRequest = autofillRequest, + ) + + // Verify + assertEquals(expected, actual) + coVerify(exactly = 1) { + autofillCipherProvider.getIdentityAutofillCiphers() + } + verify(exactly = 0) { + autofillViewState.buildFilledItemOrNull(any()) + } + } + companion object { private const val URI: String = "androidapp://com.x8bit.bitwarden" } 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 832178f4ea5..7071fbb97bc 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 @@ -35,6 +35,7 @@ import io.mockk.verify import kotlinx.coroutines.flow.MutableStateFlow import org.junit.jupiter.api.AfterEach import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -66,12 +67,26 @@ class AutofillParserTests { every { this@mockk.idEntry } returns null every { this@mockk.hint } returns null } + private val identityAutofillHint = View.AUTOFILL_HINT_NAME + private val identityAutofillId: AutofillId = mockk() + private val identityViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillHints } returns arrayOf(identityAutofillHint) + every { this@mockk.autofillId } returns identityAutofillId + every { this@mockk.childCount } returns 0 + every { this@mockk.htmlInfo } returns mockk(relaxed = true) + every { this@mockk.idPackage } returns ID_PACKAGE + every { this@mockk.idEntry } returns null + every { this@mockk.hint } returns null + } private val cardWindowNode: AssistStructure.WindowNode = mockk { every { this@mockk.rootViewNode } returns cardViewNode } private val loginWindowNode: AssistStructure.WindowNode = mockk { every { this@mockk.rootViewNode } returns loginViewNode } + private val identityWindowNode: AssistStructure.WindowNode = mockk { + every { this@mockk.rootViewNode } returns identityViewNode + } private val fillContext: FillContext = mockk { every { this@mockk.structure } returns assistStructure } @@ -87,7 +102,7 @@ class AutofillParserTests { } private val fillAssistManager: FillAssistManager = mockk() private val mutableFillAssistFlagFlow = MutableStateFlow(false) - private val mutableIdentityAutofillFlagFlow = MutableStateFlow(false) + private val mutableIdentityAutofillFlagFlow = MutableStateFlow(true) private val featureFlagManager: FeatureFlagManager = mockk { every { getFeatureFlag(FlagKey.FillAssistTargetingRules) @@ -97,6 +112,7 @@ class AutofillParserTests { every { getFeatureFlagFlow(FlagKey.FillAssistTargetingRules) } returns mutableFillAssistFlagFlow + every { getFeatureFlag(FlagKey.IdentityAutofill) } answers { @@ -137,6 +153,7 @@ class AutofillParserTests { ) every { cardViewNode.website } returns WEBSITE every { loginViewNode.website } returns WEBSITE + every { identityViewNode.website } returns WEBSITE every { fillRequest.getInlinePresentationSpecs( autofillAppInfo = autofillAppInfo, @@ -426,7 +443,6 @@ class AutofillParserTests { @Test fun `parse should choose AutofillPartition Card when a Card view is focused`() { // Setup - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -450,6 +466,7 @@ class AutofillParserTests { website = URI, ), ) + setupAssistStructure(card = cardAutofillView, login = loginAutofillView) val autofillPartition = AutofillPartition.Card( views = listOf(cardAutofillView), ) @@ -461,18 +478,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -499,7 +504,6 @@ class AutofillParserTests { @Test fun `parse should choose AutofillPartition Login when a Login view is focused`() { // Setup - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -523,6 +527,7 @@ class AutofillParserTests { website = URI, ), ) + setupAssistStructure(card = cardAutofillView, login = loginAutofillView) val autofillPartition = AutofillPartition.Login( views = listOf(loginAutofillView), ) @@ -534,18 +539,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -569,6 +562,196 @@ class AutofillParserTests { } } + @Test + fun `parse should choose AutofillPartition Identity when an Identity view is focused`() { + // Setup + val identityAutofillView: AutofillView.Identity = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = identityAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + val loginAutofillView: AutofillView.Login = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + setupAssistStructure(login = loginAutofillView, identity = identityAutofillView) + val autofillPartition = AutofillPartition.Identity( + views = listOf(identityAutofillView), + ) + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = emptyList(), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = autofillPartition, + uri = URI, + ) + + // Test + val actual = parser.parse( + autofillAppInfo = autofillAppInfo, + fillRequest = fillRequest, + ) + + // Verify + assertEquals(expected, actual) + } + + @Suppress("MaxLineLength") + @Test + fun `parse should return Unfillable when an Identity view is focused and IdentityAutofill is disabled`() { + // Setup + every { featureFlagManager.getFeatureFlag(FlagKey.IdentityAutofill) } returns false + val identityAutofillView: AutofillView.Identity = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = identityAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + val loginAutofillView: AutofillView.Login = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + setupAssistStructure(login = loginAutofillView, identity = identityAutofillView) + + // Test + val actual = parser.parse( + autofillAppInfo = autofillAppInfo, + fillRequest = fillRequest, + ) + + // Verify + assertEquals(AutofillRequest.Unfillable, actual) + } + + @Suppress("MaxLineLength") + @Test + fun `parse should keep the Identity dual-classification sibling of a nested email field in the Identity partition`() { + // Setup — a registration-style form: a focused Name field plus a (non-focused) email + // field, both nested under a container. The email field is classified as Login.Username + // and gets a dual-classification Identity.Email sibling (same autofillId). Focusing the + // Name field builds an Identity partition, which must include the email's Identity.Email + // sibling so a whole-identity fill also populates the email field. Regression guard: the + // container-redirect dedup must not drop that sibling just because its id is already + // claimed by the Login.Username primary. + val nameAutofillId: AutofillId = mockk() + val emailAutofillId: AutofillId = mockk() + val nameView: AutofillView.Identity = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = nameAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + val emailLoginView: AutofillView.Login = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = emailAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + val nameViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillId } returns nameAutofillId + every { this@mockk.childCount } returns 0 + every { this@mockk.idPackage } returns null + every { this@mockk.website } returns null + every { + this@mockk.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns nameView + } + val emailViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillId } returns emailAutofillId + every { this@mockk.autofillHints } returns arrayOf(View.AUTOFILL_HINT_EMAIL_ADDRESS) + every { this@mockk.childCount } returns 0 + every { this@mockk.idPackage } returns null + every { this@mockk.idEntry } returns null + every { this@mockk.hint } returns null + every { this@mockk.htmlInfo } returns mockk(relaxed = true) + every { this@mockk.website } returns null + every { + this@mockk.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns emailLoginView + } + val rootAutofillId: AutofillId = mockk() + val rootViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillId } returns rootAutofillId + every { this@mockk.childCount } returns 2 + every { this@mockk.getChildAt(0) } returns nameViewNode + every { this@mockk.getChildAt(1) } returns emailViewNode + every { this@mockk.idPackage } returns ID_PACKAGE + every { this@mockk.website } returns null + every { + this@mockk.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns null + } + val windowNode: AssistStructure.WindowNode = mockk { + every { this@mockk.rootViewNode } returns rootViewNode + } + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns windowNode + + // Test + val actual = parser.parse( + autofillAppInfo = autofillAppInfo, + fillRequest = fillRequest, + ) + + // Verify — the Identity partition contains both the focused Name view and the email + // field's Identity.Email sibling (reusing the Login.Username view's data). + assertTrue(actual is AutofillRequest.Fillable) + val partition = (actual as AutofillRequest.Fillable).partition + assertTrue(partition is AutofillPartition.Identity) + assertEquals( + listOf( + nameView, + AutofillView.Identity.Email(data = emailLoginView.data), + ), + (partition as AutofillPartition.Identity).views, + ) + } + @Suppress("MaxLineLength") @Test fun `parse should have Password AutofillView when the Password field is invalid, contains no other Password fields, and contains a password term`() { @@ -751,7 +934,6 @@ class AutofillParserTests { @Test fun `parse should choose first focused AutofillView for partition when there are multiple`() { // Setup - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -775,6 +957,22 @@ class AutofillParserTests { website = URI, ), ) + val identityAutofillView = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = identityAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = FILL_ASSIST_URI, + ), + ) + setupAssistStructure( + card = cardAutofillView, + login = loginAutofillView, + identity = identityAutofillView, + ) val autofillPartition = AutofillPartition.Card( views = listOf(cardAutofillView), ) @@ -786,18 +984,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -825,7 +1011,6 @@ class AutofillParserTests { @Test fun `parse should choose first fillable AutofillView for partition when there is no focused view`() { // Setup - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -849,6 +1034,7 @@ class AutofillParserTests { website = URI, ), ) + setupAssistStructure(card = cardAutofillView, login = loginAutofillView) val autofillPartition = AutofillPartition.Card( views = listOf(cardAutofillView), ) @@ -860,18 +1046,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -974,7 +1148,6 @@ class AutofillParserTests { fun `parse should return empty inline suggestions when inline autofill is disabled`() { // Setup mockIsInlineAutofillEnabled = false - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -992,12 +1165,13 @@ class AutofillParserTests { autofillId = loginAutofillId, autofillOptions = emptyList(), autofillType = AUTOFILL_TYPE, - isFocused = true, + isFocused = false, textValue = null, hasPasswordTerms = false, website = URI, ), ) + setupAssistStructure(card = cardAutofillView, login = loginAutofillView) val autofillPartition = AutofillPartition.Card( views = listOf(cardAutofillView), ) @@ -1009,18 +1183,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -1048,7 +1210,6 @@ class AutofillParserTests { fun `parse should return empty inline suggestions when parsing an AssistStructure directly`() { // Setup mockIsInlineAutofillEnabled = false - setupAssistStructureWithAllAutofillViewTypes() val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( data = AutofillView.Data( autofillId = cardAutofillId, @@ -1066,12 +1227,13 @@ class AutofillParserTests { autofillId = loginAutofillId, autofillOptions = emptyList(), autofillType = AUTOFILL_TYPE, - isFocused = true, + isFocused = false, textValue = null, hasPasswordTerms = false, website = URI, ), ) + setupAssistStructure(card = cardAutofillView, login = loginAutofillView) val autofillPartition = AutofillPartition.Card( views = listOf(cardAutofillView), ) @@ -1083,18 +1245,6 @@ class AutofillParserTests { partition = autofillPartition, uri = URI, ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView // Test val actual = parser.parse( @@ -1121,19 +1271,6 @@ class AutofillParserTests { @Test fun `parse should skip block listed URIs Login when a Login view is focused`() { // Setup all tests - setupAssistStructureWithAllAutofillViewTypes() - val cardAutofillView: AutofillView.Card = AutofillView.Card.ExpirationMonth( - data = AutofillView.Data( - autofillId = cardAutofillId, - autofillOptions = emptyList(), - autofillType = AUTOFILL_TYPE, - isFocused = true, - textValue = null, - hasPasswordTerms = false, - website = URI, - ), - monthValue = null, - ) val loginAutofillView: AutofillView.Login = AutofillView.Login.Username( data = AutofillView.Data( autofillId = loginAutofillId, @@ -1145,22 +1282,11 @@ class AutofillParserTests { website = URI, ), ) + setupAssistStructure(login = loginAutofillView) val remoteBlockList = listOf( "blockListedUri.com", "blockListedAgainUri.com", ) - every { - cardViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns cardAutofillView - every { - loginViewNode.toAutofillView( - parentWebsite = any(), - isIdentityAutofillEnabled = any(), - ) - } returns loginAutofillView every { settingsRepository.blockedAutofillUris } returns remoteBlockList // A function for asserting that a block listed URI results in an unfillable request. @@ -1474,6 +1600,154 @@ class AutofillParserTests { assertEquals(expected, actual) } + @Suppress("MaxLineLength") + @Test + fun `parse should fall back to heuristics when fill-assist rules exist but only cover account-login and an identity view is focused`() { + // Setup: fill-assist enabled with login-only rules, but an identity view is focused. + mutableFillAssistFlagFlow.value = true + mockIsFillAssistEnabled = true + every { any().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI + every { fillAssistManager.getFillAssistRules() } returns FillAssistRules( + hostRules = mapOf( + FILL_ASSIST_URI to listOf( + FillAssistRules.HostRule( + category = "account-login", + fields = mapOf( + "username" to listOf( + FillAssistRules.SelectorClause( + tag = "input", + id = "user", + name = null, + type = null, + role = null, + ), + ), + ), + ), + ), + ), + ) + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns identityWindowNode + val identityAutofillView = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = identityAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = FILL_ASSIST_URI, + ), + ) + every { + identityViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns identityAutofillView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify: heuristic identity view used + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = emptyList(), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = AutofillPartition.Identity(views = listOf(identityAutofillView)), + uri = FILL_ASSIST_URI, + ) + assertEquals(expected, actual) + } + + @Suppress("MaxLineLength") + @Test + fun `parse should use fill-assist views when rules cover account-creation or account-update and an identity view is focused`() { + // The heuristic and fill-assist paths produce views with DIFFERENT autofillIds so the + // assertion proves which path was actually taken. + listOf("account-creation", "account-update").forEach { category -> + mutableFillAssistFlagFlow.value = true + mockIsFillAssistEnabled = true + every { any().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI + every { fillAssistManager.getFillAssistRules() } returns FillAssistRules( + hostRules = mapOf( + FILL_ASSIST_URI to listOf( + FillAssistRules.HostRule( + category = category, + fields = mapOf( + "personNameGiven" to listOf( + FillAssistRules.SelectorClause( + tag = "input", + id = "first-name", + name = null, + type = null, + role = null, + ), + ), + ), + ), + ), + ), + ) + val heuristicIdentityView = AutofillView.Identity.PersonNameGiven( + data = AutofillView.Data( + autofillId = identityAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = FILL_ASSIST_URI, + ), + ) + val fillAssistAutofillId: AutofillId = mockk() + val fillAssistIdentityData = AutofillView.Data( + autofillId = fillAssistAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = WEBSITE, + ) + every { any().matchesSelectorClause(any()) } returns true + every { + identityViewNode.toAutofillViewData( + autofillId = identityAutofillId, + website = WEBSITE, + ) + } returns fillAssistIdentityData + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns identityWindowNode + every { + identityViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns heuristicIdentityView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify: fill-assist views used — partition contains fillAssistAutofillId. + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = emptyList(), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = AutofillPartition.Identity( + views = listOf( + AutofillView.Identity.PersonNameGiven(data = fillAssistIdentityData), + ), + ), + uri = FILL_ASSIST_URI, + ) + assertEquals(expected, actual, "Failed for category: $category") + } + } + @Suppress("MaxLineLength") @Test fun `parse should use fill-assist views when heuristics classify the focused view as Unused`() { @@ -1900,9 +2174,10 @@ class AutofillParserTests { // Setup: a node that heuristics would classify as Identity PhoneFull once identity // detection is active, sitting directly above a password field. Before identity // detection existed, this same node fell through to Unused and was promoted to - // Login.Username via updateForMissingUsernameFields. With IdentityAutofill disabled - // (the default), toAutofillView must still resolve it to Unused, so that promotion - // continues to work exactly as it did before identity heuristics existed. + // Login.Username via updateForMissingUsernameFields. With IdentityAutofill disabled, + // toAutofillView must still resolve it to Unused, so that promotion continues to work + // exactly as it did before identity heuristics existed. + mutableIdentityAutofillFlagFlow.value = false val (rootViewNode, phoneHintedViewNode, passwordViewNode, passwordAutofillId) = setupPhoneHintedFieldAbovePassword() val windowNode: AssistStructure.WindowNode = mockk { @@ -1965,11 +2240,13 @@ class AutofillParserTests { @Suppress("MaxLineLength") @Test - fun `parse should not promote a phone-hinted field to Login Username when IdentityAutofill is enabled and the field resolves to Identity PhoneFull`() { + fun `parse should resolve a phone-hinted field to an Identity partition instead of promoting it to Login Username when IdentityAutofill is enabled`() { // Setup: same shape as the disabled case above, but IdentityAutofill is enabled, so // toAutofillView resolves the field to Identity.PhoneFull instead of Unused before - // updateForMissingUsernameFields ever runs. The promotion is skipped, and the Login - // partition ends up missing its username field -- the regression this fix is guarding. + // updateForMissingUsernameFields ever runs. The promotion is skipped -- but unlike before + // Phase D landed, the field isn't lost: it resolves through a real Identity partition + // instead. The trade-off is that it's no longer offered together with the password field + // in the same fill action, since they now belong to different partition types. mutableIdentityAutofillFlagFlow.value = true val (rootViewNode, phoneHintedViewNode, passwordViewNode, passwordAutofillId) = setupPhoneHintedFieldAbovePassword() @@ -2016,13 +2293,14 @@ class AutofillParserTests { // Test val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) - // Verify: no promotion -- the Login partition only contains the password field. + // Verify: no promotion -- the focused view resolves to a real Identity partition instead, + // and the (unfocused, different-partition-type) password field is excluded from it. val expected = AutofillRequest.Fillable( ignoreAutofillIds = listOf(rootViewNode.autofillId!!), inlinePresentationSpecs = inlinePresentationSpecs, maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, packageName = PACKAGE_NAME, - partition = AutofillPartition.Login(views = listOf(loginPasswordAutofillView)), + partition = AutofillPartition.Identity(views = listOf(identityPhoneView)), uri = URI, ) assertEquals(expected, actual) @@ -2071,13 +2349,49 @@ class AutofillParserTests { } /** - * 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. + * Sets up [assistStructure] with one window node per non-null argument, in card → login → + * identity order, each stubbed to return the given view from `toAutofillView`. A window is + * omitted entirely (not present in the mocked structure) when its argument is null — there is + * no filler/default view for an omitted window. */ - private fun setupAssistStructureWithAllAutofillViewTypes() { - every { assistStructure.windowNodeCount } returns 2 - every { assistStructure.getWindowNodeAt(0) } returns cardWindowNode - every { assistStructure.getWindowNodeAt(1) } returns loginWindowNode + private fun setupAssistStructure( + card: AutofillView.Card? = null, + login: AutofillView.Login? = null, + identity: AutofillView.Identity? = null, + ) { + val windowNodes = buildList { + card?.let { + every { + cardViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns it + add(cardWindowNode) + } + login?.let { + every { + loginViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns it + add(loginWindowNode) + } + identity?.let { + every { + identityViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns it + add(identityWindowNode) + } + } + every { assistStructure.windowNodeCount } returns windowNodes.size + windowNodes.forEachIndexed { index, node -> + every { assistStructure.getWindowNodeAt(index) } returns node + } } } From ce091c6b6944690811797f20e8f106e1e1edf7ae Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Fri, 4 Sep 2026 15:53:33 +0100 Subject: [PATCH 4/6] Fixed Identity FillAssistNode names, fixed long line and added test for IdentityAutofill turned off --- .../autofill/parser/AutofillParserImpl.kt | 11 +-- .../autofill/parser/AutofillParserTests.kt | 84 ++++++++++++++++++- 2 files changed, 87 insertions(+), 8 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 91f33e48487..b68fdcbe461 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 @@ -141,12 +141,7 @@ class AutofillParserImpl( .flatMap { it.urlBarWebsites } .firstOrNull() // Heuristic views: the focused node's candidates with unfillable (Unused) fields removed, - // falling back to all fillable views when nothing has focus. Identity is also excluded - // here for now -- Identity partition construction lands in Phase D, so until then a field - // classified as Identity must keep falling through exactly as it would have as Unused - // (e.g. resolving to a sibling Login/Card field on the same form, or its Unused-only - // promotion in updateForMissingUsernameFields), not become the focused view and force this - // request to Unfillable. + // falling back to all fillable views when nothing has focus. val autofillViews = traversalDataList .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite) { it !is AutofillView.Unused @@ -578,7 +573,9 @@ private fun AutofillView.updateWebsiteIfNecessary(website: String?): AutofillVie } is AutofillView.Identity.PhoneFull -> this.copy(data = this.data.copy(website = site)) - is AutofillView.Identity.PostalAddressFull -> this.copy(data = this.data.copy(website = site)) + is AutofillView.Identity.PostalAddressFull -> { + this.copy(data = this.data.copy(website = site)) + } is AutofillView.Identity.PostalCode -> this.copy(data = this.data.copy(website = site)) is AutofillView.Identity.Ssn -> this.copy(data = this.data.copy(website = site)) is AutofillView.Unused -> this.copy(data = this.data.copy(website = site)) 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 7071fbb97bc..fab8ccbc4dd 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 @@ -1677,7 +1677,7 @@ class AutofillParserTests { FillAssistRules.HostRule( category = category, fields = mapOf( - "personNameGiven" to listOf( + "firstName" to listOf( FillAssistRules.SelectorClause( tag = "input", id = "first-name", @@ -1748,6 +1748,88 @@ class AutofillParserTests { } } + @Suppress("MaxLineLength") + @Test + fun `parse should use fill-assist views when rules cover account-creation and a login view is focused with IdentityAutofill disabled`() { + // Account-creation and account-update were Login categories before identity autofill + // split them out; with IdentityAutofill disabled they must still count as Login + // categories so fill-assist coverage on these hosts is unchanged. + mutableIdentityAutofillFlagFlow.value = false + mutableFillAssistFlagFlow.value = true + mockIsFillAssistEnabled = true + every { any().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI + every { fillAssistManager.getFillAssistRules() } returns FillAssistRules( + hostRules = mapOf( + FILL_ASSIST_URI to listOf( + FillAssistRules.HostRule( + category = "account-creation", + fields = mapOf( + "username" to listOf( + FillAssistRules.SelectorClause( + tag = "input", + id = "user", + name = null, + type = null, + role = null, + ), + ), + ), + ), + ), + ), + ) + val heuristicLoginView = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = FILL_ASSIST_URI, + ), + ) + val fillAssistAutofillId: AutofillId = mockk() + val fillAssistLoginData = AutofillView.Data( + autofillId = fillAssistAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = WEBSITE, + ) + every { any().matchesSelectorClause(any()) } returns true + every { + loginViewNode.toAutofillViewData(autofillId = loginAutofillId, website = WEBSITE) + } returns fillAssistLoginData + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode + every { + loginViewNode.toAutofillView( + parentWebsite = any(), + isIdentityAutofillEnabled = any(), + ) + } returns heuristicLoginView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify: fill-assist views used — partition contains fillAssistAutofillId. + // Heuristics would have produced loginAutofillId. + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = emptyList(), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = AutofillPartition.Login( + views = listOf(AutofillView.Login.Username(data = fillAssistLoginData)), + ), + uri = FILL_ASSIST_URI, + ) + assertEquals(expected, actual) + } + @Suppress("MaxLineLength") @Test fun `parse should use fill-assist views when heuristics classify the focused view as Unused`() { From 7bcecc909e3e9378d030f9e865384cc58ee21e6e Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Tue, 8 Sep 2026 15:12:03 +0100 Subject: [PATCH 5/6] Fix Identity partition selection after merging real Identity partition support --- .../autofill/parser/AutofillParserImpl.kt | 21 +++++++++++++++---- .../autofill/parser/AutofillParserTests.kt | 14 ++++++------- 2 files changed, 24 insertions(+), 11 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 b68fdcbe461..8bda2a31b5a 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 @@ -187,9 +187,7 @@ class AutofillParserImpl( autofillViews } - val effectiveFocusedView = effectiveViews - .filterNot { it is AutofillView.Identity } - .firstFocusedOrNull() + val effectiveFocusedView = effectiveViews.effectiveFocusedViewOrNull() ?: return AutofillRequest.Unfillable // Choose the first focused partition of data for fulfillment. @@ -284,8 +282,11 @@ class AutofillParserImpl( is AutofillView.Login -> rule.category in loginCategories is AutofillView.Identity -> rule.category in IDENTITY_FILL_ASSIST_CATEGORIES is AutofillView.Unused -> { + val identityCoversRule = isIdentityAutofillEnabled && + rule.category in IDENTITY_FILL_ASSIST_CATEGORIES rule.category in loginCategories || - rule.category in CARD_FILL_ASSIST_CATEGORIES + rule.category in CARD_FILL_ASSIST_CATEGORIES || + identityCoversRule } } } @@ -352,6 +353,18 @@ private fun List.selectCandidateAutofillViews( private fun List.firstFocusedOrNull(): AutofillView? = firstOrNull { it.data.isFocused } ?: firstOrNull() +/** + * Returns the [AutofillView] that should win partition selection: a focused Login/Card view + * first, then any focused view (so a real Identity partition can still be built when Identity is + * the only classification available), then any Login/Card view, then any view at all. + */ +private fun List.effectiveFocusedViewOrNull(): AutofillView? { + val focusedNonIdentity = firstOrNull { it.data.isFocused && it !is AutofillView.Identity } + val focusedAny = firstOrNull { it.data.isFocused } + val nonIdentity = firstOrNull { it !is AutofillView.Identity } + return focusedNonIdentity ?: focusedAny ?: nonIdentity ?: firstOrNull() +} + /** * This helper function updates the [ViewNodeTraversalData] if necessary for missing password * fields that were marked invalid because they contained a specific `hint` or `idEntry`. If the 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 fab8ccbc4dd..2bb58508c70 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 @@ -2074,13 +2074,12 @@ class AutofillParserTests { @Suppress("MaxLineLength") @Test - fun `parse should choose AutofillPartition Login when an Identity view is focused but a Login view is fillable elsewhere`() { + fun `parse should choose AutofillPartition Identity when an Identity view is focused even though a Login view is fillable in another window`() { // Setup: a focused field heuristics classify as Identity (e.g. "First name" on a signup // form) sits in one window, while a fillable, unfocused Login.Username field exists in - // another window on the same screen. Before Phase D's Identity partition exists, a - // focused Identity view must not force the whole request to Unfillable when a fillable - // Login/Card partition exists elsewhere -- it should be excluded from candidates exactly - // like Unused, falling through to the other fillable view. + // another window on the same screen. The Login view never enters the candidate pool -- + // selectCandidateAutofillViews only considers a window whose own views have a focused + // entry -- so the focused Identity view wins and builds a real Identity partition. val identityAutofillId: AutofillId = mockk() val identityViewNode: AssistStructure.ViewNode = mockk { every { this@mockk.autofillHints } returns emptyArray() @@ -2130,13 +2129,14 @@ class AutofillParserTests { // Test val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) - // Verify: falls through to the fillable Login view instead of becoming Unfillable. + // Verify: the focused Identity view wins; the Login view in the other window is never a + // candidate. val expected = AutofillRequest.Fillable( ignoreAutofillIds = emptyList(), inlinePresentationSpecs = inlinePresentationSpecs, maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, packageName = PACKAGE_NAME, - partition = AutofillPartition.Login(views = listOf(loginAutofillView)), + partition = AutofillPartition.Identity(views = listOf(identityAutofillView)), uri = URI, ) assertEquals(expected, actual) From 73d3e9e25ff4df630bef7b4b94eec2fc587560a8 Mon Sep 17 00:00:00 2001 From: Andre Rosado Date: Tue, 8 Sep 2026 17:56:41 +0100 Subject: [PATCH 6/6] addressing autofill issues --- .../autofill/parser/AutofillParserImpl.kt | 22 +++++--- .../data/autofill/util/ViewNodeExtensions.kt | 11 ++-- .../data/autofill/util/ViewStructureUtils.kt | 1 + .../autofill/parser/AutofillParserTests.kt | 32 +++++++---- .../util/FillAssistViewNodeExtensionsTest.kt | 56 +++++++++++++++++++ .../autofill/util/ViewNodeExtensionsTest.kt | 20 ++++++- 6 files changed, 113 insertions(+), 29 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 8bda2a31b5a..fe400fb0191 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 @@ -480,9 +480,8 @@ private fun AssistStructure.ViewNode.traverse( mutableAutofillViewList.add(AutofillView.Identity.Email(data = view.data)) } - // Some phone hints (e.g. "mobilephone") also match the username heuristic's - // "phone" term and resolve to Login.Username above, so they need the same - // dual-classification as email. + // A phone-hinted or phone-heuristic field resolves to Login.Username above (see + // supportedAutofillHint), so it needs the same dual-classification as email. if (view is AutofillView.Login.Username && this.isPhoneField) { mutableAutofillViewList.add(AutofillView.Identity.PhoneFull(data = view.data)) } @@ -499,6 +498,11 @@ private fun AssistStructure.ViewNode.traverse( isIdentityAutofillEnabled = isIdentityAutofillEnabled, ) .let { viewNodeTraversalData -> + // Ids already claimed by an ancestor's own view (e.g. a container-redirect + // target) before this child's results are considered. A primary always precedes + // its Identity dual-classification sibling in this same child's results, so the + // sibling's id is still fresh here and passes the check below. + val idsClaimedByAncestor = claimedAutofillIds.toSet() // Flatten child views into this node, keeping the first view seen for each autofill // id and dropping later duplicates (e.g. a container-redirect leftover). viewNodeTraversalData.autofillViews @@ -507,13 +511,13 @@ private fun AssistStructure.ViewNode.traverse( when (view) { // Never claims an id, so a real view for that id can still be kept. is AutofillView.Unused -> id !in claimedAutofillIds - // Always kept: a primary field, or the email/phone dual-classification - // sibling that intentionally shares a Login primary's already-claimed - // id. Claims the id when it is the primary. Container redirect never - // produces an Identity view, so keeping a claimed id is always safe. + // Kept if the id is fresh, i.e. it's a dual-classification sibling of + // a primary view from this same child. Dropped if the id was already + // claimed by an ancestor's own view -- that means this is a stale + // container-redirect leftover, not an intentional sibling. is AutofillView.Identity -> { - claimedAutofillIds.add(id) - true + (id !in idsClaimedByAncestor) + .also { keep -> if (keep) claimedAutofillIds.add(id) } } // Kept only the first time its id is seen (add returns false if known). else -> claimedAutofillIds.add(id) 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 7ec24a0edae..0c4b93f401c 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 @@ -181,7 +181,7 @@ private fun AssistStructure.ViewNode.supportedAutofillHint( this.isAddressRegionField -> AutofillHint.Identity.ADDRESS_REGION this.isAddressCountryField -> AutofillHint.Identity.ADDRESS_COUNTRY this.isPostalCodeField -> AutofillHint.Identity.POSTAL_CODE - this.isPhoneField -> AutofillHint.Identity.PHONE_FULL + this.isPhoneField -> AutofillHint.Login.USERNAME this.isCompanyField -> AutofillHint.Identity.COMPANY this.isSsnField -> AutofillHint.Identity.SSN this.isPassportNumberField -> AutofillHint.Identity.PASSPORT_NUMBER @@ -239,7 +239,7 @@ private fun String.toBitwardenAutofillHintOrNull(): AutofillHint? = } View.AUTOFILL_HINT_POSTAL_CODE -> AutofillHint.Identity.POSTAL_CODE - View.AUTOFILL_HINT_PHONE -> AutofillHint.Identity.PHONE_FULL + View.AUTOFILL_HINT_PHONE -> AutofillHint.Login.USERNAME else -> null } @@ -551,9 +551,10 @@ internal val AssistStructure.ViewNode.isPostalCodeField: Boolean */ @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal val AssistStructure.ViewNode.isPhoneField: Boolean - get() = idEntry - ?.toLowerCaseAndStripNonAlpha() - ?.containsAnyTerms(SUPPORTED_RAW_PHONE_HINTS) == true || + get() = autofillHints?.contains(View.AUTOFILL_HINT_PHONE) == true || + idEntry + ?.toLowerCaseAndStripNonAlpha() + ?.containsAnyTerms(SUPPORTED_RAW_PHONE_HINTS) == true || hint ?.toLowerCaseAndStripNonAlpha() ?.containsAnyTerms(SUPPORTED_RAW_PHONE_HINTS) == true || diff --git a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewStructureUtils.kt b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewStructureUtils.kt index 7f93758d487..ee68f72ff0b 100644 --- a/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewStructureUtils.kt +++ b/app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewStructureUtils.kt @@ -253,6 +253,7 @@ val SUPPORTED_RAW_POSTAL_CODE_HINTS: List = listOf( * The supported phone number autofill hints. */ val SUPPORTED_RAW_PHONE_HINTS: List = listOf( + "phone", "phonenumber", "telephone", "mobilephone", 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 2bb58508c70..48a93f6cbec 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 @@ -2322,22 +2322,29 @@ class AutofillParserTests { @Suppress("MaxLineLength") @Test - fun `parse should resolve a phone-hinted field to an Identity partition instead of promoting it to Login Username when IdentityAutofill is enabled`() { - // Setup: same shape as the disabled case above, but IdentityAutofill is enabled, so - // toAutofillView resolves the field to Identity.PhoneFull instead of Unused before - // updateForMissingUsernameFields ever runs. The promotion is skipped -- but unlike before - // Phase D landed, the field isn't lost: it resolves through a real Identity partition - // instead. The trade-off is that it's no longer offered together with the password field - // in the same fill action, since they now belong to different partition types. + fun `parse should classify a phone-hinted field as Login Username with an Identity PhoneFull sibling when IdentityAutofill is enabled`() { + // Setup: same shape as the disabled case above, but IdentityAutofill is enabled and the + // node genuinely declares the phone autofill hint, so the real isPhoneField check (not a + // mocked toAutofillView return) fires. The phone hint always maps to Login.Username, so + // traverse() adds an Identity.PhoneFull dual-classification sibling alongside it. That + // sibling doesn't change partition selection here -- Login.Username is focused and + // precedes it in the views list, so it still wins, producing the same Login partition as + // the disabled-flag case. mutableIdentityAutofillFlagFlow.value = true val (rootViewNode, phoneHintedViewNode, passwordViewNode, passwordAutofillId) = setupPhoneHintedFieldAbovePassword() + every { phoneHintedViewNode.autofillHints } returns arrayOf(View.AUTOFILL_HINT_PHONE) + // isEmailField is also checked alongside isPhoneField now that the real toAutofillView + // return (Login.Username) is used instead of a mocked bypass. + every { phoneHintedViewNode.idEntry } returns null + every { phoneHintedViewNode.hint } returns null + every { phoneHintedViewNode.htmlInfo } returns mockk(relaxed = true) val windowNode: AssistStructure.WindowNode = mockk { every { this@mockk.rootViewNode } returns rootViewNode } every { assistStructure.windowNodeCount } returns 1 every { assistStructure.getWindowNodeAt(0) } returns windowNode - val identityPhoneView = AutofillView.Identity.PhoneFull( + val loginUsernameAutofillView = AutofillView.Login.Username( data = AutofillView.Data( autofillId = loginAutofillId, autofillOptions = emptyList(), @@ -2364,7 +2371,7 @@ class AutofillParserTests { parentWebsite = any(), isIdentityAutofillEnabled = true, ) - } returns identityPhoneView + } returns loginUsernameAutofillView every { passwordViewNode.toAutofillView( parentWebsite = any(), @@ -2375,14 +2382,15 @@ class AutofillParserTests { // Test val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) - // Verify: no promotion -- the focused view resolves to a real Identity partition instead, - // and the (unfocused, different-partition-type) password field is excluded from it. + // Verify: Login.Username still wins partition selection, same as the disabled-flag case. val expected = AutofillRequest.Fillable( ignoreAutofillIds = listOf(rootViewNode.autofillId!!), inlinePresentationSpecs = inlinePresentationSpecs, maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, packageName = PACKAGE_NAME, - partition = AutofillPartition.Identity(views = listOf(identityPhoneView)), + partition = AutofillPartition.Login( + views = listOf(loginUsernameAutofillView, loginPasswordAutofillView), + ), uri = URI, ) assertEquals(expected, actual) diff --git a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt index 19b09d3314f..2b4fd10942f 100644 --- a/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt +++ b/app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/util/FillAssistViewNodeExtensionsTest.kt @@ -182,6 +182,62 @@ class FillAssistViewNodeExtensionsTest { ) } + @Suppress("MaxLineLength") + @Test + fun `buildFillAssistViews should return only Login Username when htmlInfo matches phone clause and IdentityAutofill is disabled`() { + val htmlInfo = createHtmlInfo() + val viewNode = createViewNode(htmlInfo = htmlInfo) + val assistStructure = createAssistStructure(viewNode) + val data = autofillData() + every { + viewNode.toAutofillViewData( + autofillId = autofillId, + website = null, + ) + } returns data + + val hostRule = FillAssistRules.HostRule( + category = "account-login", + fields = mapOf( + "phone" to listOf(selectorClause(tag = "input", id = "phone")), + ), + ) + + val actual = assistStructure.buildFillAssistViews( + hostRules = listOf(hostRule), + urlBarWebsite = null, + isIdentityAutofillEnabled = false, + ) + + assertEquals(listOf(AutofillView.Login.Username(data = data)), actual) + } + + @Suppress("MaxLineLength") + @Test + fun `buildFillAssistViews should exclude identity-only field key when IdentityAutofill is disabled`() { + val htmlInfo = createHtmlInfo() + val viewNode = createViewNode(htmlInfo = htmlInfo) + val assistStructure = createAssistStructure(viewNode) + every { + viewNode.toAutofillViewData(autofillId = autofillId, website = null) + } returns autofillData() + + val hostRule = FillAssistRules.HostRule( + category = "identity-form", + fields = mapOf( + "firstName" to listOf(selectorClause(tag = "input", id = "first-name")), + ), + ) + + val actual = assistStructure.buildFillAssistViews( + hostRules = listOf(hostRule), + urlBarWebsite = null, + isIdentityAutofillEnabled = false, + ) + + assertEquals(emptyList(), actual) + } + @Suppress("MaxLineLength") @Test fun `buildFillAssistViews should return Login Password when htmlInfo matches password clause`() { 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 535b2de6f49..ae6713e032a 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 @@ -1625,8 +1625,9 @@ class ViewNodeExtensionsTest { assertEquals(AutofillView.Identity.PostalCode(data = autofillViewData), actual) } + @Suppress("MaxLineLength") @Test - fun `toAutofillView should return AutofillView Identity PhoneFull when autofillHints match`() { + fun `toAutofillView should return AutofillView Login Username when phone autofillHints match`() { every { viewNode.autofillHints } returns arrayOf(View.AUTOFILL_HINT_PHONE) val actual = viewNode.toAutofillView( @@ -1634,7 +1635,7 @@ class ViewNodeExtensionsTest { isIdentityAutofillEnabled = true, ) - assertEquals(AutofillView.Identity.PhoneFull(data = autofillViewData), actual) + assertEquals(AutofillView.Login.Username(data = autofillViewData), actual) } //endregion Identity: official autofillHints dispatch (toAutofillView) @@ -1654,7 +1655,6 @@ class ViewNodeExtensionsTest { "province" to AutofillView.Identity.AddressRegion(data = autofillViewData), "country" to AutofillView.Identity.AddressCountry(data = autofillViewData), "postalcode" to AutofillView.Identity.PostalCode(data = autofillViewData), - "mobile" to AutofillView.Identity.PhoneFull(data = autofillViewData), "company" to AutofillView.Identity.Company(data = autofillViewData), "socialsecurity" to AutofillView.Identity.Ssn(data = autofillViewData), "passport" to AutofillView.Identity.PassportNumber(data = autofillViewData), @@ -1674,6 +1674,20 @@ class ViewNodeExtensionsTest { } } + @Suppress("MaxLineLength") + @Test + fun `toAutofillView should return AutofillView Login Username when idEntry matches phone heuristic and IdentityAutofill is enabled`() { + setupUnsupportedInputFieldViewNode() + every { viewNode.idEntry } returns "mobile" + + val actual = viewNode.toAutofillView( + parentWebsite = null, + isIdentityAutofillEnabled = true, + ) + + assertEquals(AutofillView.Login.Username(data = autofillViewData), actual) + } + //region Identity: flag-off gate (isIdentityAutofillEnabled = false) @Suppress("MaxLineLength")