Add new rule `no-invalid-aria-attributes` #2276

judithhinlung · 2021-12-14T00:00:02Z

This PR adds the new rule no-invalid-aria-attributes which fixes #2275 .

Background

Currently, Ember-template-lint does not prevent invalid ARIA attributes, or any ARIA-* properties not found in the WAI-ARIA States and Properties spec, on an element. Using invalid ARIA attributes reduces usability by failing to provide the element it belongs to with the intended accessibility function.

This proposes the authoring of a new rule that enforces that ARIA attributes should be valid, and invalid attributes will generate an error.
Reference:
Supported States and Properties | Accessible Rich Internet Applications (WAI-ARIA) 1.0

Allowed:

# examples
<input type="email" aria-required="true" />
<div role="region" aria-live="polite">Valid live region</div>

Forbidden:

# examples
<input type="text" aria-text="inaccessible text" />
<div role="alert" aria--live="polite"></div>

Testing

Test Suites: 130 passed, 130 total
Tests: 6 skipped, 7051 passed, 7057 total
Snapshots: 887 passed, 887 total
Time: 50.697 s, estimated 55 s
Ran all test suites.
✨ Done in 62.73s.

bmish · 2021-12-14T00:02:04Z

lib/rules/no-invalid-aria-attributes.js

+}
+
+module.exports = class NoInvalidAriaAttributes extends Rule {
+  logNode({ node, message }) {


This helper doesn't seem to be necessary, can we remove it?

Let's remove this logNode helper since it's no better than just calling this.log directly.

lib/rules/no-invalid-aria-attributes.js

docs/rule/no-invalid-aria-attributes.md

bmish · 2021-12-14T00:21:31Z

docs/rule/no-invalid-aria-attributes.md

@@ -0,0 +1,22 @@
+# no-invalid-aria-attributes
+
+This rule checks for the use of invalid ARIA attributes, or any aria-\* property on an element that is not included in the [WAI-ARIA States and Properties spec](https://www.w3.org/WAI/PF/aria-1.1/states_and_properties).


Can you explain what happens when someone uses a non-existent aria attribute? E.g. A non-existent ARIA attribute has no effect.

bmish · 2021-12-14T00:22:33Z

docs/rule/no-invalid-aria-attributes.md

+
+## Examples
+
+This rule **forbids** the following:


Is there anything else we can validate about ARIA attributes? Anything we validate about their values or what attributes they are used on? It's easier to make this rule comprehensive now and cover multiple things, since adding violations later will be a breaking change.

bmish · 2021-12-14T00:23:16Z

lib/config/a11y.js

@@ -13,6 +13,7 @@ module.exports = {
    'no-duplicate-landmark-elements': 'error',
    'no-empty-headings': 'error',
    'no-heading-inside-button': 'error',
+    'no-invalid-aria-attributes': 'error',


I imagine we should add this to recommended in the upcoming v4 release right?

docs/rule/no-invalid-aria-attributes.md

README.md

judithhinlung · 2021-12-16T22:39:57Z

aria-query is a package that contains all the existing ARIA attributes and their valid types and values, which would be very useful in implementing the attribute-value checks. However, when looking at the other lint rules, I don't see any of them using an external import. Is it preferred for ETL rules to be self-contained, or could I leverage this package in my implementation of this rule?

bmish · 2021-12-16T22:55:39Z

aria-query is a package that contains all the existing ARIA attributes and their valid types and values, which would be very useful in implementing the attribute-value checks. However, when looking at the other lint rules, I don't see any of them using an external import. Is it preferred for ETL rules to be self-contained, or could I leverage this package in my implementation of this rule?

@judithhinlung I'm open to adding this dependency, since it sounds like it will make this lint rule much more powerful. Let's give it a try.

judithhinlung · 2021-12-24T00:15:18Z

I have updated this PR to use the NPM package Aria-query as the source of truth for ARIA validation. This PR now also includes validation for ARIA property types and the use of permitted values. The no-invalid-aria-attribute rule is now essentially a direct port of the JSX rules aria-props and Aria-proptypes.

bmish · 2021-12-24T15:10:18Z

@judithhinlung it looks like there is missing test coverage so the tests are failing. Note that we need to ensure every line in the rule file is covered by tests.

  no-invalid-aria-attributes.js                  |   88.24 |     80.3 |     100 |   88.24 | 26-28,42,44,48,54,80,87

bmish

Nice work on this so far.

bmish · 2021-12-24T15:20:55Z

lib/rules/no-invalid-aria-attributes.js

+}
+
+module.exports = class NoInvalidAriaAttributes extends Rule {
+  logNode({ node, message }) {


Let's remove this logNode helper since it's no better than just calling this.log directly.

lib/rules/no-invalid-aria-attributes.js

bmish · 2022-01-04T14:54:44Z

@judithhinlung do you think we can finish this up this week? If so, we can include it as a recommended rule in the upcoming v4 release (#1908) which I'd like to release later this week.

judithhinlung · 2022-01-04T17:23:55Z

Hey @bmish unfortunately I don't think I will be able to finish this until sometime next week, when I return from my vacation.

judithhinlung · 2022-01-13T00:05:00Z

I have updated this PR with more test coverage, removed the extraneous logNode helper, and modified the invalid-ARIA-attribute message to say that the attribute is not recognized.

drewlee · 2022-01-13T00:26:03Z

package.json

@@ -61,6 +61,7 @@
    ]
  },
  "dependencies": {
+    "aria-query": "^5.0.0",


@judithhinlung Looks like the build is failing because the listed deps are out of alphabetical order. aria-query should come after @lint-todo/utils.

…he aria-query dependency

bmish

Looks good. Is this still a draft or is it ready?

lib/rules/no-invalid-aria-attributes.js

bmish · 2022-01-18T22:52:25Z

Thanks for your great work on this!

Add new rule no-invalid-aria-attributes

0763126

judithhinlung force-pushed the no-invalid-aria-attributes branch from 5bd7274 to 0763126 Compare December 14, 2021 00:12

bmish added the enhancement label Dec 14, 2021

bmish requested changes Dec 14, 2021

View reviewed changes

drewlee reviewed Dec 14, 2021

View reviewed changes

README.md Outdated Show resolved Hide resolved

judithhinlung added 3 commits December 14, 2021 11:50

Removed intermediary helper to make code more efficient

ed6d1ad

Updated README formatting

2f56850

Updated docs

d241316

judithhinlung added 3 commits December 20, 2021 17:47

Updated rule to import from aria-query

7658a0b

Added aria-attribute validation

729fafe

Merge branch 'master' into no-invalid-aria-attributes

25ecb72

bmish requested changes Dec 24, 2021

View reviewed changes

bmish mentioned this pull request Jan 6, 2022

Plan v5 Release #2319

Closed

judithhinlung added 3 commits January 12, 2022 15:54

Expanded test coverage

6db7ef2

Merge branch 'master' into no-invalid-aria-attributes

7e31004

Updated rule to be compatible with v4

f1748a7

drewlee reviewed Jan 13, 2022

View reviewed changes

added tests for attributes that allow undefined values, reordered t…

4d29ca3

…he aria-query dependency

bmish approved these changes Jan 15, 2022

View reviewed changes

lib/rules/no-invalid-aria-attributes.js Show resolved Hide resolved

judithhinlung marked this pull request as ready for review January 18, 2022 19:21

bmish merged commit fbf4790 into ember-template-lint:master Jan 18, 2022

bmish added this to the 4.1.0 milestone Jan 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new rule `no-invalid-aria-attributes` #2276

Add new rule `no-invalid-aria-attributes` #2276

judithhinlung commented Dec 14, 2021 •

edited

bmish Dec 14, 2021

bmish Dec 24, 2021

bmish Dec 14, 2021

bmish Dec 14, 2021

bmish Dec 14, 2021

judithhinlung commented Dec 16, 2021

bmish commented Dec 16, 2021 •

edited

judithhinlung commented Dec 24, 2021

bmish commented Dec 24, 2021

bmish left a comment

bmish Dec 24, 2021

bmish commented Jan 4, 2022

judithhinlung commented Jan 4, 2022

judithhinlung commented Jan 13, 2022

drewlee Jan 13, 2022

bmish left a comment

bmish commented Jan 18, 2022

		@@ -0,0 +1,22 @@
		# no-invalid-aria-attributes

		This rule checks for the use of invalid ARIA attributes, or any aria-\* property on an element that is not included in the [WAI-ARIA States and Properties spec](https://www.w3.org/WAI/PF/aria-1.1/states_and_properties).

Add new rule no-invalid-aria-attributes #2276

Add new rule no-invalid-aria-attributes #2276

Conversation

judithhinlung commented Dec 14, 2021 • edited

Background

Testing

bmish Dec 14, 2021

Choose a reason for hiding this comment

bmish Dec 24, 2021

Choose a reason for hiding this comment

bmish Dec 14, 2021

Choose a reason for hiding this comment

bmish Dec 14, 2021

Choose a reason for hiding this comment

bmish Dec 14, 2021

Choose a reason for hiding this comment

judithhinlung commented Dec 16, 2021

bmish commented Dec 16, 2021 • edited

judithhinlung commented Dec 24, 2021

bmish commented Dec 24, 2021

bmish left a comment

Choose a reason for hiding this comment

bmish Dec 24, 2021

Choose a reason for hiding this comment

bmish commented Jan 4, 2022

judithhinlung commented Jan 4, 2022

judithhinlung commented Jan 13, 2022

drewlee Jan 13, 2022

Choose a reason for hiding this comment

bmish left a comment

Choose a reason for hiding this comment

bmish commented Jan 18, 2022

Add new rule `no-invalid-aria-attributes` #2276

Add new rule `no-invalid-aria-attributes` #2276

judithhinlung commented Dec 14, 2021 •

edited

bmish commented Dec 16, 2021 •

edited