Skip to content
This repository has been archived by the owner on Dec 7, 2022. It is now read-only.

IDREFs that refer to an ambiguous ID should be an error #141

Closed
ghost opened this issue Mar 19, 2015 · 3 comments
Closed

IDREFs that refer to an ambiguous ID should be an error #141

ghost opened this issue Mar 19, 2015 · 3 comments
Labels

Comments

@ghost
Copy link

ghost commented Mar 19, 2015

Some accessibility audits give a warning if you have two elements on the page with the same id even if it's never used, and I think that's silly. It just wastes people's time in cases where it's not actually being used.

However, I think that code like this should be an error, since there's a label with an IDREF and there are multiple controls with that same id.

<label for="foo">First</label>
<input id="foo" value="First box">

<label for="foo">Second</label>
<input id="foo" value="Second box">
@stevefaulkner
Copy link

@dmazzoni-google your code examples are not displaying, but agree that flagging mulitple id's as an issue is generally not an accessibility issue, though it is a HTML conformance error.

The twist is that depending on the state of a document, an id value may or not be referenced in a relation so it is difficult to know when duplicate id's are problematic or not.

@ricksbrown
Copy link
Collaborator

Alice and I discussed this a while back (see #102) and ended up writing an audit to cover this.

In terms of whether to only audit "used" ids or determine which ones are relevant to a11y I think this comment from the audit code explains our reasoning on it:

/*
 * Checks for duplicate IDs within the context of this element.
 * This is not a pure a11y check however IDREF attributes in ARIA and HTML (label 'for', td 'headers)
 *    depend on IDs being correctly implemented.
 * While we could limit this audit to IDs which are actually referred to via any IDREF attribute
 *    that would be incomplete, because naturally many of these attributes change over time e.g.
 *    'aria-activedescendant' is likely to change over time and therefore safer to vet them all.
 */

It's basically exactly what @stevefaulkner said.

EDIT: the version of the extension currently available in the webstore is not up to date with the latest version of the accessibility developer tools library, that is why it is currently not reporting duplicate IDs.

It's up to @alice to determine when it should be updated but I'm thinking it's looking pretty good (maybe after we action PR #138 ;) )

FYI I fixed up the original comment from @dmazzoni-google so that the code example would display.

@alice
Copy link
Contributor

alice commented Jul 31, 2015

Since this test is so noisy I'm now convinced by @dmazzoni-google's reasoning.

IDREFs in the same scope really should be unique, otherwise there is no point having them, but it is indeed the case that automatically generated code often generates non-unique IDREFs and it is harmless unless something attempts to refer to that ID. Since there is a finite number of situations in which IDREFs may be used, perhaps we should follow @dmazzoni-google's original suggestion of finding all the cases where an IDREF is specified and make sure that each one refers to exactly one element (unless the element is aria-hidden or not rendered).

ricksbrown added a commit to ricksbrown/accessibility-developer-tools that referenced this issue Aug 2, 2015
ricksbrown added a commit that referenced this issue Sep 24, 2015
Closes issue #141 - only audit IDs in IDREFs
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants