fix: correctly interpret empty aria- attribute

[Rich-Harris]

fixes #11323

Before submitting the PR, please make sure you do the following

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• Prefix your PR title with feat:, fix:, chore:, or docs:.
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests and linting

• Run the tests with pnpm test and lint the project with pnpm lint

[changeset-bot]

🦋 Changeset detected

Latest commit: 7a70a3f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
svelte	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dummdidumm]

I think this is too broad - for example for aria-labbeledby this warning would be wrong. Looking at the code further below it seems we have existing warnings for boolean aria attributes already, why don't we just use those?

[Rich-Harris]

What do you mean it would be wrong? aria-labelledby can't be empty, it has to be one or more IDs

[dummdidumm]

The message would be wrong because it would tell you to set it to true. For aria-describedby it would be equally confusing. And again I'm wondering why we need a new warning in the first place, it seems the code below has existing handling for incorrect values already, depending on what type they are.

[Rich-Harris]

We need a new warning because there's no situation in which an empty attribute is valid, and given that elsewhere an empty attribute means 'true' it's confusing to tell people that 'true' is a valid value but their empty attribute isn't. What if we have two warnings — keep as it is here for boolean and tristate attributes, and for everything else just say

'%attribute%' cannot be empty

?

[dummdidumm]

I don't think we need any new warnings? Below this check there's a bunch of statements emitting warnings for various cases. For example

if (type === 'boolean' && value !== 'true' && value !== 'false') {
	w.a11y_incorrect_aria_attribute_type_boolean(attribute, name);
}

will automatically handle the invalid boolean value case.
For type=token and type=tokenlist the if checks need to be slightly adjusted, but other than that this should then handle this just fine?

[Rich-Harris]

That's what I'm saying — you'd get a warning like

The value of '%attribute%' must be either 'true' or 'false'

to which a developer might reasonably say 'but <div aria-hidden> already means 'true', unaware of the difference between boolean attributes and boolean ARIA attributes.

We could adjust a11y_incorrect_aria_attribute_type_boolean to say this...

The value of '%attribute%' must be either 'true' or 'false'. It cannot be empty

...but then you're no longer narrowing the type of value to string, which complicates the code further below. It seems like the suggestion above (different warning on empty attributes for boolean/tristate) is just more pragmatic

[dummdidumm]

I just realized that we can also assign the empty string to the value instead (that's what it means after all) - then all the code further below doesn't need to be adjusted. And adjusting the warning sounds ok to me.