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 66ff6aca71..7405a9d0d4 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 @@ -123,14 +123,29 @@ class AutofillParserImpl( val urlBarWebsite = traversalDataList .flatMap { it.urlBarWebsites } .firstOrNull() - val autofillViews = traversalDataList.toAutofillViews(urlBarWebsite = urlBarWebsite) + // Heuristic views: the focused node's candidates with unfillable (Unused) fields removed, + // falling back to all fillable views when nothing has focus. + val autofillViews = traversalDataList + .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite) { + it !is AutofillView.Unused + } + + val isFillAssistEnabled = featureFlagManager + .getFeatureFlag(FlagKey.FillAssistTargetingRules) && + settingsRepository.isFillAssistEnabled // Find the focused view, or fallback to the first fillable item on the screen (so - // we at least have something to hook into) - val focusedView = autofillViews - .firstOrNull { it.data.isFocused } - ?: autofillViews.firstOrNull() - // The view is unfillable if there are no focused views. + // we at least have something to hook into). If heuristics found nothing at all, only + // give fill-assist a chance to rescue the page when it's actually enabled -- otherwise + // the view is unfillable since there are no focused views. + val focusedView = autofillViews.firstFocusedOrNull() + ?: if (isFillAssistEnabled) { + traversalDataList + .selectCandidateAutofillViews(urlBarWebsite = urlBarWebsite) + .firstFocusedOrNull() + } else { + null + } ?: return AutofillRequest.Unfillable val packageName = traversalDataList.buildPackageNameOrNull( @@ -143,9 +158,6 @@ class AutofillParserImpl( return AutofillRequest.Unfillable } - val isFillAssistEnabled = featureFlagManager - .getFeatureFlag(FlagKey.FillAssistTargetingRules) && - settingsRepository.isFillAssistEnabled val effectiveViews = if (isFillAssistEnabled) { autofillViews.toEffectiveViews( assistStructure = assistStructure, @@ -157,9 +169,7 @@ class AutofillParserImpl( autofillViews } - val effectiveFocusedView = effectiveViews - .firstOrNull { it.data.isFocused } - ?: effectiveViews.firstOrNull() + val effectiveFocusedView = effectiveViews.firstFocusedOrNull() ?: return AutofillRequest.Unfillable // Choose the first focused partition of data for fulfillment. @@ -177,8 +187,9 @@ class AutofillParserImpl( } is AutofillView.Unused -> { - // The view is unfillable since the field is not meant to be used for autofill. - // This will never happen since we filter out all unused views above. + // This will never happen: the heuristic path filters out Unused views, and the + // fill-assist path never constructs one (toAutofillViewForFieldKey has no Unused + // case). return AutofillRequest.Unfillable } } @@ -233,19 +244,25 @@ class AutofillParserImpl( when (focusedView) { is AutofillView.Card -> rule.category in CARD_FILL_ASSIST_CATEGORIES is AutofillView.Login -> rule.category in LOGIN_FILL_ASSIST_CATEGORIES - is AutofillView.Unused -> false + is AutofillView.Unused -> { + rule.category in LOGIN_FILL_ASSIST_CATEGORIES || + rule.category in CARD_FILL_ASSIST_CATEGORIES + } } } + if (!coversCurrentPartition) return this - return if (coversCurrentPartition) { - Timber.d("FillAssist invoked for this autofill attempt") - assistStructure.buildFillAssistViews( - hostRules = hostRules, - urlBarWebsite = urlBarWebsite, - ) - } else { - this - } + val fillAssistViews = assistStructure.buildFillAssistViews( + hostRules = hostRules, + urlBarWebsite = urlBarWebsite, + ) + // Fill-assist is authoritative for a partition its rules cover (guarded by + // coversCurrentPartition above), so its views are used even when empty: for Login/Card + // that discards the already heuristically-confirmed views, and for the Unused rescue path + // there were no heuristic views to fall back to. Reaching here means fill-assist has taken + // over this attempt (an empty result leaves the request Unfillable). + Timber.d("FillAssist invoked for this autofill attempt") + return fillAssistViews } } @@ -264,26 +281,32 @@ private fun AssistStructure.traverse(): List = } /** - * Assembles the [AutofillView] list from this [ViewNodeTraversalData] list. - * Take only the autofill views from the node that currently has focus. - * Then remove all the fields that cannot be filled with data. - * We fall back to taking all the fillable views if nothing has focus. + * Selects the autofill views from the node that currently has focus, or falls back to all + * fillable views if nothing has focus. The optional [predicate] filters the views *before* the + * emptiness/fallback check, so callers that only want fillable fields (e.g. excluding + * [AutofillView.Unused]) still get the multi-window fallback applied to the filtered set. By + * default no views are filtered out. */ -private fun List.toAutofillViews( +private fun List.selectCandidateAutofillViews( urlBarWebsite: String?, + predicate: (AutofillView) -> Boolean = { true }, ): List { val viewsLists = map { it.autofillViews } - val autofillViewLists = viewsLists + val candidates = viewsLists .filter { views -> views.any { it.data.isFocused } } .flatten() - .filter { it !is AutofillView.Unused } + .filter(predicate) .takeUnless { it.isEmpty() } - ?: viewsLists - .flatten() - .filter { it !is AutofillView.Unused } - return autofillViewLists.map { it.updateWebsiteIfNecessary(website = urlBarWebsite) } + ?: viewsLists.flatten().filter(predicate) + return candidates.map { it.updateWebsiteIfNecessary(website = urlBarWebsite) } } +/** + * Returns the focused [AutofillView], or falls back to the first entry if none is focused. + */ +private fun List.firstFocusedOrNull(): AutofillView? = + firstOrNull { it.data.isFocused } ?: 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 cd741f6a3d..f1ee29df1f 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 @@ -106,17 +106,19 @@ class AutofillParserTests { // ViewNodeExtensionsKt class, so one mockkStatic call covers all three. mockkStatic(AssistStructure.ViewNode::toAutofillView) // Default stub for toAutofillViewData (same mocked class — no separate mockkStatic needed). + // Note: this is a mocked extension function, so the receiver occupies arg position 0 -- + // `autofillId` and `website` are therefore the second and third args, not first/second. every { any().toAutofillViewData(autofillId = any(), website = any()) } answers { AutofillView.Data( - autofillId = firstArg(), + autofillId = secondArg(), autofillOptions = emptyList(), autofillType = AUTOFILL_TYPE, isFocused = false, textValue = null, hasPasswordTerms = false, - website = secondArg(), + website = thirdArg(), ) } mockkStatic( @@ -809,6 +811,71 @@ class AutofillParserTests { } } + @Suppress("MaxLineLength") + @Test + fun `parse should fall back to another window's fillable view when the focused window yields only Unused views`() { + // Setup: two window nodes. The focused window (e.g. a browser url bar EditText with no + // hint) contributes only an Unused view, while a second, unfocused window holds the real + // Login.Username field -- a shape that occurs for the browsers in URL_BARS. The focused + // window's emptiness check must run AFTER Unused views are filtered out, so the request + // falls back to the fillable view in the other window instead of short-circuiting to + // Unfillable. Fill-assist stays disabled (default), so there is no rescue path. + val urlBarAutofillId: AutofillId = mockk() + val urlBarViewNode: AssistStructure.ViewNode = mockk { + every { this@mockk.autofillHints } returns emptyArray() + every { this@mockk.autofillId } returns urlBarAutofillId + 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.website } returns null + } + val urlBarWindowNode: AssistStructure.WindowNode = mockk { + every { this@mockk.rootViewNode } returns urlBarViewNode + } + every { assistStructure.windowNodeCount } returns 2 + every { assistStructure.getWindowNodeAt(0) } returns urlBarWindowNode + every { assistStructure.getWindowNodeAt(1) } returns loginWindowNode + val unusedFocusedView = AutofillView.Unused( + data = AutofillView.Data( + autofillId = urlBarAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = null, + ), + ) + val loginAutofillView = AutofillView.Login.Username( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + hasPasswordTerms = false, + website = URI, + ), + ) + every { urlBarViewNode.toAutofillView(parentWebsite = any()) } returns unusedFocusedView + every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify: the request falls back to the fillable Login.Username in the unfocused window. + val expected = AutofillRequest.Fillable( + ignoreAutofillIds = emptyList(), + inlinePresentationSpecs = inlinePresentationSpecs, + maxInlineSuggestionsCount = MAX_INLINE_SUGGESTION_COUNT, + packageName = PACKAGE_NAME, + partition = AutofillPartition.Login(views = listOf(loginAutofillView)), + uri = URI, + ) + assertEquals(expected, actual) + } + @Test fun `parse should return empty inline suggestions when inline autofill is disabled`() { // Setup @@ -1263,6 +1330,156 @@ class AutofillParserTests { assertEquals(expected, actual) } + @Suppress("MaxLineLength") + @Test + fun `parse should use fill-assist views when heuristics classify the focused view as Unused`() { + // Setup: heuristics found nothing recognizable (focused view is Unused), but fill-assist + // rules exist for this host and match. Fill-assist should rescue the request instead of + // it dying as Unfillable before fill-assist is ever consulted. + 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, + ), + ), + ), + ), + ), + ), + ) + val unusedLoginView = AutofillView.Unused( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = null, + ), + ) + every { any().matchesSelectorClause(any()) } returns true + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode + every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify: fill-assist rescued a page heuristics found nothing on. + val fillAssistData = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = false, + textValue = null, + hasPasswordTerms = false, + website = WEBSITE, + ) + 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 = fillAssistData)), + ), + uri = FILL_ASSIST_URI, + ) + assertEquals(expected, actual) + } + + @Suppress("MaxLineLength") + @Test + fun `parse should return Unfillable when heuristics classify the focused view as Unused and fill-assist matches nothing`() { + // Fill-assist takes over the attempt but matches no fields, and the Unused rescue path has + // no heuristic views to fall back to -- so the request ends up Unfillable. + 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, + ), + ), + ), + ), + ), + ), + ) + val unusedLoginView = AutofillView.Unused( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = null, + ), + ) + // matchesSelectorClause defaults to false per setup -- fill-assist matches nothing. + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode + every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify + assertEquals(AutofillRequest.Unfillable, actual) + } + + @Suppress("MaxLineLength") + @Test + fun `parse should return Unfillable when fill-assist is disabled even though heuristics classify the focused view as Unused`() { + // Setup: fill-assist stays disabled (default). The rescue path must never activate when + // the feature flag/setting are off, regardless of what fill-assist rules would say. + every { any().buildUriOrNull(PACKAGE_NAME) } returns FILL_ASSIST_URI + val unusedLoginView = AutofillView.Unused( + data = AutofillView.Data( + autofillId = loginAutofillId, + autofillOptions = emptyList(), + autofillType = AUTOFILL_TYPE, + isFocused = true, + textValue = null, + hasPasswordTerms = false, + website = null, + ), + ) + every { assistStructure.windowNodeCount } returns 1 + every { assistStructure.getWindowNodeAt(0) } returns loginWindowNode + every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedLoginView + + // Test + val actual = parser.parse(autofillAppInfo = autofillAppInfo, fillRequest = fillRequest) + + // Verify + assertEquals(AutofillRequest.Unfillable, actual) + } + @Suppress("MaxLineLength") @Test fun `parse should deduplicate when a child node returns an AutofillView with the same autofillId already claimed by a non-Unused parent view`() {