testing-library / user-event

🐕 Simulate user events
https://testing-library.com/user-event
MIT License
2.19k stars 249 forks source link

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

Open hesxenon opened 1 week ago

hesxenon commented 1 week ago

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/user-event@14.5.2?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 commented 1 week ago

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.