fix: correctly interpret empty aria- attribute (#11325)

Rich-Harris · web-flow · commit 8e43e9aae0ad · 2024-04-25T13:34:51.000-04:00
* fix: correctly interpret empty aria- attribute

* changeset

* huh

* only suggest default value for boolean/tristate attributes

* adjust messages, disallow empty strings

* gah
diff --git a/.changeset/brown-geckos-fry.md b/.changeset/brown-geckos-fry.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: correctly interpret empty aria- attribute
diff --git a/packages/svelte/messages/compile-warnings/a11y.md b/packages/svelte/messages/compile-warnings/a11y.md
@@ -44,11 +44,11 @@
 
 ## a11y_incorrect_aria_attribute_type
 
-> The value of '%attribute%' must be of type %type%
+> The value of '%attribute%' must be a %type%
 
 ## a11y_incorrect_aria_attribute_type_boolean
 
-> The value of '%attribute%' must be either 'true' or 'false'
+> The value of '%attribute%' must be either 'true' or 'false'. It cannot be empty
 
 ## a11y_incorrect_aria_attribute_type_id
 
diff --git a/packages/svelte/src/compiler/phases/2-analyze/a11y.js b/packages/svelte/src/compiler/phases/2-analyze/a11y.js
@@ -103,7 +103,7 @@ function has_disabled_attribute(attribute_map) {
 	const aria_disabled_attr = attribute_map.get('aria-disabled');
 	if (aria_disabled_attr) {
 		const aria_disabled_attr_value = get_static_value(aria_disabled_attr);
-		if (aria_disabled_attr_value === true) {
+		if (aria_disabled_attr_value === 'true') {
 			return true;
 		}
 	}
@@ -601,20 +601,19 @@ function is_parent(parent, elements) {
  */
 function validate_aria_attribute_value(attribute, name, schema, value) {
 	const type = schema.type;
-	const is_string = typeof value === 'string';
 
 	if (value === null) return;
-	if (value === true) value = 'true'; // TODO this is actually incorrect, and we should fix it
+	if (value === true) value = '';
 
 	if (type === 'boolean' && value !== 'true' && value !== 'false') {
 		w.a11y_incorrect_aria_attribute_type_boolean(attribute, name);
-	} else if (type === 'integer' && !Number.isInteger(+value)) {
+	} else if (type === 'integer' && (value === '' || !Number.isInteger(+value))) {
 		w.a11y_incorrect_aria_attribute_type_integer(attribute, name);
-	} else if (type === 'number' && isNaN(+value)) {
+	} else if (type === 'number' && (value === '' || isNaN(+value))) {
 		w.a11y_incorrect_aria_attribute_type(attribute, name, 'number');
-	} else if ((type === 'string' || type === 'id') && !is_string) {
-		w.a11y_incorrect_aria_attribute_type(attribute, name, 'string');
-	} else if (type === 'idlist' && !is_string) {
+	} else if ((type === 'string' || type === 'id') && value === '') {
+		w.a11y_incorrect_aria_attribute_type(attribute, name, 'non-empty string');
+	} else if (type === 'idlist' && value === '') {
 		w.a11y_incorrect_aria_attribute_type_idlist(attribute, name);
 	} else if (type === 'token') {
 		const values = (schema.values ?? []).map((value) => value.toString());
diff --git a/packages/svelte/src/compiler/warnings.js b/packages/svelte/src/compiler/warnings.js
@@ -134,22 +134,22 @@ export function a11y_img_redundant_alt(node) {
 }
 
 /**
- * The value of '%attribute%' must be of type %type%
+ * The value of '%attribute%' must be a %type%
  * @param {null | NodeLike} node
  * @param {string} attribute
  * @param {string} type
  */
 export function a11y_incorrect_aria_attribute_type(node, attribute, type) {
-	w(node, "a11y_incorrect_aria_attribute_type", `The value of '${attribute}' must be of type ${type}`);
+	w(node, "a11y_incorrect_aria_attribute_type", `The value of '${attribute}' must be a ${type}`);
 }
 
 /**
- * The value of '%attribute%' must be either 'true' or 'false'
+ * The value of '%attribute%' must be either 'true' or 'false'. It cannot be empty
  * @param {null | NodeLike} node
  * @param {string} attribute
  */
 export function a11y_incorrect_aria_attribute_type_boolean(node, attribute) {
-	w(node, "a11y_incorrect_aria_attribute_type_boolean", `The value of '${attribute}' must be either 'true' or 'false'`);
+	w(node, "a11y_incorrect_aria_attribute_type_boolean", `The value of '${attribute}' must be either 'true' or 'false'. It cannot be empty`);
 }
 
 /**
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-boolean/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-boolean/warnings.json
@@ -1,7 +1,7 @@
 [
 	{
 		"code": "a11y_incorrect_aria_attribute_type_boolean",
-		"message": "The value of 'aria-disabled' must be either 'true' or 'false'",
+		"message": "The value of 'aria-disabled' must be either 'true' or 'false'. It cannot be empty",
 		"start": {
 			"line": 5,
 			"column": 8
@@ -13,7 +13,7 @@
 	},
 	{
 		"code": "a11y_incorrect_aria_attribute_type_boolean",
-		"message": "The value of 'aria-disabled' must be either 'true' or 'false'",
+		"message": "The value of 'aria-disabled' must be either 'true' or 'false'. It cannot be empty",
 		"start": {
 			"line": 6,
 			"column": 8
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-integer/input.svelte b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-integer/input.svelte
@@ -1,7 +1,6 @@
 <div aria-level="yes"></div>
 <div aria-level="no"></div>
 <div aria-level={`abc`}></div>
-<div aria-level={true}></div>
 <div aria-level></div>
 <div aria-level={"false"}></div>
 <div aria-level={!"false"}></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-integer/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-integer/warnings.json
@@ -27,11 +27,11 @@
 		"code": "a11y_incorrect_aria_attribute_type_integer",
 		"message": "The value of 'aria-level' must be an integer",
 		"start": {
-			"line": 5,
+			"line": 4,
 			"column": 5
 		},
 		"end": {
-			"line": 5,
+			"line": 4,
 			"column": 15
 		}
 	}
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-number/input.svelte b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-number/input.svelte
@@ -2,6 +2,6 @@
 <div aria-valuemax="no"></div>
 <div aria-valuemax={`abc`}></div>
 <div aria-valuemax={true}></div>
-<div aria-valuemax></div>
+<div aria-valuemax="true"></div>
 <div aria-valuemax={'false'}></div>
 <div aria-valuemax={!'false'}></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-number/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-number/warnings.json
@@ -1,7 +1,7 @@
 [
 	{
 		"code": "a11y_incorrect_aria_attribute_type",
-		"message": "The value of 'aria-valuemax' must be of type number",
+		"message": "The value of 'aria-valuemax' must be a number",
 		"start": {
 			"line": 1,
 			"column": 5
@@ -13,7 +13,7 @@
 	},
 	{
 		"code": "a11y_incorrect_aria_attribute_type",
-		"message": "The value of 'aria-valuemax' must be of type number",
+		"message": "The value of 'aria-valuemax' must be a number",
 		"start": {
 			"line": 2,
 			"column": 5
@@ -25,14 +25,14 @@
 	},
 	{
 		"code": "a11y_incorrect_aria_attribute_type",
-		"message": "The value of 'aria-valuemax' must be of type number",
+		"message": "The value of 'aria-valuemax' must be a number",
 		"start": {
 			"line": 5,
 			"column": 5
 		},
 		"end": {
 			"line": 5,
-			"column": 18
+			"column": 25
 		}
 	}
 ]
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-string/input.svelte b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-string/input.svelte
@@ -1,4 +1,4 @@
-<div aria-label></div>
+<div aria-label="true"></div>
 <div aria-label={true}></div>
 <div aria-label={false}></div>
 <div aria-label={1234}></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-string/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-string/warnings.json
@@ -1,14 +1 @@
-[
-	{
-		"code": "a11y_incorrect_aria_attribute_type",
-		"message": "The value of 'aria-label' must be of type string",
-		"start": {
-			"line": 1,
-			"column": 5
-		},
-		"end": {
-			"line": 1,
-			"column": 15
-		}
-	}
-]
+[]
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-token/input.svelte b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-token/input.svelte
@@ -1,6 +1,6 @@
 <div aria-sort=""></div>
 <div aria-sort="incorrect"></div>
-<div aria-sort></div>
+<div aria-sort="true"></div>
 <div aria-sort={true}></div>
 <div aria-sort={"false"}></div>
 <div aria-sort="ascending descending"></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-token/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-token/warnings.json
@@ -32,7 +32,7 @@
 		},
 		"end": {
 			"line": 3,
-			"column": 14
+			"column": 21
 		}
 	},
 	{
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-tokenlist/input.svelte b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-tokenlist/input.svelte
@@ -1,6 +1,6 @@
 <div aria-relevant=""></div>
 <div aria-relevant="foobar"></div>
-<div aria-relevant></div>
+<div aria-relevant="true"></div>
 <div aria-relevant={true}></div>
 <div aria-relevant={"false"}></div>
 <div aria-relevant="additions removals_"></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-aria-proptypes-tokenlist/warnings.json b/packages/svelte/tests/validator/samples/a11y-aria-proptypes-tokenlist/warnings.json
@@ -32,7 +32,7 @@
 		},
 		"end": {
 			"line": 3,
-			"column": 18
+			"column": 25
 		}
 	},
 	{
diff --git a/packages/svelte/tests/validator/samples/a11y-click-events-have-key-events/input.svelte b/packages/svelte/tests/validator/samples/a11y-click-events-have-key-events/input.svelte
@@ -57,7 +57,7 @@
 <input on:click={noop} type="hidden" />
 
 <!-- svelte-ignore a11y_no_static_element_interactions -->
-<div on:click={noop} aria-hidden></div>
+<div on:click={noop} aria-hidden="true"></div>
 <!-- svelte-ignore a11y_no_static_element_interactions -->
 <div on:click={noop} aria-hidden="true"></div>
 <!-- svelte-ignore a11y_no_static_element_interactions -->
diff --git a/packages/svelte/tests/validator/samples/a11y-heading-has-content/input.svelte b/packages/svelte/tests/validator/samples/a11y-heading-has-content/input.svelte
@@ -1,2 +1,2 @@
 <h1></h1>
-<h2 aria-hidden>invisible header</h2>
+<h2 aria-hidden="true">invisible header</h2>
diff --git a/packages/svelte/tests/validator/samples/a11y-heading-has-content/warnings.json b/packages/svelte/tests/validator/samples/a11y-heading-has-content/warnings.json
@@ -20,7 +20,7 @@
 		},
 		"end": {
 			"line": 2,
-			"column": 15
+			"column": 22
 		}
 	}
 ]
diff --git a/packages/svelte/tests/validator/samples/a11y-img-redundant-alt/input.svelte b/packages/svelte/tests/validator/samples/a11y-img-redundant-alt/input.svelte
@@ -1,5 +1,5 @@
 <img src="foo" alt="Foo eating a sandwich." />
-<img src="bar" aria-hidden alt="Picture of me taking a photo of an image" />
+<img src="bar" aria-hidden="true" alt="Picture of me taking a photo of an image" />
 <img src="foo" alt="Photo of foo being weird." />
 <img src="bar" alt="Image of me at a bar!" />
 <img src="foo" alt="Picture of baz fixing a bug." />
diff --git a/packages/svelte/tests/validator/samples/a11y-interactive-supports-focus/input.svelte b/packages/svelte/tests/validator/samples/a11y-interactive-supports-focus/input.svelte
@@ -1,6 +1,6 @@
 <!-- VALID -->
-<div aria-hidden role="button" on:keypress={() => {}}></div>
-<div aria-disabled role="button" on:keypress={() => {}}></div>
+<div aria-hidden="true" role="button" on:keypress={() => {}}></div>
+<div aria-disabled="true" role="button" on:keypress={() => {}}></div>
 <div disabled role="button" on:keypress={() => {}}></div>
 <div role="presentation" on:keypress={() => {}}></div>
 <button on:click={() => {}}></button>
diff --git a/packages/svelte/tests/validator/samples/a11y-media-has-caption/input.svelte b/packages/svelte/tests/validator/samples/a11y-media-has-caption/input.svelte
@@ -2,6 +2,5 @@
 <video></video>
 <video><track /></video>
 <audio></audio>
-<video aria-hidden></video>
-<video aria-hidden="false"></video>
 <video aria-hidden="true"></video>
+<video aria-hidden="false"></video>
diff --git a/packages/svelte/tests/validator/samples/a11y-media-has-caption/warnings.json b/packages/svelte/tests/validator/samples/a11y-media-has-caption/warnings.json
@@ -23,18 +23,6 @@
 			"line": 3
 		}
 	},
-	{
-		"code": "a11y_media_has_caption",
-		"end": {
-			"column": 27,
-			"line": 5
-		},
-		"message": "`<video>` elements must have a `<track kind=\"captions\">`",
-		"start": {
-			"column": 0,
-			"line": 5
-		}
-	},
 	{
 		"code": "a11y_media_has_caption",
 		"end": {
diff --git a/packages/svelte/tests/validator/samples/a11y-no-interactive-element-to-noninteractive-role/input.svelte b/packages/svelte/tests/validator/samples/a11y-no-interactive-element-to-noninteractive-role/input.svelte
@@ -83,19 +83,19 @@
 <div role="menu"></div>
 <div role="menubar"></div>
 <div role="menuitem"></div>
-<div role="menuitemcheckbox" aria-checked></div>
-<div role="menuitemradio" aria-checked></div>
-<div role="option" aria-selected></div>
+<div role="menuitemcheckbox" aria-checked="true"></div>
+<div role="menuitemradio" aria-checked="true"></div>
+<div role="option" aria-selected="true"></div>
 <div role="progressbar"></div>
-<div role="radio" aria-checked></div>
+<div role="radio" aria-checked="true"></div>
 <div role="radiogroup"></div>
 <div role="row"></div>
 <div role="rowheader"></div>
 <div role="scrollbar" aria-controls={[]} aria-valuenow={0}></div>
 <div role="searchbox"></div>
 <div role="slider" aria-valuenow={0}></div>
 <div role="spinbutton"></div>
-<div role="switch" aria-checked></div>
+<div role="switch" aria-checked="true"></div>
 <div role="tab"></div>
 <div role="textbox"></div>
 <div role="treeitem" aria-selected={true}></div>
diff --git a/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-interactions/input.svelte b/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-interactions/input.svelte
@@ -1,7 +1,7 @@
 <!-- VALID -->
 <div role="presentation" on:mouseup={() => {}}></div>
 <div role="button" tabindex="-1" on:click={() => {}} on:keypress={() => {}}></div>
-<div role="listitem" aria-hidden on:click={() => {}} on:keypress={() => {}}></div>
+<div role="listitem" aria-hidden="true" on:click={() => {}} on:keypress={() => {}}></div>
 <button on:click={() => {}}></button>
 <h1 contenteditable="true" on:keydown={() => {}}>Heading</h1>
 <h1>Heading</h1>
diff --git a/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-to-interactive-role/input.svelte b/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-to-interactive-role/input.svelte
diff --git a/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-to-interactive-role/warnings.json b/packages/svelte/tests/validator/samples/a11y-no-noninteractive-element-to-interactive-role/warnings.json
diff --git a/packages/svelte/tests/validator/samples/a11y-role-supports-aria-props/input.svelte b/packages/svelte/tests/validator/samples/a11y-role-supports-aria-props/input.svelte
diff --git a/packages/svelte/tests/validator/samples/a11y-role-supports-aria-props/warnings.json b/packages/svelte/tests/validator/samples/a11y-role-supports-aria-props/warnings.json

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'svelte': patch
 +---
++
 +fix: correctly interpret empty aria- attribute
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`[`
`2`	`2`	`{`
`3`	`3`	`"code": "a11y_incorrect_aria_attribute_type_boolean",`
`4`		`- "message": "The value of 'aria-disabled' must be either 'true' or 'false'",`
	`4`	`+ "message": "The value of 'aria-disabled' must be either 'true' or 'false'. It cannot be empty",`
`5`	`5`	`"start": {`
`6`	`6`	`"line": 5,`
`7`	`7`	`"column": 8`
`@@ -13,7 +13,7 @@`
`13`	`13`	`},`
`14`	`14`	`{`
`15`	`15`	`"code": "a11y_incorrect_aria_attribute_type_boolean",`
`16`		`- "message": "The value of 'aria-disabled' must be either 'true' or 'false'",`
	`16`	`+ "message": "The value of 'aria-disabled' must be either 'true' or 'false'. It cannot be empty",`
`17`	`17`	`"start": {`
`18`	`18`	`"line": 6,`
`19`	`19`	`"column": 8`
Original file line number	Diff line number	Diff line change
`@@ -27,11 +27,11 @@`
`27`	`27`	`"code": "a11y_incorrect_aria_attribute_type_integer",`
`28`	`28`	`"message": "The value of 'aria-level' must be an integer",`
`29`	`29`	`"start": {`
`30`		`- "line": 5,`
	`30`	`+ "line": 4,`
`31`	`31`	`"column": 5`
`32`	`32`	`},`
`33`	`33`	`"end": {`
`34`		`- "line": 5,`
	`34`	`+ "line": 4,`
`35`	`35`	`"column": 15`
`36`	`36`	`}`
`37`	`37`	`}`
Original file line number	Diff line number	Diff line change
`@@ -1,7 +1,7 @@`
`1`	`1`	`[`
`2`	`2`	`{`
`3`	`3`	`"code": "a11y_incorrect_aria_attribute_type",`
`4`		`- "message": "The value of 'aria-valuemax' must be of type number",`
	`4`	`+ "message": "The value of 'aria-valuemax' must be a number",`
`5`	`5`	`"start": {`
`6`	`6`	`"line": 1,`
`7`	`7`	`"column": 5`
`@@ -13,7 +13,7 @@`
`13`	`13`	`},`
`14`	`14`	`{`
`15`	`15`	`"code": "a11y_incorrect_aria_attribute_type",`
`16`		`- "message": "The value of 'aria-valuemax' must be of type number",`
	`16`	`+ "message": "The value of 'aria-valuemax' must be a number",`
`17`	`17`	`"start": {`
`18`	`18`	`"line": 2,`
`19`	`19`	`"column": 5`
`@@ -25,14 +25,14 @@`
`25`	`25`	`},`
`26`	`26`	`{`
`27`	`27`	`"code": "a11y_incorrect_aria_attribute_type",`
`28`		`- "message": "The value of 'aria-valuemax' must be of type number",`
	`28`	`+ "message": "The value of 'aria-valuemax' must be a number",`
`29`	`29`	`"start": {`
`30`	`30`	`"line": 5,`
`31`	`31`	`"column": 5`
`32`	`32`	`},`
`33`	`33`	`"end": {`
`34`	`34`	`"line": 5,`
`35`		`- "column": 18`
	`35`	`+ "column": 25`
`36`	`36`	`}`
`37`	`37`	`}`
`38`	`38`	`]`
Original file line number	Diff line number	Diff line change
`@@ -1,4 +1,4 @@`
`1`		`-<div aria-label></div>`
	`1`	`+<div aria-label="true"></div>`
`2`	`2`	`<div aria-label={true}></div>`
`3`	`3`	`<div aria-label={false}></div>`
`4`	`4`	`<div aria-label={1234}></div>`