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

Accessible name not based on placeholder #926

Open nphmuller opened 3 years ago

nphmuller commented 3 years ago

Relevant code or config:

App.js

function App() {
  return (
    <div className="App">
      <input type="text" placeholder="placeholder-value" />
    </div>
  );
}
export default App;

App.test.js

test('finds textbox by role and placeholder', () => {
  render(<App />);
  expect(screen.getByRole('textbox', { name: 'placeholder-value' })).toBeVisible()
});

What you did:

Run test (via yarn test)

What happened:

The test fails with the message:

TestingLibraryElementError: Unable to find an accessible element with the role "textbox" and name "placeholder-value"

    Here are the accessible roles:

      textbox:

      Name ""

Note that the value for Name is empty

Reproduction:

Relevant commit: https://github.com/nphmuller/dom-testing-placeholder-repro/commit/1786a0a4c048f244a469ac8a0fa1bdaf92fc54a3

Problem description:

When looking at the Chrome dev tools, the accessible name is also calculated based on the placeholder. See the screenshot below. dom-testing-library seems to ignore the placeholder value for calculating this name.

image

Suggested solution:

It would really help if you could support the placeholder during accessible name calculation.

MatanBobi commented 3 years ago

Hi @nphmuller, thanks for taking the time to open this!

I found two contradicting approaches about this in the spec: https://w3c.github.io/html-aam/#input-type-text-input-type-password-input-type-search-input-type-tel-input-type-email-input-type-url-and-textarea-element-accessible-name-computation

But as far as I know, and @eps1lon correct me if I'm wrong, the code actually follows: https://www.w3.org/TR/accname-1.1/ And that page doesn't mention the placeholder attribute.

nphmuller commented 3 years ago

Thanks for the quick reply!

It seems to be something that's officially not supported (in the spec) but in practice still used a lot. I did a bit of Googling around and most articles mention it's not recommended, but in practice still works most of the time.

I've seen the placeholder being misused as a label in many projects.

eps1lon commented 3 years ago

I seem to remember putting some notes into https://github.com/eps1lon/dom-accessibility-api concerning placeholder. So I would suggest doing some digging there. Then we need to double check what accname would say and then what assistive technology (e.g. NVDA, VoiceOver, JAWS) actually do in these cases. Chrome devtools is not our primary source of truth here.

Regardless, don't use placeholders.

nphmuller commented 3 years ago

This blog post is a bit dated (2018), but it mentions that most assistive technologies read the placeholder (but not in all cases). https://www.davidmacd.com/blog/is-placeholder-accessible-label.html

I'd imagine that support has only grown since then.

MatanBobi commented 3 years ago

I seem to remember putting some notes into https://github.com/eps1lon/dom-accessibility-api concerning placeholder. So I would suggest doing some digging there. Then we need to double check what accname would say and then what assistive technology (e.g. NVDA, VoiceOver, JAWS) actually do in these cases. Chrome devtools is not our primary source of truth here.

Regardless, don't use placeholders.

@eps1lon Looks like the only thing you have that relates to placeholder is a commit removing the TODO since you didn't find any reference for it 😅 https://github.com/eps1lon/dom-accessibility-api/commit/482e4daed7506a58fc432ca753276f4081b18877

eps1lon commented 3 years ago

I'd imagine that support has only grown since then.

Or not. In any case, we need to verify that claim. It might also be that AT just do this to fix author errors. AT and the ARIA spec make a big effort to fix author errors. That doesn't mean that we as a dev tool should accept these errors which effectively encourages them.

@eps1lon Looks like the only thing you have that relates to placeholder is a commit removing the TODO since you didn't find any reference for it 😅

Thanks for digging this up. That comment basically confirms my current suspicion: There are no references to placeholder being used in the accessible name. But that doesn't mean there aren't actually any.

rpokorny commented 1 year ago

Is https://www.w3.org/TR/html-aam-1.0/ considered a valid source of information here? It does mention placeholder for accessible name computation, listing it beneath title in priority. In regards to the accname spec, as far as I understand that one is language-agnostic and leaves it to more specific specs (like html-aam) to fill in the details. I think placeholder would fall under step D from the accname spec as a "native markup ... attribute ... that defines a text alternative".