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

input type="search" isn't considered of role "searchbox" #511

Closed
rivajunior opened this issue Jun 13, 2023 · 7 comments · Fixed by #515
Closed

input type="search" isn't considered of role "searchbox" #511

rivajunior opened this issue Jun 13, 2023 · 7 comments · Fixed by #515

Comments

@rivajunior
Copy link

Previously considered the following code as of role "searchbox" but it is now considered as "textbox"

<input type="search" />

Now I have to add a list attribute to have the input with the implicit role of "searchbox".

<input type="search" list="foo" />

Shouldn't this be considered a breaking change? I have an entire project that relies on the previous behavior.

As from MDN Technical summary

Implicit ARIA role

  • type=search
    • with no list attribute: searchbox
    • with list attribute:combobox

This behavior is happening since v5.2.0

@aaronhargrove-grub
Copy link

Also seeing this issue in v5.2.1, which we have pulled in through RTL

@jlp-craigmorten
Copy link
Contributor

jlp-craigmorten commented Jun 15, 2023

I wonder if this has arisen due to:

combobox role, with the aria-controls property set to the same value as the list attribute

REF: https://www.w3.org/TR/html-aam-1.0/#html-element-role-mappings

Which has potentially been interpreted in https://github.com/A11yance/aria-query/pull/447/files#diff-474a847bde3a1c2a805f8616f2f2acd267500c5298c16c597743c26323994b5a to mean that the role of combobox is only applied when the aria-controls is set to the same as the list (or in the case of this lib, simply that it must be explicitly set)?

Comparing with the W3C Recommendation for HTML-ARIA it seems this is not a stable part of specs as yet.

R.E. the searchbox role, it seems the constraint for list being undefined was removed https://github.com/A11yance/aria-query/pull/447/files#diff-e0bad3efd508fd648f2892d7def88d5247eb5e3ff0bf02a8de027cc7057669c2, likely in error

(V. possible I am interpreting how this repo works and way off 😅)

zburke added a commit to folio-org/ui-erm-comparisons that referenced this issue Jun 15, 2023
Avoid `aria-query` `v5`, transiently pulled in via `@testing-library/react`, due to A11yance/aria-query#511
@jlp-craigmorten
Copy link
Contributor

jlp-craigmorten commented Jun 15, 2023

Test cases (using resolutions to force aria-query to 5.2.1:

import "react";
import "@testing-library/jest-dom";
import { render, screen } from "@testing-library/react";

describe("W3C Recommendations: https://www.w3.org/TR/html-aria/#docconformance", () => {
  test("input with type search and no list attribute is a searchbox", async () => {
    render(<input type="search" />);

    expect(await screen.queryByRole("searchbox")).not.toBeNull(); // Fail ❌
  });

  test("input with type search and a list attribute is a combobox", async () => {
    render(<input type="search" list="foo" />);

    expect(await screen.queryByRole("combobox")).not.toBeNull(); // Fail ❌
  });
});

describe("W3C Working Draft: https://www.w3.org/TR/html-aam-1.0/#html-element-role-mappings", () => {
  test("input with type search and matching list and aria-controls attributes is a combobox under the W3C Draft Spec", async () => {
    render(<input type="search" aria-controls="foo" list="foo" />);

    expect(await screen.queryByRole("combobox")).not.toBeNull(); // Passes ✅
  });
});

See testing-library/dom-testing-library#1235 (comment) for notes on how this regressed (and other cases).

@jessebeach
Copy link
Member

I wonder if this has arisen due to:

combobox role, with the aria-controls property set to the same value as the list attribute

REF: https://www.w3.org/TR/html-aam-1.0/#html-element-role-mappings

Which has potentially been interpreted in https://github.com/A11yance/aria-query/pull/447/files#diff-474a847bde3a1c2a805f8616f2f2acd267500c5298c16c597743c26323994b5a to mean that the role of combobox is only applied when the aria-controls is set to the same as the list (or in the case of this lib, simply that it must be explicitly set)?

Comparing with the W3C Recommendation for HTML-ARIA it seems this is not a stable part of specs as yet.

R.E. the searchbox role, it seems the constraint for list being undefined was removed https://github.com/A11yance/aria-query/pull/447/files#diff-e0bad3efd508fd648f2892d7def88d5247eb5e3ff0bf02a8de027cc7057669c2, likely in error

(V. possible I am interpreting how this repo works and way off 😅)

This part of the spec is super confusing. And it varies from versions 1.1 to 1.2 to the upcoming 1.3. I wasn't aware of this testing library using aria-query, but now that I have the test cases, I can investigate the assumptions in the tests and determine if my library has a bug here or if the downstream was built to accommodate a bug in the previous minor version of aria-query. Thank you for the report! I'll have a look this weekend.

@jlp-craigmorten
Copy link
Contributor

This part of the spec is super confusing. And it varies from versions 1.1 to 1.2 to the upcoming 1.3. I wasn't aware of this testing library using aria-query, but now that I have the test cases, I can investigate the assumptions in the tests and determine if my library has a bug here or if the downstream was built to accommodate a bug in the previous minor version of aria-query. Thank you for the report! I'll have a look this weekend.

As a starter for 10 I've raised #515 which I believe addresses the issues - depending on whether go with 1.2 vs 1.3 on this particular issue, it may need some tweaks 🙂

@jessebeach
Copy link
Member

I think we should go with 1.3 going forward, but given the breakages that my recent release caused, I'm going to shift the changes into a v6 branch of this project, and keep v5 on #515 fixes that you proposed.

@jessebeach
Copy link
Member

@jlp-craigmorten #515 is committed and released in v5.3.0.

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

Successfully merging a pull request may close this issue.

4 participants