Skip to content

PM-26896: Fix Autofill ancestry #6014

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

PM-26896: Fix Autofill ancestry #6014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -170,7 +169,7 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> =
.mapNotNull { windowNode ->
windowNode
.rootViewNode
?.traverse(parentWebsite = null)
?.traverse()
?.updateForMissingPasswordFields()
?.updateForMissingUsernameFields()
}
Expand Down Expand Up @@ -248,9 +247,7 @@ private fun ViewNodeTraversalData.copyAndMapAutofillViews(
* Recursively traverse this [AssistStructure.ViewNode] and all of its descendants. Convert the
* data into [ViewNodeTraversalData].
*/
private fun AssistStructure.ViewNode.traverse(
parentWebsite: String?,
): ViewNodeTraversalData {
private fun AssistStructure.ViewNode.traverse(): ViewNodeTraversalData {
// Set up mutable lists for collecting valid AutofillViews and ignorable view ids.
val mutableAutofillViewList: MutableList<AutofillView> = mutableListOf()
val mutableIgnoreAutofillIdList: MutableList<AutofillId> = mutableListOf()
Expand All @@ -260,15 +257,15 @@ 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)

// Recursively traverse all of this view node's children.
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)
Expand Down
6 changes: 2 additions & 4 deletions app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down
46 changes: 21 additions & 25 deletions app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ class AutofillParserTests {
every { [email protected] } returns 0
every { [email protected] } returns null
every { [email protected] } returns false
every { [email protected](parentWebsite = any()) } returns null
every { [email protected]() } returns null
every { [email protected] } returns null
}
// `invalidChildViewNode` simulates the OS assigning a node's idPackage to "android", which
Expand All @@ -196,7 +196,7 @@ class AutofillParserTests {
every { [email protected] } returns 0
every { [email protected] } returns ID_PACKAGE_ANDROID
every { [email protected] } returns false
every { [email protected](parentWebsite = any()) } returns null
every { [email protected]() } returns null
every { [email protected] } returns null
}
val parentAutofillHint = View.AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_YEAR
Expand All @@ -217,7 +217,7 @@ class AutofillParserTests {
every { [email protected] } returns arrayOf(parentAutofillHint)
every { [email protected] } returns parentAutofillId
every { [email protected] } returns null
every { [email protected](parentWebsite = any()) } returns parentAutofillView
every { [email protected]() } returns parentAutofillView
every { [email protected] } returns 2
every { [email protected](0) } returns childViewNode
every { [email protected](1) } returns invalidChildViewNode
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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.
Expand Down
Loading
Loading