eslint-plugin-jsx-a11y icon indicating copy to clipboard operation
eslint-plugin-jsx-a11y copied to clipboard

Improve `ambiguous-anchor-text` false negatives (via extending `getAccessibleChildText`)

Open mattxwang opened this issue 3 years ago • 2 comments

I implemented ambiguous-anchor-text in #873, which uses a new util getAccessibleChildText to determine what counts as screenreader-accessible text inside an anchor tag. However, I think there are extra cases that would be valuable for the rule to catch.

Current Approach

The current approach is simple: recursively add children, with base cases for:

  • elements with an aria-label, returning the label - note that in this case, a label of "click here" is still an error
  • elements that are hidden from a screenreader (via isHiddenFromScreenReader), returning the empty string
  • the literal value for literals and text (Literal and JSXText types)

Proposed Extensions

However, this misses some nuance in how screenreader text is interpreted, leading to false negatives. I have some specific examples that I think should also be in-scope, all of which seem reasonable to implement:

  • [x] alt text for <img />; this should behave like aria-label: implemented in #880
  • [x] deemed unnecessary, see comment below ~~the SVG <title> element; this should also behave like aria-label~~

There's also a more challenging (but important) case of aria-labelledby. This is tricker since:

  • we're not guaranteed that the id referenced by the attribute is a child of the anchor tag
  • or that it exists at all!
  • aria-labelledby supports reference lists, and order matters, though needs to ignore repeated IDs; chaining is disallowed

It also seems that aria-labelledby overrides aria-label, so we should be sure to implement that.

I imagine that aria-describedby is out of scope, since it's a long-form description (and may not make sense for a screenreader to read).

What do we think? I don't have great familiarity in this field, so perhaps I am missing some nuance or other motivating examples. I may also split up the PRs so that it's easier to review (i.e. the low hanging fruit first).

mattxwang avatar Jul 22 '22 22:07 mattxwang

I've thought about this a bit more (and done some local testing) - I don't actually think the <svg> override is necessary, since <title> is parsed anyways as a child and there should not be any other text inside it.

I think that addresses my shallow follow-ups, and I'm curious once we release this rule how the community responds / how important the referential checks are. @ljharb do you know if you're planning on cutting a new release soon? Would be very helpful for me to then resolve this issue (of course, no rush!).

mattxwang avatar Aug 22 '22 20:08 mattxwang

@mattxwang no schedule, but i can try to do it in the next few weeks.

ljharb avatar Aug 23 '22 01:08 ljharb