Skip to content
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

Merged
merged 17 commits into from
Jun 28, 2024

Conversation

dbajpeyi
Copy link
Collaborator

@dbajpeyi dbajpeyi commented Jun 19, 2024

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 skip select type input, if the input was added to the Set touched.

Additionally, select type elements were not added to this.touched, as the handler that attaches it was intentionally skipped in the code. The new approach creates a separate handler called handleSelect in Form.js to simply add the select element to the touched Set.

Steps to test

(Pre-requisite: Have an identity added to autofill)

  1. Locally build your mac+autofill environment,
  2. Go to https://fill.dev/form/identity-simple in the locally built browser,
  3. Change a select field (country/state),
  4. Autofill one of the text field inputs (name, email e.g),
  5. The select field should not get autofilled,

Credit card
(Pre-requisite: have credit card added to your autofill)

  1. Go to https://fill.dev/form/credit-card-simple
  2. Change the select field (expiry date),
  3. Autofill on card number,
  4. Expiry date should get autofilled.

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

@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from b08479b to d272e2c Compare June 20, 2024 09:39
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 94e818d to 4855431 Compare June 21, 2024 08:45
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch 2 times, most recently from 471c405 to f613cae Compare June 21, 2024 09:34
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from f613cae to 9ce75a3 Compare June 21, 2024 10:38
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 63cc475 to 433448a Compare June 21, 2024 11:22
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 120bcf6 to 2dc94a1 Compare June 21, 2024 11:33
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch 2 times, most recently from dffbb9c to e821ff2 Compare June 25, 2024 08:59
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch 5 times, most recently from 75325d3 to b847e94 Compare June 25, 2024 09:17
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from b847e94 to 59e4674 Compare June 25, 2024 09:26
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from e6fd389 to e8f96c5 Compare June 25, 2024 09:44
@dbajpeyi dbajpeyi changed the title wip: remove 'select' type check in autofill input [Autofill select input] Skip select input if it was changed already Jun 25, 2024
@dbajpeyi dbajpeyi changed the title [Autofill select input] Skip select input if it was changed already [select field] Skip autofilling select input if it was changed already Jun 25, 2024
@dbajpeyi dbajpeyi marked this pull request as ready for review June 25, 2024 14:24
@dbajpeyi dbajpeyi requested a review from shakyShane June 25, 2024 14:24
@dbajpeyi dbajpeyi changed the title [select field] Skip autofilling select input if it was changed already [Form] Skip autofilling select input if it was changed already Jun 25, 2024
Copy link
Collaborator

@shakyShane shakyShane left a 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.

integration-test/tests/email-autofill.macos.spec.js Outdated Show resolved Hide resolved
integration-test/tests/email-autofill.macos.spec.js Outdated Show resolved Hide resolved
src/Form/Form.js Outdated Show resolved Hide resolved
src/Form/Form.js Outdated Show resolved Hide resolved
src/Form/Form.js Show resolved Hide resolved
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 4539e23 to 9c6837e Compare June 27, 2024 13:47
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 9c6837e to f32b965 Compare June 27, 2024 13:56
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch 3 times, most recently from 64cbc7f to 869ea29 Compare June 27, 2024 15:58
@dbajpeyi dbajpeyi force-pushed the dbajpeyi/autofill-select-bug branch from 869ea29 to 151066a Compare June 27, 2024 16:02
Copy link
Collaborator

@shakyShane shakyShane left a 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 💪

@dbajpeyi
Copy link
Collaborator Author

dbajpeyi commented Jun 28, 2024

Thank you for helping me step up so quickly and for all the feedback!

@dbajpeyi dbajpeyi merged commit 7d6a5fd into main Jun 28, 2024
1 check passed
@dbajpeyi dbajpeyi deleted the dbajpeyi/autofill-select-bug branch June 28, 2024 09:06
CDRussell pushed a commit to duckduckgo/Android that referenced this pull request Aug 2, 2024
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants