Memory leak caused by Safari bug with input element not being garbage collected

[loredanaspataru]

The memory leak issue is caused by a Safari bug with input elements that aren't garbage collected properly. This happens because the browser runs an extension used by Autofill code that retains references to the input elements. SWC components that are impacted by this bug are sp-textfield, sp-search, sp-number-field.

For large apps that use web components extensively like ours, this results in a considerable number of detached DOM subtrees not being garbage collected because of the input elements in the tree.

A couple of my colleagues investigated this issue last year and they put together a patch for TextfieldBase. The solution they came up with, after multiple rounds of testing, is to wrap the input in a form element for WebKit and manually detach the form from the tree on disconnectedCallback. Although this doesn't entirely solve the memory leak issue, it mitigates the impact to just the input and form wrapper elements being retained in memory, while the other nodes are garbage collected properly.

I've added here the patch that we currently use for @spectrum-web-components/textfield: 0.33.2.

Based on Kin Blas's work, I've put together a project to test and showcase how using the patched version impacts the items being retained in the DOM tree, using sp-textfield instead of HTML input element.

The script builds a snapshot of the DOM tree and uses WeakRefs to track which nodes are retained after they are detached from the tree. You can see here how the results look for the patched VS unpatched version:

The project used to test the impact of Textfield and patched changes can be found at https://github.com/loredanaspataru/swc-input-patch-test.

One big pain for our project is that each time we do an upgrade we have to maintain the patch and test that everything still works as expected. That's why we wanted to start a discussion here to check if introducing such changes in the actual SWC component would be a viable option.

Or if you have any other suggestions that might help in minimizing bug impact or maintenance work for this issue, that would also help us a lot. What do you think?

[Westbrook]

A couple of questions:

• While working on an element patch, did you consider an application patch that specifically wrapped all of the elements in question in <form> element to fully manage this at the application level?
• Have you investigated whether this Safari issue persists when the custom elements in question are fully prepared for Form Association or not?
• You mention this being related to an "autofill" plugin... in the case that autofill is turned off at the element level, does this issue persist?
• Are none of the remaining <input /> wrapping elements (sp-checkbox, sp-radio, sp-switch, et al) effected by this situation?
  • If not, could a similar work around involve switching the <input> element to a non-leaking type at disconnect time, rather than changing the larger DOM structure?
• You list sp-search as exhibiting this behavior, yet it already has a <form> element internal to its DOM structure. Can you share more about the leak and you proposed workaround in this context?

[loredanaspataru]

At the time of the investigation, multiple solutions were considered both at the app and SWC level and my understanding was that this was the one that best mitigates the impact of the leak issue to just the form wrapper.

For sp-search specifically, although it's already wrapped in a form element, it seems that form events aren't cleaned up properly and that's why the form wrapper in Textfield adds the events using addEventListener and manually removes them on disconnect.

I've added some tests for different HTML input types and it looks like input type radio, checkbox, and range don't have the memory leak issue. However, changing the input type on disconnect, as you suggested, doesn't fix the issue, because by the time we change the input type the reference to the input element is already stored by Safari's extension.

To simplify the patch, I've removed the form wrapper element and I've just added the input and form event listeners in firstUpdated (added here because the reference to input/form elements wasn't available in connectedCallback). Also, removed them on disconnectedCallback removing the usage of lit @ expression for adding the events.

• textfield patch
• search patch

This seems to work in the test env, with just the input being leaked for sp-textfield and form and input for sp-search.
However, we still need to confirm this would work as expected for us by updating the patches in our app.

Do you see any issues with this approach? Could that be something we can introduce to SWC?

[loredanaspataru]

@Westbrook I have some updates on this issue. After checking how the above changes work in our app, besides adding and removing form event listeners manually instead of using lit @ expression, we also have to remove the form element on disconnect.

Doing this results in the form element being detached from our panels, thus allowing the panels to be garbage collected properly. I also needed to apply same logic for adding/removing manually event for clear button because the panels still leak when search element has value, because of the clear button.

Following the same logic, in order to fix the issue for textfield and number-field I've added a wrapper form element for each of them and confirmed that only the form element is leaked and the panels are garbage collected. In order to avoid having form in form, Search would need to extend TextfieldBase instead of Textfield as it does now.

I've created a draft PR to be easier to follow the logic involved #4250.

I understand these changes are not trivial to introduce, especially since they don't fix completely the Safari bug and rather provide a workaround to keep leaks to a minimum. What do you think, should we go further with the work of adding this to SWC or keep using the patch we have?