svelte: Accessibility warnings that rely on event directives are wrong

Agree that this is a less common use case

What’s the common use case? Trying to game the semantics and turn a div into a link? contenteditable?

I can give you many cases where one might legitimately listen for child mouse and keyboard events on divs. Modals, dropdowns, etc. These cases are perhaps even more common in Svelte than other frameworks, since Svelte, nicely, is not officious about HTML (e.g., no <Link/> component, not everything is assumed to be a component, etc.)

false positive

These errors are not false positives. The methodology is wrong, making the results universally suspect. Svelte tries to infer the a11y of an element from the presence of event listeners. The goodness or badness of this…

<!-- obviously bad -->
<div on:click={() => goto('/foo')}>Foo</div>

…and this…

<!-- probably legit -->
<div on:click={childListener}><slot/></div>

…cannot be inferred solely from the presence of on:click. The fact that the errors on the first guy are approximately correct does not make the methodology sound.

we need to keep the current rule

A11y in general is hard. The ARIA API is, well, ahem (~70 aria- attributes plus lord knows how many roles.) The official documentation is not great. There’s not much overlap among people proficient in modern javascript and people proficient in a11y. (I’m not claiming to be anywhere on that Venn diagram.)

Given that environment, developers would be better served by Svelte doing only what it can do. In this case it’s pretending to do something it can’t. The pretense is demonstrably harmful. Folks are turning off the a11y linter entirely because certain errors, like the ones above, are unreliable. Other folks, like me in this case, get sent down rabbit holes – “Oh @$#^, do user agents actually infer the a11y role from the listener?” – and end up having to confirm, at great length, that the linter is talking nonsense. (Then writing lengthy GitHub issues.)

If the linter is key feature, rather than just being able to say “yeah, sure, Svelte has one,” then it should work as well as everything else in the framework does.

In the meantime, I’ll use comments to suppress the errors that bother me. Thanks as always for your work.

The problem would surface with a bare minimum of keyboard testing

You are just way too optimistic about this. This is assuming that there even is any keyboard testing to begin with.

It’s indeed a false positive for catching bubbled event. But the compiler won’t know that there is a button inside the slot. That’s why there is the <!--svelte-ignore--> comment that can be used to disable when you’re sure it is a false positive. https://svelte.dev/docs/accessibility-warnings

It’s way more common for people to use on:event on the exact element the event triggers. We’re not going to give up most of the cases we can catch because of the false positives.

As for hints about using a button instead, there is already an issue in the svelte core https://github.com/sveltejs/svelte/issues/8001.

This is overall a svelte core issue, so I’ll close this later in favour of the upstream issue. What we can do is add a code action quick fix to turn div into a button. And maybe even add styles to remove the border and background. But I’ll prefer to add this after svelte core add the hint to the warning message.

You are just way too optimistic about this. This is assuming that there even is any keyboard testing to begin with.

That’s as may be (I haven’t been accused of optimism for a couple of decades, so I’ll take it as a compliment.)

Seriously – there’s only so much babysitting the linter can do. It can only be a reasonable starting point. If developers or their employers choose to ship software without adequate testing, then that’s not Svelte’s fault.

On the other hand, if developers turn off or ignore the linter because parts of it are unreliable, then that is Svelte’s concern.

It’s way more common for people to use on:event on the exact element the event triggers. We’re not going to give up most of the cases we can catch because of the false positives.

I disagree. The salient case is listening for child events, where it would be inconvenient or impossible to attach the relevant listeners to each child.

Leave this open, please.

but accessibility is impacted, because there’s an interaction that can be accessed by mouse but not keyboard

@oscarhermoso, fair enough. Your point might be better made / more easily understood with a real-world example. But I’ll concede the point, and go with the assumption that the div in the example belongs in the accessibility tree.

So, let’s compare the proposals, tabindex (comment above) vs. |nonself:

What gets fixed

• tabindex: Primarily, does away with the wrong warnings that appear in the common case of listening for child events, where HTML/a11y semantics are not gamed. Secondarily, accounts for gaming HTML semantics (if that is indeed necessary in real life. )
• |nonself: Primarily leaves things as they are, on the assumption that gaming semantics is the salient case. Secondarily, provides a hoop for developers to jump through to fix the event propagation problem.

Alignment with reality

• tabindex: Aligned. User agents and assistive tech use tabindex to determine inclusion in the tree.
• |nonself: Not aligned. Assumes users agents use event attachment to determine inclusion.

Svelte-ishness

• tabindex: Not Svelte-ish. Just add tabindex="0", which is table stakes for the gaming case with or without Svelte.
• |nonself: Svelte-ish. Developers would have to know/remember one more special thing about Svelte. (For example, I didn’t remember the existence of the |self modifier until it was brought up here.)

Explanation in docs

(please assume some of the terms below are linked to WAI or MDN documentation)

• tabindex: Heads up! The a11y linter does not assume a semantically non-interactive element (like a div) with a mouse or keyboard listener is meant to be in the accessibility tree. If you intend the element to be part of the tree, then add tabindex="0". User agents will include the element in the tree, and the linter suggest appropriate attributes to make the markup accessible.
• |nonself: Heads up! The a11y linter assumes a semantically non-interactive element (like a div) with a mouse or keyboard listener is meant to be in the accessibility tree. This differs from the behavior of user agents and assistive technology, and ignores event propagation. If your non-interactive element isn’t doing anything interactive, you must add the |nonself modifier to each on:eventName directive in order to suppress the incorrect linter warnings.

IMO, it’s pretty clear that tabindex is the way to go.

I’d just use patch-package. It’s more productive. 😁

I highly doubt the “probably legit” part.

People adding click handlers on divs, unaware of any problems, is very common. And I would agree with the maintainers that valid cases are relatively rare. The most common thing should always be regular buttons with immediate handlers.

I just wish that #8001 would be addressed, just about anything would be better than the current state of affairs. If the warning explains everything properly, there will be no rabbit holes and lengthy research necessary to determine what to do with the warning. Either the user will realise they made a mistake and correct it or can just confidently ignore it.