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

False Positive: Form element has multiple <label> elements #689

Closed
jnurthen opened this issue Jan 19, 2018 · 24 comments
Closed

False Positive: Form element has multiple <label> elements #689

jnurthen opened this issue Jan 19, 2018 · 24 comments
Assignees
Labels
docs Documentation changes support

Comments

@jnurthen
Copy link

jnurthen commented Jan 19, 2018

IMO this rule should be retired completely. Multiple labels for a form field are allowed by spec and work in most useragents. I believe it is only IE where they do not. This would be the simplest way to resolve.

However, even if it were to be kept the following should certainly not throw an error
https://codepen.io/anon/pen/MrLgKm

<input type="checkbox" id="A" aria-labelledby="B">
<label class="indicator" aria-hidden="true" for="A">X</label>
<label for="A" id="B" class="ps-label">Yes/No Value</label>

Here there are 2 labels for the checkbox (the first is actually a CSS image in the real thing - but here I have added X for simplicity). They are both labels in order to increase the hit area for the checkbox. The first is hidden from screen readers with aria-hidden as it is not meaningful.

Even the addition of aria-labelledby on the checkbox which should override the labels does not seem to remove the aXe error.

@WilcoFiers
Copy link
Contributor

@jnurthen Going by the WebAIM survey, IE is still the second most used browser with screen readers. https://webaim.org/projects/screenreadersurvey7/#browsers I don't think we can drop support for IE just yet. Axe is highly configurable, so you can turn off rules / checks that don't fit your accessibility support baseline.

@marcysutton
Copy link
Contributor

I think we should at least check the aria-hidden case to see if it has any impact in IE. I can look into that today.

@jnurthen
Copy link
Author

@WilcoFiers If someone is overriding the accessible name using aria-labelledby as in this example, why is the fact that there are multiple labels relevant? Neither of the elements are used in the accessible name calculation. They are ONLY here to act as a larger target area for mouse clicks.
We would make the UI worse by changing one or both of the label elements to DIVs yet this is the only was we can get a clean aXe run on this.

@dylanb
Copy link
Contributor

dylanb commented Jan 25, 2018

aria-labelledby should override the double labels

Also, last I tested, VO did not respect double labels

@donifer
Copy link

donifer commented Mar 18, 2019

Sorry for reviving this thread. In @jnurthen's example one of the labels has aria-hidden="true" so now I'm a bit confused.

Is that not a a valid case? Or should I also use aria-labelledby on top of aria-hidden?

@meghnashah29
Copy link

meghnashah29 commented Mar 18, 2019

I did not completely understand the conclusion on this thread. I faced the same situation today that WorldSpace attest (axe) is throwing the error "Form element has multiple elements" while using same label multiple times as shown below:

<input type="checkbox" id="A" aria-labelledby="B">
<label class="indicator" aria-hidden="true" for="A">X</label>
<label for="A" id="B" class="ps-label">Yes/No Value</label>

Is it not recommended to use multiple label with aria-hidden?
Is it false positive?

Came across this issue which was resolved:
#202

@dylanb dylanb reopened this Mar 22, 2019
@dylanb
Copy link
Contributor

dylanb commented Mar 22, 2019

I am re-opening this because the aria-labelledby is overriding the other labels. This is a case we need to support.

We need to decide how to handle this case:

        <input type="checkbox" id="C">
        <label class="indicator" aria-hidden="true" for="C">X</label>
        <label for="C" class="ps-label">Thingy</label>

I have tested on VO and it works correctly but we need more evidence that this is intended and fully implemented behavior

@AutoSponge
Copy link
Contributor

AutoSponge commented May 3, 2019

AT often only reads the first/last visible label when multiple labels are associated with an element. When multiple label elements are present, they will both be read by AT if aria-labelledby is used with both idRefs, so this must pass. If an idRef does not appear in aria-labelledby it must be hidden from AT with aria-hidden="true".

@dylanb
Copy link
Contributor

dylanb commented May 6, 2019

@AutoSponge two questions:

Have you done the testing for the other accessibility supported ATs?
What are you basing your statement about "Normally..." on?

@AutoSponge
Copy link
Contributor

AutoSponge commented May 6, 2019

I reworded my statement to "often".

Based on 2008 study:

Apple VoiceOver does not recognise more than one label element associated with a form control and reads only the label that comes first in the document's source order.
JAWS and Window-Eyes both do the opposite and read only the last label when an input field gains focus.
The only screen reader of those that I tested that does handle multiple labels is Fire Vox.

2012 Study (similar findings)

@straker
Copy link
Contributor

straker commented May 7, 2019

After talking with @AutoSponge and @WilcoFiers, and running extensive tests (https://codepen.io/straker/full/PvqONy), this is what we've come up with:

  • Multiple visible labels on an input is not consistently supported across all screen reader/AT combinations. VoiceOver just reads the first label, JAWS just reads the last label, and NVDA and Talkback read all labels.
  • Hiding all but the first label is consistently supported.
  • Hiding the first label is not quite consistently supported. Talkback reads all labels (even the hidden ones).
  • Adding aria-labelledby that points to hidden labels is consistently supported.
  • Adding aria-labelledby that points to multiple visible labels is not quite supported. VoiceOver on iOS fails to read all labels.
  • Hiding the first element if it does not have any text content is not supported. VoiceOver on iOS, NVDA, and Talkback announce as just "checkbox" (no label text announced)

Taking that all into account, I propose we update the rule to:

  • fail if there are multiple visible labels (even if aria-labelledby is present due to iOS Safari)
  • fail if there are multiple labels and any are hidden by aria-hidden (if aria-labelledby is not present due to Talkback)
  • pass if there is only one visible label and the rest are hidden via CSS
  • pass if there is only one visible label and aria-labelledby is present (doesn't matter if hidden via aria-hidden or CSS)

@WilcoFiers WilcoFiers removed this from the HTMLTools Sprint 2 milestone May 8, 2019
@cyrilledjdj
Copy link

So now using multiple labels with 'for' attributes will pass so long as only one is visible and all label IDs are referenced in the aria-labelledby of the input.

@dylanb
Copy link
Contributor

dylanb commented May 16, 2019

@cyrilledjdj - in a nutshell - yes

@AutoSponge
Copy link
Contributor

@jeankaplansky jeankaplansky self-assigned this May 17, 2019
@jeankaplansky
Copy link
Contributor

@aellsey Please move this to the next sprint.

@aellsey aellsey added this to the Sprint 4 milestone May 21, 2019
@jeankaplansky
Copy link
Contributor

@dylanb : I take it you want the rule-help to clearly state @straker's plan?:

"fail if there are multiple visible labels (even if aria-labelledby is present due to iOS Safari)
fail if there are multiple labels and any are hidden by aria-hidden (if aria-labelledby is not present due to Talkback)
pass if there is only one visible label and the rest are hidden via CSS
pass if there is only one visible label and aria-labelledby is present (doesn't matter if hidden via aria-hidden or CSS)"

Please advise.
Thanks.

@jeankaplansky
Copy link
Contributor

jeankaplansky commented Jun 4, 2019

@AutoSponge : Please see my question to @dylanb above - I suspect he is too busy to answer. Please confirm that you implemented:

  • fail if there are multiple visible labels (even if aria-labelledby is present due to iOS Safari)
  • fail if there are multiple labels and any are hidden by aria-hidden (if aria-labelledby is not present due to Talkback)
  • pass if there is only one visible label and the rest are hidden via CSS
  • pass if there is only one visible label and aria-labelledby is present (doesn't matter if hidden via aria-hidden or CSS)

I'm reluctant to include this info in the rule help as it's default browser functionality + screen reader default functionality and the rule help is not designed to document browser or screen reader default functionality. PLEASE ADVISE:

  • Multiple visible labels on an input is not consistently supported across all screen reader/AT combinations. VoiceOver just reads the first label, JAWS just reads the last label, and NVDA and Talkback read all labels.
  • Hiding all but the first label is consistently supported.
  • Hiding the first label is not quite consistently supported. Talkback reads all labels (even the hidden ones).
  • Adding aria-labelledby that points to hidden labels is consistently supported.
  • Adding aria-labelledby that points to multiple visible labels is not quite supported. VoiceOver on iOS fails to read all labels.
  • Hiding the first element if it does not have any text content is not supported. VoiceOver on iOS, NVDA, and Talkback announce as just "checkbox" (no label text announced)

cc: @JKODU, @WilcoFiers

@straker
Copy link
Contributor

straker commented Jun 4, 2019

We can probably boil down the help text to say that there should only be one visible label per form element, with an expanded detail on the site help that talks about aria-hidden is not sufficient and you need to hide them using CSS.

@jeankaplansky
Copy link
Contributor

@straker : Thank you! that helps!

Now... The rule help isn't supposed to document browser or screen reader default functionality. BUT... We know that default browser and screen reader default functionality does interfere with the markup intent... But we do have to say something about this in the rule help... How much is too much in this instance? My concern is that the minute I write that a browser/screen reader combo does A by default, a new release will come along and the default will change and we won't necessarily know it until someone stumbles on it some day.

Note to self: https://codepen.io/straker/pen/PvqONy for example code that passes and doesn't pass.

@jeankaplansky jeankaplansky added the docs Documentation changes label Jun 4, 2019
@dylanb
Copy link
Contributor

dylanb commented Jun 15, 2019

@jeankaplansky @straker I would like the examples that DO work to be listed in the help so that if there is a legitimate reason to "need" multiple labels, the correct way to do this is documented.

@jeankaplansky
Copy link
Contributor

@dylanb: partially documented in INR as "fix" - 6/17/2019; plan to include full examples from https://codepen.io/straker/pen/PvqONy and https://github.com/dequelabs/axe-core/blob/develop/test/integration/rules/form-field-multiple-labels/form-field-multiple-labels.json in delivered axe-core 3.3 rule help

@jeankaplansky
Copy link
Contributor

@straker @dylanb : Rule help per notes above is here: https://dequeuniversity.com/rules/axe/3.3/form-field-multiple-labels. Please confirm this is the level of detail you were looking for. Thanks.

@somaalapati
Copy link

@chandana7393 Let me know if you need help in verifying this ticket. This ticket is in "Ready for QA" column for more than 20 day which is not good.

@chandana7393
Copy link

Tested, working as expected.
one_label_form
Tested Environment:
Attest - 2.5.1.21115v
Axe - 3.8.1.21115v
Axe-coconut - 3.8.1.21304v
Chrome - 76.0.3809.87v
Firefox - 68.0.1v
OS - Windows 10 64 bit.

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

No branches or pull requests