testing-library / dom-testing-library

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

Images with an empty alt attribute are not being assigned `presentation` role (and are still being found by testing library even though they are hidden from the accessibility tree) #1290

Closed ahayes91 closed 3 months ago

ahayes91 commented 4 months ago

Relevant code or config:

Component

export const ImageWithEmptyAlt = () => {
  return <img alt="" src="https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png" />
}

Test file

import React from 'react'
import {render, screen} from '@testing-library/react'
import {ImageWithEmptyAlt} from '../'

test('ImageWithEmptyAlt should not render an img role because the img has alt="" - this fails', () => {
  render(<ImageWithEmptyAlt />)
  expect(screen.queryByRole('img')).not.toBeInTheDocument();
});

test('ImageWithEmptyAlt should render presentation role because the img has alt="" - this fails', () => {
  render(<ImageWithEmptyAlt />)
  expect(screen.getByRole('presentation')).toBeInTheDocument();
});

What you did:

⬆️

What happened:

⬆️

Reproduction:

https://github.com/ahayes91/dom-testing-library-template

Problem description:

https://github.com/testing-library/dom-testing-library/issues/1235 suggests that an img with an empty alt attribute should be given a presentation role:

According to the ARIA docs an img should only default to the role of presentation if it has an alt attribute equal to "":

Additionally, given that an image with alt="" removes it from the accessibility tree (screenreader users can't access it), I would expect that testing library should not be able to find it unless I include hidden: true in the query. The fact that the test "finds" the image when a screenreader user would not reduces our confidence that the automation is behaving correctly.

Suggested solution:

Not sure, it seems like a bug here but would need the maintainers to confirm! Thank you!

Yohan27x commented 4 months ago

https://testing-library.com/docs/queries/byrole/#hidden

If you set hidden to true elements that are normally excluded from the accessibility tree are considered for the query as well. The default behavior follows https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion with the exception of role="none" and role="presentation" which are considered in the query in any case.

So it's a normal behavior that you cant still find it.

ahayes91 commented 3 months ago

https://testing-library.com/docs/queries/byrole/#hidden

If you set hidden to true elements that are normally excluded from the accessibility tree are considered for the query as well. The default behavior follows https://www.w3.org/TR/wai-aria-1.2/#tree_exclusion with the exception of role="none" and role="presentation" which are considered in the query in any case.

So it's a normal behavior that you cant still find it.

Thank you @Yohan27x ! It still seems like there is some strange behaviour here though - why does the image have a role of img and not presentation when alt=""?

Additionally I have not edited the default value of hidden here in the tests. Even when I do, the result is still the same, because Testing Library is seeing the image as in the accessibility tree when it shouldn't be:

 FAIL  src/__tests__/index.test.js
  ✕ ImageWithEmptyAlt should not render an img role because the img has alt="" - this fails (37 ms)
  ✕ ImageWithEmptyAlt should render presentation role because the img has alt="" - this fails (9 ms)

  ● ImageWithEmptyAlt should not render an img role because the img has alt="" - this fails

    expect(element).not.toBeInTheDocument()

    expected document not to contain element, found <img alt="" src="https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png" /> instead

       5 | test('ImageWithEmptyAlt should not render an img role because the img has alt="" - this fails', () => {
       6 |   render(<ImageWithEmptyAlt />)
    >  7 |   expect(screen.queryByRole('img', { hidden: true})).not.toBeInTheDocument();
         |                                                          ^
       8 | });
       9 |
      10 | test('ImageWithEmptyAlt should render presentation role because the img has alt="" - this fails', () => {

      at Object.toBeInTheDocument (src/__tests__/index.test.js:7:58)
      at TestScheduler.scheduleTests (node_modules/react-scripts/node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (node_modules/react-scripts/node_modules/@jest/core/build/runJest.js:404:19)
      at _run10000 (node_modules/react-scripts/node_modules/@jest/core/build/cli/index.js:320:7)
      at runCLI (node_modules/react-scripts/node_modules/@jest/core/build/cli/index.js:173:3)

  ● ImageWithEmptyAlt should render presentation role because the img has alt="" - this fails

    TestingLibraryElementError: Unable to find an element with the role "presentation"

    Here are the available roles:

      img:

      Name "":
      <img
        alt=""
        src="https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png"
      />

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

    Ignored nodes: comments, script, style
    <body>
      <div>
        <img
          alt=""
          src="https://www.google.com/images/branding/googlelogo/1x/googlelogo_color_272x92dp.png"
        />
      </div>
    </body>

      10 | test('ImageWithEmptyAlt should render presentation role because the img has alt="" - this fails', () => {
      11 |   render(<ImageWithEmptyAlt />)
    > 12 |   expect(screen.getByRole('presentation', { hidden: true})).toBeInTheDocument();
         |                 ^
      13 | });
      14 |
      15 |

      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/__tests__/index.test.js:12:17)
      at TestScheduler.scheduleTests (node_modules/react-scripts/node_modules/@jest/core/build/TestScheduler.js:333:13)
      at runJest (node_modules/react-scripts/node_modules/@jest/core/build/runJest.js:404:19)
      at _run10000 (node_modules/react-scripts/node_modules/@jest/core/build/cli/index.js:320:7)
      at runCLI (node_modules/react-scripts/node_modules/@jest/core/build/cli/index.js:173:3)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 2 total
Snapshots:   0 total
Time:        2.407 s
cmorten commented 3 months ago

Afaik this was resolved in https://github.com/testing-library/dom-testing-library/pull/1241 and is available in the alpha version 10 release 🙂

To quote @MatanBobi from similar issues:

Any chance you can test this against our alpha release? (can be installed using npm install @testing-library/dom@alpha). We've updated the underlying package (aria-query) there. We're still running some tests and I didn't get a chance to push it forwards to a release but having someone else testing that release will really help us. Thanks!

Note: you will need to use resolutions/overrides to get the react package to use the alpha version of dom testing library at the moment.

MatanBobi commented 3 months ago

Thanks @cmorten! This is indeed resolved in DTL version 10 which was just released a few hours ago: Repro: https://stackblitz.com/edit/rtl-template-lxaduy?file=package.json,src%2FApp.test.tsx