feat: a11y warning for <button> / <a> without aria-label and content (#13130)

* Introduce button a11y warnings

* Log changeset

* Don't check for label if aria-hidden

* State based flag to avoid multiple warnings firing

* Update existing validators to include aria labels

* false positives are okay, false negatives aren't

* prefer text over aria-label in tests

* remove unnecessary aria-labels

* tweak message

* update test

* fix

* simplify

* various

* DRY out

* Apply suggestions from code review

---------

Co-authored-by: Rich Harris <[email protected]>
Co-authored-by: Rich Harris <[email protected]>

Author: JR-G

.changeset/odd-shirts-hear.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+feat: Add accessibility warnings for buttons and anchors without explicit labels and content

packages/svelte/messages/compile-warnings/a11y.md

@@ -22,6 +22,10 @@
 
 > Visible, non-interactive elements with a click event must be accompanied by a keyboard event handler. Consider whether an interactive element such as `<button type="button">` or `<a>` might be more appropriate. See https://svelte.dev/docs/accessibility-warnings#a11y-click-events-have-key-events for more details
 
+## a11y_consider_explicit_label
+
+> Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute
+
 ## a11y_distracting_elements
 
 > Avoid `<%name%>` elements
@@ -104,7 +108,7 @@
 
 ## a11y_missing_content
 
-> `<%name%>` element should have child content
+> `<%name%>` element should contain text
 
 ## a11y_mouse_events_have_key_events
 

packages/svelte/src/compiler/phases/2-analyze/visitors/shared/a11y.js

@@ -403,9 +403,9 @@ const a11y_required_attributes = {
 	object: ['title', 'aria-label', 'aria-labelledby']
 };
 const a11y_distracting_elements = ['blink', 'marquee'];
+
+// this excludes `<a>` and `<button>` because they are handled separately
 const a11y_required_content = [
-	// anchor-has-content
-	'a',
 	// heading-has-content
 	'h1',
 	'h2',
@@ -989,16 +989,17 @@ export function check_element(node, state) {
 	}
 
 	// element-specific checks
-	let contains_a11y_label = false;
+	const is_labelled = attribute_map.has('aria-label') || attribute_map.has('aria-labelledby');
 
-	if (node.name === 'a') {
-		const aria_label_attribute = attribute_map.get('aria-label');
-		if (aria_label_attribute) {
-			if (get_static_value(aria_label_attribute) !== '') {
-				contains_a11y_label = true;
-			}
+	if (node.name === 'a' || node.name === 'button') {
+		const is_hidden = get_static_value(attribute_map.get('aria-hidden')) === 'true';
+
+		if (!is_hidden && !is_labelled && !has_content(node)) {
+			w.a11y_consider_explicit_label(node);
 		}
+	}
 
+	if (node.name === 'a') {
 		const href = attribute_map.get('href') || attribute_map.get('xlink:href');
 		if (href) {
 			const href_value = get_static_text_value(href);
@@ -1139,11 +1140,34 @@ export function check_element(node, state) {
 
 	// Check content
 	if (
-		!contains_a11y_label &&
+		!is_labelled &&
 		!has_contenteditable_binding &&
 		a11y_required_content.includes(node.name) &&
-		node.fragment.nodes.length === 0
+		!has_content(node)
 	) {
 		w.a11y_missing_content(node, node.name);
 	}
 }
+
+/**
+ * @param {AST.RegularElement | AST.SvelteElement} element
+ */
+function has_content(element) {
+	for (const node of element.fragment.nodes) {
+		if (node.type === 'Text') {
+			if (node.data.trim() === '') {
+				continue;
+			}
+		}
+
+		if (node.type === 'RegularElement' || node.type === 'SvelteElement') {
+			if (!has_content(node)) {
+				continue;
+			}
+		}
+
+		// assume everything else has content — this will result in false positives
+		// (e.g. an empty `{#if ...}{/if}`) but that's probably fine
+		return true;
+	}
+}

packages/svelte/src/compiler/warnings.js

@@ -50,6 +50,7 @@ export const codes = [
 	"a11y_autocomplete_valid",
 	"a11y_autofocus",
 	"a11y_click_events_have_key_events",
+	"a11y_consider_explicit_label",
 	"a11y_distracting_elements",
 	"a11y_figcaption_index",
 	"a11y_figcaption_parent",
@@ -174,6 +175,14 @@ export function a11y_click_events_have_key_events(node) {
 	w(node, "a11y_click_events_have_key_events", "Visible, non-interactive elements with a click event must be accompanied by a keyboard event handler. Consider whether an interactive element such as `<button type=\"button\">` or `<a>` might be more appropriate. See https://svelte.dev/docs/accessibility-warnings#a11y-click-events-have-key-events for more details");
 }
 
+/**
+ * Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute
+ * @param {null | NodeLike} node
+ */
+export function a11y_consider_explicit_label(node) {
+	w(node, "a11y_consider_explicit_label", "Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute");
+}
+
 /**
  * Avoid `<%name%>` elements
  * @param {null | NodeLike} node
@@ -355,12 +364,12 @@ export function a11y_missing_attribute(node, name, article, sequence) {
 }
 
 /**
- * `<%name%>` element should have child content
+ * `<%name%>` element should contain text
  * @param {null | NodeLike} node
  * @param {string} name
  */
 export function a11y_missing_content(node, name) {
-	w(node, "a11y_missing_content", `\`<${name}>\` element should have child content`);
+	w(node, "a11y_missing_content", `\`<${name}>\` element should contain text`);
 }
 
 /**

packages/svelte/tests/runtime-browser/samples/head-script/main.svelte

@@ -7,4 +7,4 @@
 		});
 	</script>
 </svelte:head>
-<button></button>
+<button>click me</button>

packages/svelte/tests/validator/samples/a11y-anchor-has-content/warnings.json

@@ -1,7 +1,7 @@
 [
 	{
-		"code": "a11y_missing_content",
-		"message": "`<a>` element should have child content",
+		"code": "a11y_consider_explicit_label",
+		"message": "Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute",
 		"start": {
 			"line": 1,
 			"column": 0

packages/svelte/tests/validator/samples/a11y-aria-proptypes-boolean/input.svelte

@@ -2,7 +2,7 @@
     const abc = 'abc';
 </script>
 
-<button aria-disabled="yes"></button>
-<button aria-disabled="no"></button>
-<button aria-disabled={1234}></button>
-<button aria-disabled={`${abc}`}></button>
+<button aria-disabled="yes">click me</button>
+<button aria-disabled="no">click me</button>
+<button aria-disabled={1234}>click me</button>
+<button aria-disabled={`${abc}`}>click me</button>

packages/svelte/tests/validator/samples/a11y-click-events-have-key-events/input.svelte

@@ -31,7 +31,7 @@
 <div class="foo"></div>
 
 <a href="http://x.y.z" on:click={noop}>foo</a>
-<button on:click={noop}></button>
+<button on:click={noop}>click me</button>
 <select on:click={noop}></select>
 
 <input type="button" on:click={noop} />

packages/svelte/tests/validator/samples/a11y-consider-explicit-label/input.svelte

@@ -0,0 +1,11 @@
+<button></button>
+<a href="/#"><b></b></a>
+
+<button aria-label="Valid empty button"></button>
+<a href="/#" aria-label="Valid empty link"></a>
+
+<button aria-hidden='true'></button>
+<a href="/#" aria-hidden='true'><b></b></a>
+
+<button>Click me</button>
+<a href="/#">Link text</a>

packages/svelte/tests/validator/samples/a11y-consider-explicit-label/warnings.json

@@ -0,0 +1,26 @@
+[
+	{
+		"code": "a11y_consider_explicit_label",
+		"message": "Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute",
+		"start": {
+			"line": 1,
+			"column": 0
+		},
+		"end": {
+			"line": 1,
+			"column": 17
+		}
+	},
+	{
+		"code": "a11y_consider_explicit_label",
+		"message": "Buttons and links should either contain text or have an `aria-label` or `aria-labelledby` attribute",
+		"start": {
+			"line": 2,
+			"column": 0
+		},
+		"end": {
+			"line": 2,
+			"column": 24
+		}
+	}
+]