Revert mutation observer for forms #396
Conversation
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
Signed-off-by: Emanuele Feliziani <feliziani.emanuele@gmail.com>
| this.formAnalyzer = new FormAnalyzer(this.form, input, this.matching) | ||
| this.recategorizeAllInputs() | ||
| return this | ||
| } |
There was a problem hiding this comment.
This is what was causing the problem. Basically, in certain edge cases the container of the field (what here we call this.form) could become the body. When that happened, any new field was hitting this conditional thus dropping everything and starting from scratch. We were doing this for every single added field, which in Asana could be hundreds at a time as you scroll through large projects. This wasn't always hitting the failsafes because we kept destroying and re-building the form data.
The failsafe we built for the form-specific mutation observer wasn't running here because the trigger comes from outside (the scanner), not from the internal observer.
In our browser the slowdown was limited (still massive), but on Firefox it could hang for several seconds and show the warning.
| foundForm.addInput(input) | ||
| } else { | ||
| this.stopScanner('The form has too many inputs, destroying.') | ||
| } |
There was a problem hiding this comment.
I've moved this check in the scanner as well for added security. I have a few follow up tasks in Asana to try and understand this flow better and see how we can make it more resilient to breakage.
| @@ -8,7 +8,7 @@ import {test as base} from '@playwright/test' | |||
| */ | |||
| const test = base.extend({}) | |||
|
|
|||
| test.describe('Mutating form page', () => { | |||
| test.describe.skip('Mutating form page', () => { | |||
There was a problem hiding this comment.
Just switching the test off for now.
| } | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
This wasn't directly related to the breakage, but I'm dropping it now before investigating things further in the next few days.
Task/Issue URL: https://app.asana.com/0/1205691418983700/1205691418983700 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/8.4.2 ## Description Updates Autofill to version [8.4.2](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/8.4.2). ### Autofill 8.4.2 release notes ## What's Changed * Ema/perf followups by @GioSensation in duckduckgo/duckduckgo-autofill#386 * Fix handler regression by @GioSensation in duckduckgo/duckduckgo-autofill#392 * Bump markdown-it from 13.0.1 to 13.0.2 by @dependabot in duckduckgo/duckduckgo-autofill#388 * Revert mutation observer for forms by @GioSensation in duckduckgo/duckduckgo-autofill#396 **Full Changelog**: duckduckgo/duckduckgo-autofill@8.4.1...8.4.2 ## 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 <GioSensation@users.noreply.github.com>
Task/Issue URL: https://app.asana.com/0/1205691418983700/1205691418983700 Autofill Release: https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/8.4.2 ## Description Updates Autofill to version [8.4.2](https://github.com/duckduckgo/duckduckgo-autofill/releases/tag/8.4.2). ### Autofill 8.4.2 release notes ## What's Changed * Ema/perf followups by @GioSensation in duckduckgo/duckduckgo-autofill#386 * Fix handler regression by @GioSensation in duckduckgo/duckduckgo-autofill#392 * Bump markdown-it from 13.0.1 to 13.0.2 by @dependabot in duckduckgo/duckduckgo-autofill#388 * Revert mutation observer for forms by @GioSensation in duckduckgo/duckduckgo-autofill#396 **Full Changelog**: duckduckgo/duckduckgo-autofill@8.4.1...8.4.2 ## 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 <GioSensation@users.noreply.github.com>
Reviewer: @shakyShane
Asana: https://app.asana.com/0/0/1205690199450910/f
CC: @kzar
Description
Reverts the code that observes for form changes. This was causing the script to spin out of control in certain cases. To clarify, it wasn't the observer itself, but the way we were re-scanning the page when new fields were added (added in version 8.1.0). This is just a patch to stop the bleeding, I will follow up with a more comprehensive set of fixes as roughly outlined here.
Steps to test
To repro the issue:
With this version it should not trigger the warning or freeze the page.