Skip to content

Commit d37eb91

Browse files
david-livefrontdavid-livefront
committed
PM-26896: Fix Autofill ancestry
1 parent f7cbcd2 commit d37eb91

File tree

4 files changed

+76
-111
lines changed

4 files changed

+76
-111
lines changed

‎app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserImpl.kt‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import com.x8bit.bitwarden.data.autofill.util.buildUriOrNull
1313
import com.x8bit.bitwarden.data.autofill.util.getInlinePresentationSpecs
1414
import com.x8bit.bitwarden.data.autofill.util.getMaxInlineSuggestionsCount
1515
import com.x8bit.bitwarden.data.autofill.util.toAutofillView
16-
import com.x8bit.bitwarden.data.autofill.util.website
1716
import com.x8bit.bitwarden.data.platform.repository.SettingsRepository
1817
import timber.log.Timber
1918

@@ -170,7 +169,7 @@ private fun AssistStructure.traverse(): List<ViewNodeTraversalData> =
170169
.mapNotNull { windowNode ->
171170
windowNode
172171
.rootViewNode
173-
?.traverse(parentWebsite = null)
172+
?.traverse()
174173
?.updateForMissingPasswordFields()
175174
?.updateForMissingUsernameFields()
176175
}
@@ -248,25 +247,23 @@ private fun ViewNodeTraversalData.copyAndMapAutofillViews(
248247
* Recursively traverse this [AssistStructure.ViewNode] and all of its descendants. Convert the
249248
* data into [ViewNodeTraversalData].
250249
*/
251-
private fun AssistStructure.ViewNode.traverse(
252-
parentWebsite: String?,
253-
): ViewNodeTraversalData {
250+
private fun AssistStructure.ViewNode.traverse(): ViewNodeTraversalData {
254251
// Set up mutable lists for collecting valid AutofillViews and ignorable view ids.
255252
val mutableAutofillViewList: MutableList<AutofillView> = mutableListOf()
256253
val mutableIgnoreAutofillIdList: MutableList<AutofillId> = mutableListOf()
257254
var idPackage: String? = this.idPackage
258255

259256
// Try converting this `ViewNode` into an `AutofillView`. If a valid instance is returned, add
260257
// it to the list. Otherwise, ignore the `AutofillId` associated with this `ViewNode`.
261-
toAutofillView(parentWebsite = parentWebsite)
258+
toAutofillView()
262259
?.run(mutableAutofillViewList::add)
263260
?: autofillId?.run(mutableIgnoreAutofillIdList::add)
264261

265262
// Recursively traverse all of this view node's children.
266263
for (i in 0 until childCount) {
267264
// Extract the traversal data from each child view node and add it to the lists.
268265
getChildAt(i)
269-
.traverse(parentWebsite = website)
266+
.traverse()
270267
.let { viewNodeTraversalData ->
271268
viewNodeTraversalData.autofillViews.forEach(mutableAutofillViewList::add)
272269
viewNodeTraversalData.ignoreAutofillIds.forEach(mutableIgnoreAutofillIdList::add)

‎app/src/main/kotlin/com/x8bit/bitwarden/data/autofill/util/ViewNodeExtensions.kt‎

Lines changed: 22 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -49,37 +49,28 @@ private val AssistStructure.ViewNode.isInputField: Boolean
4949
* doesn't contain a valid autofillId, it isn't an a view setup for autofill, so we return null. If
5050
* it doesn't have a supported hint and isn't an input field, we also return null.
5151
*/
52-
fun AssistStructure.ViewNode.toAutofillView(
53-
parentWebsite: String?,
54-
): AutofillView? =
55-
this
56-
.autofillId
57-
// We only care about nodes with a valid `AutofillId`.
58-
?.let { nonNullAutofillId ->
59-
if (supportedAutofillHint != null || this.isInputField) {
60-
val autofillOptions = this
61-
.autofillOptions
62-
.orEmpty()
63-
.map { it.toString() }
64-
65-
val autofillViewData = AutofillView.Data(
66-
autofillId = nonNullAutofillId,
67-
autofillOptions = autofillOptions,
68-
autofillType = this.autofillType,
69-
isFocused = this.isFocused,
70-
textValue = this.autofillValue?.extractTextValue(),
71-
hasPasswordTerms = this.hasPasswordTerms(),
72-
website = this.website ?: parentWebsite,
73-
)
74-
buildAutofillView(
75-
autofillOptions = autofillOptions,
76-
autofillViewData = autofillViewData,
77-
autofillHint = supportedAutofillHint,
78-
)
79-
} else {
80-
null
81-
}
82-
}
52+
fun AssistStructure.ViewNode.toAutofillView(): AutofillView? {
53+
val nonNullAutofillId = this.autofillId ?: return null
54+
if (this.supportedAutofillHint == null && !this.isInputField) return null
55+
val autofillOptions = this
56+
.autofillOptions
57+
.orEmpty()
58+
.map { it.toString() }
59+
val autofillViewData = AutofillView.Data(
60+
autofillId = nonNullAutofillId,
61+
autofillOptions = autofillOptions,
62+
autofillType = this.autofillType,
63+
isFocused = this.isFocused,
64+
textValue = this.autofillValue?.extractTextValue(),
65+
hasPasswordTerms = this.hasPasswordTerms(),
66+
website = this.website,
67+
)
68+
return buildAutofillView(
69+
autofillOptions = autofillOptions,
70+
autofillViewData = autofillViewData,
71+
autofillHint = this.supportedAutofillHint,
72+
)
73+
}
8374

8475
/**
8576
* The first supported autofill hint for this view node, or null if none are found.

‎app/src/test/kotlin/com/x8bit/bitwarden/data/autofill/parser/AutofillParserTests.kt‎

Lines changed: 21 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ class AutofillParserTests {
183183
every { this@mockk.childCount } returns 0
184184
every { this@mockk.idPackage } returns null
185185
every { this@mockk.isFocused } returns false
186-
every { this@mockk.toAutofillView(parentWebsite = any()) } returns null
186+
every { this@mockk.toAutofillView() } returns null
187187
every { this@mockk.website } returns null
188188
}
189189
// `invalidChildViewNode` simulates the OS assigning a node's idPackage to "android", which
@@ -196,7 +196,7 @@ class AutofillParserTests {
196196
every { this@mockk.childCount } returns 0
197197
every { this@mockk.idPackage } returns ID_PACKAGE_ANDROID
198198
every { this@mockk.isFocused } returns false
199-
every { this@mockk.toAutofillView(parentWebsite = any()) } returns null
199+
every { this@mockk.toAutofillView() } returns null
200200
every { this@mockk.website } returns null
201201
}
202202
val parentAutofillHint = View.AUTOFILL_HINT_CREDIT_CARD_EXPIRATION_YEAR
@@ -217,7 +217,7 @@ class AutofillParserTests {
217217
every { this@mockk.autofillHints } returns arrayOf(parentAutofillHint)
218218
every { this@mockk.autofillId } returns parentAutofillId
219219
every { this@mockk.idPackage } returns null
220-
every { this@mockk.toAutofillView(parentWebsite = any()) } returns parentAutofillView
220+
every { this@mockk.toAutofillView() } returns parentAutofillView
221221
every { this@mockk.childCount } returns 2
222222
every { this@mockk.getChildAt(0) } returns childViewNode
223223
every { this@mockk.getChildAt(1) } returns invalidChildViewNode
@@ -303,8 +303,8 @@ class AutofillParserTests {
303303
partition = autofillPartition,
304304
uri = URI,
305305
)
306-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
307-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
306+
every { cardViewNode.toAutofillView() } returns cardAutofillView
307+
every { loginViewNode.toAutofillView() } returns loginAutofillView
308308

309309
// Test
310310
val actual = parser.parse(
@@ -366,8 +366,8 @@ class AutofillParserTests {
366366
partition = autofillPartition,
367367
uri = URI,
368368
)
369-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
370-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
369+
every { cardViewNode.toAutofillView() } returns cardAutofillView
370+
every { loginViewNode.toAutofillView() } returns loginAutofillView
371371

372372
// Test
373373
val actual = parser.parse(
@@ -422,7 +422,7 @@ class AutofillParserTests {
422422
partition = autofillPartition,
423423
uri = URI,
424424
)
425-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns unusedAutofillView
425+
every { loginViewNode.toAutofillView() } returns unusedAutofillView
426426

427427
// Test
428428
val actual = parser.parse(
@@ -525,13 +525,9 @@ class AutofillParserTests {
525525
partition = autofillPartition,
526526
uri = URI,
527527
)
528-
every { rootViewNode.toAutofillView(parentWebsite = any()) } returns null
529-
every {
530-
hiddenUserNameViewNode.toAutofillView(parentWebsite = any())
531-
} returns unusedAutofillView
532-
every {
533-
passwordViewNode.toAutofillView(parentWebsite = any())
534-
} returns loginPasswordAutofillView
528+
every { rootViewNode.toAutofillView() } returns null
529+
every { hiddenUserNameViewNode.toAutofillView() } returns unusedAutofillView
530+
every { passwordViewNode.toAutofillView() } returns loginPasswordAutofillView
535531

536532
// Test
537533
val actual = parser.parse(
@@ -593,8 +589,8 @@ class AutofillParserTests {
593589
partition = autofillPartition,
594590
uri = URI,
595591
)
596-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
597-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
592+
every { cardViewNode.toAutofillView() } returns cardAutofillView
593+
every { loginViewNode.toAutofillView() } returns loginAutofillView
598594

599595
// Test
600596
val actual = parser.parse(
@@ -657,8 +653,8 @@ class AutofillParserTests {
657653
partition = autofillPartition,
658654
uri = URI,
659655
)
660-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
661-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
656+
every { cardViewNode.toAutofillView() } returns cardAutofillView
657+
every { loginViewNode.toAutofillView() } returns loginAutofillView
662658

663659
// Test
664660
val actual = parser.parse(
@@ -721,8 +717,8 @@ class AutofillParserTests {
721717
partition = autofillPartition,
722718
uri = URI,
723719
)
724-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
725-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
720+
every { cardViewNode.toAutofillView() } returns cardAutofillView
721+
every { loginViewNode.toAutofillView() } returns loginAutofillView
726722

727723
// Test
728724
val actual = parser.parse(
@@ -785,8 +781,8 @@ class AutofillParserTests {
785781
partition = autofillPartition,
786782
uri = URI,
787783
)
788-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
789-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
784+
every { cardViewNode.toAutofillView() } returns cardAutofillView
785+
every { loginViewNode.toAutofillView() } returns loginAutofillView
790786

791787
// Test
792788
val actual = parser.parse(
@@ -841,8 +837,8 @@ class AutofillParserTests {
841837
"blockListedUri.com",
842838
"blockListedAgainUri.com",
843839
)
844-
every { cardViewNode.toAutofillView(parentWebsite = any()) } returns cardAutofillView
845-
every { loginViewNode.toAutofillView(parentWebsite = any()) } returns loginAutofillView
840+
every { cardViewNode.toAutofillView() } returns cardAutofillView
841+
every { loginViewNode.toAutofillView() } returns loginAutofillView
846842
every { settingsRepository.blockedAutofillUris } returns remoteBlockList
847843

848844
// A function for asserting that a block listed URI results in an unfillable request.

0 commit comments

Comments
 (0)