-
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
scanner: discard Form instances with no classified inputs #574
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When encountering <form/>s with no fillable inputs, the Scanner and Form classes previously maintained MutationObservers for each <form/>. This allowed the global MutationObserver to detect newly-added <input/>s and attribute them to existing Form instances without having to create a new Form, but also meant we maintained observers and listeners that often went unused. For performance reasons, we stop processing <form/> elements once 30 Form instances have been created. This allowed a large number of unfillable <form/>s could consume the entire Form pool before an actually-fillable <form/> is encountered. Immediately `destroy()` a Form instance that contains no fillable <input/>s, and avoid adding that instance to a Scanner's set of known Form instances. This ensures form-scoped listeners and observers are only maintained when a fillable field is present. When a new <input/> is added the global MutationObserver and Scanner create a new Form instance to manage it, enabling slightly lazier listener/observer initialization.
e28474f
to
b9c8959
Compare
dbajpeyi
approved these changes
Jul 2, 2024
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.
Looking very good to me, and tests are good too. Tested locally and the change works too! Merging it.
This was referenced Jul 23, 2024
This was referenced Jul 29, 2024
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Reviewer: @GioSensation (while @shakyShane is unavailable)
Asana: https://app.asana.com/0/1206682621538333/1207230997693280/f
Description
When encountering
<form/>
s with no fillable inputs, the Scanner and Form classes previously maintained MutationObservers for each<form/>
. This allowed the global MutationObserver to detect newly-added<input/>
s and attribute them to existing Form instances without having to create a new Form, but also meant we maintained observers and listeners that often went unused. For performance reasons, we stop processing<form/>
elements once 30 Form instances have been created. This allowed a large number of unfillable<form/>
s could consume the entire Form pool before an actually-fillable<form/>
is encountered.Immediately
destroy()
a Form instance that contains no fillable<input/>
s, and avoid adding that instance to a Scanner's set of known Form instances. This ensures form-scoped listeners and observers are only maintained when a fillable field is present. When a new<input/>
is added the global MutationObserver and Scanner create a new Form instance to manage it, enabling slightly lazier listener/observer initialization.Steps to test
npm run test
<form/>
elements).data-ddg-inputtype="credentials.username"
anddata-ddg-inputtype="credentials.password.current"
attributes