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.

Sign up for GitHub

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

Jump to bottom

click leads to endless loop for custom elements in label #1237

Closed
hesxenon opened this issue Nov 5, 2024 · 4 comments · Fixed by #1238
Closed

click leads to endless loop for custom elements in label #1237

hesxenon opened this issue Nov 5, 2024 · 4 comments · Fixed by #1238
Labels
bug Something isn't working released

Comments

@hesxenon
Copy link
Contributor

hesxenon commented Nov 5, 2024 •
edited
Loading

Reproduction example

self contained html provided in prerequisites

Prerequisites

<!doctype html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />
    <title></title>
  </head>
  <body>
    <script type="module">
      import userEvent from "https://esm.sh/@testing-library/[email protected]?bundle";
      const user = userEvent.setup();

      class MyFoo extends HTMLElement {
        static formAssociated = true;
      }
      customElements.define("my-foo", MyFoo);

      const label = document.createElement("label");
      const myFoo = document.createElement("my-foo");
      label.append(myFoo);

      document.body.append(label);

      user.click(document.querySelector("label"));
    </script>
  </body>
</html>

loading this throws the following error:

isElementType.js:5 Uncaught (in promise) RangeError: Maximum call stack size exceeded
    at Function.isArray (<anonymous>)
    at c (isElementType.js:5:17)
    at T.click (click.js:17:32)
    at Object.ne (dispatchEvent.js:25:160)
    at c.type (click.js:23:22)
    at Object.ne (dispatchEvent.js:39:13)
    at c.type (click.js:23:22)
    at Object.ne (dispatchEvent.js:39:13)
    at c.type (click.js:23:22)
    at Object.ne (dispatchEvent.js:39:13)

Expected behavior

Should not lead to endless loop

Actual behavior

leads to endless loop

User-event version

14.5.2

Environment

nope.

Additional context

I tried debugging it a bit and found that the PR #850 in which the closest label of a click target is selected and that is supposed to break the loop - I'm not sure how?
@hesxenon hesxenon added bug Something isn't working needs assessment This needs to be looked at by a team member labels Nov 5, 2024
@hesxenon
Copy link
Contributor Author

hesxenon commented Nov 6, 2024

hmmmm, so after a bit of digging I found out that the following points hold true:

1.

clicking on a <input> that qualifies as a labeled control does not dispatch a click event, because the input element is matched here via a .closest self-match.

2.

clicking on a custom element that qualifies as a labeled control does dispatch a click event, because it cannot be matched in the same way. This behaviour then re-dispatches a click event whose behaviour re-dispatches a click event and so on until the exception above is thrown.

3.

the spec does not specify how user agents should behave sufficiently to warrant either way - dispatching the click event is allowed and not doing so is also allowed.

For example, on platforms where clicking a label activates the form control, clicking the label in the following snippet could trigger the user agent to fire a click event at the input element, as if the element itself had been triggered by the user: [...] On other platforms, the behavior in both cases might be just to focus the control, or to do nothing.

4.

the spec does specify that a user agent should treat labelable elements the same.

Form-associated custom elements are labelable elements, so for user agents where the label element's activation behavior impacts the labeled control, both built-in and custom elements will be impacted.

So no matter what the user-agent decides is okay as long as both, CEs and builtins, are handled the same way.

5.

while there are non-form-associated labelable elements there are no custom elements that are labelable but not form-associated.

This means, as I read it, that the activation behaviour of form-associated custom elements can be left to the user-agent since they should be treated "the same" as other labeled controls. Which in turn means that the correct change imho would be this:

-  const control = context && isElementType(context, 'label') && context.control
+  const control = context && isElementType(context, 'label') && !(target.constructor as {formAssociated?: boolean}).formAssociated && context.control

This way form-associated custom elements would also not have a defined click behaviour which prevents the endless loop.

@hesxenon hesxenon mentioned this issue Nov 6, 2024
Merged
2 tasks
@hesxenon
Copy link
Contributor Author

also relates to jsdom/jsdom#3790

@ph-fritsche ph-fritsche removed the needs assessment This needs to be looked at by a team member label Jan 13, 2025
@ph-fritsche
Copy link
Member

Great report for a non-obvious bug. Thanks for contributing to this library 💙

@github-actions GitHub Actions
Copy link

🎉 This issue has been resolved in version 14.6.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working released
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(click): skip explicit re-dispatch for face hesxenon/user-event
2 participants
@hesxenon @ph-fritsche