testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.27k stars 467 forks source link

*ByRole form seems not to find <form /> elements #1293

Open julienw opened 9 months ago

julienw commented 9 months ago

Prior issue #1021

Testcase:

it(`testcase`, async () => {
    render(
        <div>
            <h1>helloworld</h1>
            <form>
                <h2>content</h2>
                <label htmlFor="tinput">hello</label>
                <input type="text" id="tinput" />
            </form>
        </div>
    )

    expect(screen.getByRole("form")).toBeInTheDocument()
})

Result:

 FAIL  src/test/foo.test.js
  ● testcase

    TestingLibraryElementError: Unable to find an accessible element with the role "form"

    Here are the accessible roles:

      heading:

      Name "helloworld":
      <h1 />

      Name "content":
      <h2 />

      --------------------------------------------------
      textbox:

      Name "hello":
      <input
        id="tinput"
        type="text"
      />

      --------------------------------------------------

    Ignored nodes: comments, script, style
    <body>
      <div>
        <div>
          <h1>
            helloworld
          </h1>
          <form>
            <h2>
              content
            </h2>
            <label
              for="tinput"
            >
              hello
            </label>
            <input
              id="tinput"
              type="text"
            />
          </form>
        </div>
      </div>
    </body>

      15 |   );
      16 |
    > 17 |   expect(screen.getByRole('form')).toBeInTheDocument();
         |                 ^
      18 | });
      19 |

      at Object.getElementError (node_modules/@testing-library/dom/dist/config.js:37:19)
      at node_modules/@testing-library/dom/dist/query-helpers.js:76:38
      at node_modules/@testing-library/dom/dist/query-helpers.js:52:17
      at node_modules/@testing-library/dom/dist/query-helpers.js:95:19
      at Object.getByRole (src/test/foo.test.js:17:17)

In the previously mentioned bug, @eps1lon explained that this was following the roles from html-aam. However it looks like they've been updated, see https://www.w3.org/TR/core-aam-1.2/#role-map-form-nameless. It looks like that a form element without an accessible name gets a computed role form too. However they mention that it shouldn't be exposed as a landmark. Is that where my interpretation is wrong?

Thanks for your feedback!

julienw commented 9 months ago

It looks like it's been changed in https://github.com/w3c/html-aria/pull/402 (found that from the change on MDN in https://github.com/mdn/content/commit/e31cb5978e9f3c731c49db9ed0a15795b870e141#diff-25921a273c422a060a3204a4c8e46fb9d42be0ce77679b9d1e5094c9fc627985)

julienw commented 9 months ago

Should the fix be made in https://github.com/A11yance/aria-query/blob/main/src/etc/roles/literal/formRole.js ?

cmorten commented 8 months ago

Looking at:

I’m inclined to agree, all the specs point to the form element having the form role regardless of accessible name. As you describe the only condition is regarding whether it’s also considered a landmark.

cmorten commented 8 months ago

In addition to the update to aria-query, perhaps worth removing the comment in https://github.com/eps1lon/dom-accessibility-api/blob/main/sources/getRole.ts#L30 as well (although no code change needed as seems already assumed a form role)

jlp-craigmorten commented 8 months ago

Some more context:

The last one means that WAI-ARIA is somewhat in conflict with the AAM specs currently as it still states that an accessible name is required on a form (something that is a bug in in aria-query as it has accessibleNameRequired: false, currently).

If we assume that WAI-ARIA is updated (e.g. in 1.3) then we could ignore that aria-query has been inconsistent / incorrect till now on this one point.