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

Hidden form label: failure message not clearly identifying the problem #242

Closed
ithinkihaveacat opened this issue Oct 20, 2016 · 12 comments · Fixed by #792
Closed

Hidden form label: failure message not clearly identifying the problem #242

ithinkihaveacat opened this issue Oct 20, 2016 · 12 comments · Fixed by #792
Labels
feat New feature or enhancement rules Issue or false result from an axe-core rule

Comments

@ithinkihaveacat
Copy link

axe incorrectly reports that an element within the following HTML does not have a label (when embedded within a larger HTML file):

        <form class="search" action="/" data-gp-element="search">
          <label class="gp-offline-disable" for="s">Search</label>
          <input class="gp-offline-disable" type="search" name="s" id="s" placeholder="Search" value="">
          <button type="submit">Go</button>
        </form>

The input is correctly detected if the entire HTML file is as above; it's only a problem if embedded within a larger file.

To reproduce, first load https://cdn.rawgit.com/ithinkihaveacat/8577d6d00eb08f48b4a6e3fd015c379d/raw/f943193a01bff03ea7df37b9bf4ef206d9f2486c/index.html, then paste the following in the JS console to run the accessibility tests:

function loadScript(src) {
  return new Promise(resolve => {
    const e = document.createElement("script");
    e.src = src;
    e.onload = resolve; 
    document.head.appendChild(e);
  });
}

loadScript("https://unpkg.com/axe-core/axe.min.js").then(() => {;
  axe.a11yCheck(document, r => console.log(r.violations[2]));
});

Actual output:

A Form elements must have labels error on the element <input class="gp-offline-disable" type="search" name="s" id="s" placeholder="Search" value="">.

Expected output:

No error report (on this element; there are other accessibility errors in the HTML).

@paulirish
Copy link
Contributor

Aye. this was discovered downstream via Lighthouse.

Looking at the payload, it's not clear which of the label checks this is failing with:
image

I'm seeing empty relatedNodes arrays for all 5 of the label checks.

@trav-is
Copy link

trav-is commented Oct 20, 2016

aXe is correctly calling this out due to display: none; (some screen readers respect display:none; ,visibility:hidden, etc)

@ithinkihaveacat
Copy link
Author

Interesting, yes I can confirm setting display: block eliminates the warning, thanks!

function loadScript(src) {
  return new Promise(resolve => {
    const e = document.createElement("script");
    e.src = src;
    e.onload = resolve; 
    document.head.appendChild(e);
  });
}

document.querySelector("*[for='s']").style.display = "block";

loadScript("https://unpkg.com/axe-core/axe.min.js").then(() => {;
  axe.a11yCheck(document, console.log.bind(console));
});

@paulirish
Copy link
Contributor

I think that it'd be slightly more clear if the error message indicated the label is hidden.

@dylanb dylanb changed the title Form label not detected Hidden form label: failure message not clearly identifying the problem Oct 21, 2016
@dylanb
Copy link
Contributor

dylanb commented Oct 21, 2016

@paulirish that is a valid point, I think we could improve this.

Note to implementor: we would probably want to create a separate "fail" check that looks for a hidden explicit label and flags this while simultaneously changing the explicit label check to allow hidden labels to pass

@dylanb dylanb reopened this Oct 21, 2016
@dylanb dylanb added feat New feature or enhancement help wanted We welcome PRs or discussions for issues marked as help wanted rules Issue or false result from an axe-core rule labels Oct 21, 2016
@hcaprette
Copy link

@dylanb I agree that it would help if the error message indicated that the label is hidden. But, why should a hidden label pass an accessibility check? Form labels are implemented to help not only people who can't see, using screen readers, but for people with mobility impairments. From what I understand, the form label increases the area in which someone can click to move the cursor insertion into the input area. That is, if I click on the label text, it will move my cursor into the input.

@dylanb
Copy link
Contributor

dylanb commented Mar 23, 2017

@hcaprette you are right it should not. What I meant is that it would be a good additional piece of information to expose to let the developer know that the label exists but is failing because it is hidden using display:none

@marcysutton
Copy link
Contributor

I just tested this and the issue is no longer occurring, so I think we can close it without any changes. Let us know if it still persists.

@dylanb
Copy link
Contributor

dylanb commented Nov 1, 2017

How did you test this?

@marcysutton
Copy link
Contributor

marcysutton commented Nov 1, 2017

I took the HTML snippet in this issue and ran aXe against it. https://s.codepen.io/marcysutton/debug/aYJpyL

@dylanb
Copy link
Contributor

dylanb commented Nov 1, 2017

Add this code to the CSS section

label.gp-offline-disable {
  display:none;
}

@marcysutton
Copy link
Contributor

Ah, you're right. I botched that one! We talked about one part of that being fixed, but the messaging is still confusing.

mrtnvh pushed a commit to mrtnvh/axe-core that referenced this issue Nov 24, 2023
Co-authored-by: Stephen Mathieson <me@stephenmathieson.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or enhancement rules Issue or false result from an axe-core rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants