-
Notifications
You must be signed in to change notification settings - Fork 13
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.
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
[Form] Skip autofilling select input if it was changed already #580
Conversation
b08479b
to
d272e2c
Compare
94e818d
to
4855431
Compare
471c405
to
f613cae
Compare
f613cae
to
9ce75a3
Compare
63cc475
to
433448a
Compare
120bcf6
to
2dc94a1
Compare
dffbb9c
to
e821ff2
Compare
75325d3
to
b847e94
Compare
b847e94
to
59e4674
Compare
e6fd389
to
e8f96c5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dbajpeyi congrats on your first contribution! it certainly solves the problem 💪🏻
There are a few comments, mostly very small changes needed - and to double-check the new test cases can fail as expected.
4539e23
to
9c6837e
Compare
9c6837e
to
f32b965
Compare
64cbc7f
to
869ea29
Compare
869ea29
to
151066a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dbajpeyi - especially for working with me on the feedback :)
I love that we covered this with a new tests, and also that you are now testing additional things (with selects) that were previously absent 💪
Thank you for helping me step up so quickly and for all the feedback! |
Task/Issue URL: https://app.asana.com/0/1207920271697429/1207920271697429 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/12.1.0 ## Description Updates Autofill to version [12.1.0](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/12.1.0). ### Autofill 12.1.0 release notes ## What's Changed * [Form] Skip autofilling select input if it was changed already by @dbajpeyi in duckduckgo/duckduckgo-autofill#580 * scanner: discard Form instances with no classified inputs by @sjbarag in duckduckgo/duckduckgo-autofill#574 * [Credentials] Autofill when elements are in shadow by @dbajpeyi in duckduckgo/duckduckgo-autofill#592 * Update password-related json files (2024-07-19) by @daxmobile in duckduckgo/duckduckgo-autofill#607 * [Scanner] Don't stop scanner fully on max forms by @dbajpeyi in duckduckgo/duckduckgo-autofill#617 * tests: remove spurious logging from unit tests by @sjbarag in duckduckgo/duckduckgo-autofill#575 * Add credit card test form failure by @GioSensation in duckduckgo/duckduckgo-autofill#588 ## New Contributors * @dbajpeyi made their first contribution in duckduckgo/duckduckgo-autofill#580 **Full Changelog**: duckduckgo/duckduckgo-autofill@12.0.1...12.1.0 ## Steps to test This release has been tested during autofill development. For smoke test steps see [this task](https://app.asana.com/0/1198964220583541/1200583647142330/f). Co-authored-by: GioSensation <1828326+GioSensation@users.noreply.github.com>
Reviewer: @shakyShane
Asana: https://app.asana.com/0/1200930669568058/1204544631545889/f
Description
select
elements were simply autofilled every time, without considering if the input was already changed by a user action. This PR extends the behavior and adds logic to skipselect
type input, if the input was added to theSet
touched
.Additionally,
select
type elements were not added tothis.touched
, as thehandler
that attaches it was intentionally skipped in the code. The new approach creates a separate handler calledhandleSelect
inForm.js
to simply add the select element to thetouched
Set
.Steps to test
(Pre-requisite: Have an identity added to autofill)
Credit card
(Pre-requisite: have credit card added to your autofill)
Example videos
Amazon
fix-amazon.com.mp4
Amazon credit card
regression-cc-amazon.com.mp4
Zip code search
fix-usephonelookup.mp4
fill.dev
Fix.on.fill.dev.mp4