dequelabs / axe-core

Accessibility engine for automated Web UI testing
https://www.deque.com/axe/
Mozilla Public License 2.0
5.86k stars 762 forks source link

Rule aliases #1006

Open WilcoFiers opened 6 years ago

WilcoFiers commented 6 years ago

The axe rules don't have the best IDs. We've more than a bit inconsistent about them. Some describe what should happen (for example "frame-title-unique"), others describe what shouldn't happen ("empty-heading"). Some include verbs (html-has-lang) others are just nouns (image-alt). Sometimes we put the adjective first (valid-lang) and other times we put it last (scope-attr-valid). We sometimes include a rule "scope" such as "landmark-*", we sometimes use -attr in the rule ID, but not all the time. In short, the IDs are all over the place.

Changing IDs is a breaking change, regardless of how we do it. But one way we may be able to lessen the pain of that is if we use aliases for our rules. The basic idea is: Let's keep the old ID in an alias property on the rule, so that if axe encounters an ID it doesn't know, it can look at the aliases and see if there's a match there. Doing that ensures that existing configurations don't throw errors. Because the IDs change in the output, some work will need to be done by anyone consuming the data to ensure continuity throughout major versions - but that's a solvable problem.

Anyway, here's what it'd look like:

// aria-valid-role-value.json
{
  "id": "aria-valid-role-value",
  "aliases": ["aria-roles"],
  ...
}

This could be a solution to #770

marcysutton commented 6 years ago

I like this idea! In the case above though, should we swap the two so the actual rule name is the desired one, and the alias is simply for backwards compatibility?

// aria-valid-role-value.json
{
  "id": "aria-roles",
  "aliases": ["aria-valid-role-value"],
  ...
}
WilcoFiers commented 6 years ago

Hmm, thinking about it some more. I think this feature should only be used to deprecate rules. So maybe instead of alias we called it deprecates, and you include the ID of the rule it replaced. That way we can log a warning that the rule you've tried to run is deprecated, but we don't throw an error.

Before 3.0 axe ignored when you put a rule in runOnly that it didn't know about. As of 3.0 it throws an error when you do. That seems appropriate when you made a typo in the rule ID or something, but not for rules that got deprecated. With this, axe could run the new rule, and warn users that they need to update their configuration.

jeeyyy commented 6 years ago

The above sounds like the perfect solution for - https://github.com/dequelabs/axe-core/issues/1062

If I understand this right, the suggestion is if we have a rule by the name of my-current-id and we want to change it to my-new-id, we are suggesting that we create the below structure in the rules meta data.

{
"id": "my-new-id",
"deprecates": ["my-current-id"]
}

I am in favour of "deprecates" keyword.