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.

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

computeAccessibleName won't compute combobox name from self referencing aria-labelledby attr #1018

Open
agentdylan opened this issue Dec 7, 2023 · 4 comments

Comments

@agentdylan
Copy link

agentdylan commented Dec 7, 2023

When a div element role is combobox and the element has an aria-labelledby attribute that references itself, computeAccessibleName is failing to return the accessible name.

Role combobox supports name from author https://www.w3.org/TR/wai-aria/#namefromauthor

According to 2B of https://www.w3.org/TR/accname-1.2/#mapping_additional_nd_te

if computing a name, and the current node has an aria-labelledby attribute that contains at least one valid IDREF, and the current node is not already part of an aria-labelledby traversal, process its IDREFs in the order they occur

Please correct if I misunderstand but my interpretation is it should be returning the name. I note the name is seen in Chrome dev tools in the accessibility pane.

combobox-labelledby-self

When role is combobox the following test cases demonstrate it failing to return an accessible name when the element refers to itself using aria-labelledby.

Test cases

test.each([
[
		`<div data-test id="el1" role="combobox" aria-labelledby="el1">I reference myself for my name</div>`,
		"I reference myself for my name",
	],
	[
		`
		<div data-test id="el1" role="combobox" aria-labelledby="label">I reference another element for my name</div>
		<div id="label" role="presentation">I'm prohibited a name</div>
		`,
		"I'm prohibited a name",
	],
	[
		`<div data-test id="el1" role="combobox" aria-label="I use aria-label for my name"></div>`,
		"I use aria-label for my name",
	],
])(`misc #%#`, (markup, expectedAccessibleName) => {
	expect(markup).toRenderIntoDocumentAccessibleName(expectedAccessibleName);
});

Test output

misc #0

    expected <div
      aria-labelledby="el1"
      data-test=""
      id="el1"
      role="combobox"
    >
      I reference myself for my name
    </div> to have accessible name 'I reference myself for my name' but got ''
    - Expected
    + Received

    - I reference myself for my name

      496 |             ],
      497 |     ])(`misc #%#`, (markup, expectedAccessibleName) => {
    > 498 |             expect(markup).toRenderIntoDocumentAccessibleName(expectedAccessibleName);
          |                            ^
      499 |     });
      500 |
      501 | test("text nodes are not concatenated by space", () => {

      at toRenderIntoDocumentAccessibleName (__tests__/accessible-name.js:498:18)

At a quick glance this would appear to be because the logic is skipping to "step 2E" for role combobox, ultimately returning an empty string on line 649 of sources/accessible-name-and-description.ts
return isHTMLInputElement(current) ? current.value : "";

@MatanBobi
Copy link
Contributor

AFAIU from the spec, that's because role="combobox" should only be on an input element and not on a div element.
Having said that, the combobox role's spec is extremely convoluted.

@agentdylan
Copy link
Author

agentdylan commented Dec 7, 2023

Agreed it is quite difficult to make sense of. I've interpreted those descriptions in the more abstract sense of input as in also emulating controls rather than only literal html input elements. The word input there links to https://www.w3.org/TR/wai-aria-1.2/#input describing it as

A generic type of widget that allows user input.

The description of combobox in the spec also says

A combobox input MAY be either a single-line text field that supports editing and typing or an element that only displays the current value of the combobox

That last part "or an element that only displays the current value of the combobox" might be the key that it isn't just text inputs?

To me it appears there is intent for it to be used on div elements and we can see that in w3.org own examples e.g.
https://www.w3.org/WAI/ARIA/apg/patterns/combobox/examples/combobox-select-only/

image
image

In popular UI libraries it is also common to see combobox role on div elements.
e.g. Material UI
https://mui.com/material-ui/react-select/

@MatanBobi
Copy link
Contributor

I understand. I know that @eps1lon is also involved with mui so let's wait for his call on this one :)

@neaumusic
Copy link

neaumusic commented Dec 19, 2023

@agentdylan Just my two cents here, but I've noticed this fits a pattern of issues I've seen; I have two WIP pull requests up that simply check a string is not empty before returning. I'm unsure of the exact logic, but for our use case/fork we're rolling with those WIP modifications 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants