Axe-core: Impact being bound to checks is limiting when composing new rules

[rdeltour]

The _impact_ property ("minor", "moderate", "serious", or "critical") is currently bound to a _check_. It means that when composing a new _rule_, this rule will necessarily inherit the impacts of the underlying checks.

In my understanding however, a check is an "abstract" atomic piece of logic. A _rule_ on the contrary is the "concrete" implementation of a given accessible guideline. The actual impact of a violation only makes sense for a given rule.

For instance, the non-empty-title check has a serious impact. But there may be cases where a rule checks the presence of empty titles but isn’t about a serious violation (we have such rules in the digital publishing context). In that case, we have no other choice than implementing a custom check with a lesser impact, even if the implementation logic is exactly the same as non-empty-title.

A possible improvement IMO would be to associate the impact to the check only when the rule is composed, but not bake it in the check.

[WilcoFiers]

Can you clarify what you mean with:

associate the impact to the check only when the rule is composed

[rdeltour]

Can you clarify what you mean with:

associate the impact to the check only when the rule is composed

If the rule’s violation impact depends on which check actually failed, then the checks’ impacts can be bound (or overridden) when the rule is configured. Concretely, the any, all, and none fields of the rule, could be set to (in addition to arrays of string IDs) arrays of tuples (ID,impact).

For instance:

{
  "id": "aria-roles",
  "selector": "[role]",
   (…)
  "all": [],
  "any": [],
  "none": [
    ["invalidrole", "critical"],
    ["abstractrole", "serious"],
    ["unsupportedrole", "moderate"]
  ]
}

[WilcoFiers]

Had a quick talk with the team. I think your proposal overcomplicates things. It would certainly take a bunch of reworking of internal tools, so I'm not a fan of that. I think we should simplify this instead. we think it would be better to simply allow an impact property on the rule metadata object. This can override whatever comes out of the checks.

[rdeltour]

we think it would be better to simply allow an impact property on the rule metadata object. This can override whatever comes out of the checks.

Yes, I think it's a good intermediary workaround.

In fact my first answer draft included something similar to what you describe, as an alternative to my comment above :-) but since the clarification was specifically about check/impact association, I ended up pruning that part…

By the way, FWIW this issue is currently not critical to us (DAISY). I just logged it here after @marcysutton asked for API feedback for 3.0 the other day on Twitter.

[chaals]

It makes little sense to me to have the impact on checks, since the same check can be used by different rules in different ways. I would prefer it was only a property on the rule.

It might be more complicated - some rule failures matter a lot in practice, and some don't. I'm not sure there is a simple way of describing how that works for automation...

[dylanb]

I agree that it makes more sense for the impact to be attached to the check, however, it makes sense to attach that impact on the rule to the check.

We could make this backwards compatible by making the list of checks a list of check names or objects that contain a check name and an impact. E.g.

Currently, the checks section for the label rule looks like this:

  "all": [],
  "any": [
    "aria-label",
    "aria-labelledby",
    "implicit-label",
    "explicit-label",
    "non-empty-title"
  ],
  "none": [
    "help-same-as-label",
    "multiple-label"
  ]

It might look like this if we wished to override/specify the impact of the check within the context of the rule:

  "all": [],
  "any": [
    "aria-label",
    "aria-labelledby",
    "implicit-label",
    "explicit-label",
    "non-empty-title"
  ],
  "none": [
    { "ruleId": "help-same-as-label", "impact": "minor" },
    "multiple-label"
  ]

[WilcoFiers]

I think that just overcomplicates things. Very few rules can actually come up with issues of different levels of impact (aria-roles, button-name, label, link-name and scope-attr-valid). And only for one of their checks - one that should probably have been its own rule instead of getting tagged onto an existing rule. I think if you have multiple impact levels, you're probably dealing with different issues, in which case they shouldn't be part of the same rule IMO.

[dylanb]

Having a different impact for checks is only one use case.

The other common case is having two different rules with different impacts for the same check.

Custom rule writers quite often require this and right now we have to duplicate the check to achieve this.

[WilcoFiers]

@dylanb That would also be solved if you can simply override the impact at the rule level. What is the benefit of overriding the impact of the check at the rule level?

[dylanb]

What you are proposing is that the ability to have different impact level failures for a single rule is not valuable. I am not convinced that your assertion is true.

If it is true, then what we should be talking about is a fundamental rearchitecting of this feature. If we don't do that, then adding an override at the rule level is only a partial solution.

[WilcoFiers]

Why I'm proposing we make this configurable at the rule level, is because I think that changing the impact of a rule should be something users can do without having to learn how impact aggregation works in checks. You shouldn't have to be an advanced user to do something that's this trivial.

Now we could add your check configuration idea in addition to that. The change I'm suggesting doesn't prevent us from adding your solution as well. But I suspect that adding this "basic" feature will be enough that people won't have to duplicate checks anymore, or at least only in very rare cases.

[padmavemulapati]

Verified by configuring the rule html-has-lang , from checks its impact is "serious".
tried to make it minor with the below method.
axe.configure({ rules: [{ id: 'html-has-lang', impact: "minor" }] });
Not able to see the impact. @straker / @WilcoFiers please help me out for the right configure

[straker]

@padmavemulapati you are correct. Put in a pr that should fix this.

[straker]

@padmavemulapati should be good to test this again.

[padmavemulapati]

Thank you @straker , its working now. I am able to see the modified impact at rule level update, instead checks level.
verified for html-has-lang rule.

[jeankaplansky]

included as a new feature in internal release notes 7/29/2020