search

alphagov / accessibility-tool-audit

Automated accessibility tools audit
https://alphagov.github.io/accessibility-tool-audit/
MIT License
80 stars 30 forks source link

Remove less helpful categories #33

Closed selfthinker closed 6 years ago

selfthinker commented 6 years ago

The three result types 'Different issue found', 'Wrong issue reported' and 'Unrelated issue' are a) too similar and b) not very important and c) apply to a lot more results than the ones we have currently categorised as such. They should either be simplified and consolidated or removed.

When I discussed this with @aduggin, he said he would be happy to get rid of them if all the categories they get replaced with make sense. During the re-testing of some of the tools I already replaced most of the results with those categories. This change is about getting rid of the final 12 instances.

I have re-tested those 12 instances and am quite happy with 10 of those being replaced. Only two (quite similar cases) give me a headache and I'm not quite sure what to do with them:

Tenon's error message for link with a role=button does not work with space bar is:

This element has a role attribute even though native semantics are available. Replace this element's role attribute with native semantics.

While our test is too descriptive (it could be a more generic "Link with a role=button doesn't have all typical button attributes and behaviour"), Tenon's error message doesn't really say what the problem is. Although having "a role attribute even though native semantics are available" is bad practice, if it's implemented correctly, it wouldn't necessarily lead to accessibility issues. But on the other hand, using their solution (to replace with native semantics) would fix our original issue. So far I've marked this with "Issue found" but none of our current categories really fit (including the ones I'm removing here).

HTML_CodeSniffer's warning message for inline style adds colour is:

Check that this element has an inherited background colour or image to complement the corresponding inline foreground colour.

While our test is not exactly a really good one, it is meant to show issues users might have when they change their colour settings. From that aspect HTML_CodeSniffer found more or less what we were looking for, but is maybe not specific enough. So far I've marked this with "Warning only" but, again, none of our current categories really fit here either.

selfthinker commented 6 years ago

This PR is marked with "[DON'T MERGE]" because I'm looking for some feedback first and haven't generated the flat files yet.

selfthinker commented 6 years ago

I wonder if we should introduce a new neutral category "Related issue found"? I don't think it's worth mentioning if it's unrelated, that should be the same as "Not found". But when the issue is related and is nearly the issue we were looking for but there is something not sufficient with it, maybe it should be different from "Not found". It's not a warning, it's not anything to check, and it's also not "noticed but not a fail" because it is a fail but not exactly the one we were looking for.

cfq commented 6 years ago

Hi @selfthinker,

I remember being torn between "different issue found" and "wrong issue reported" whilst testing because on the one hand, they did detect a partial or related issue but on the other hand they didn't really fit any of the boxes. Luckily we don't have many tests that don't fit in any of the definite categories. I also agree it would be beneficial to aggregate them under a single group since it doesn't really make a significant difference in stats and we're not sure if it communicates much to users.

Like you mentioned, it would be interesting to reconsider our tests if most of the tools are inconclusive on a definite negative or positive result.

selfthinker commented 6 years ago

Thanks for your thoughts @cfq.

I've discussed the two remaining issues with @aduggin and we decided to change them both the "Not found". I forgot to discuss a potential "related issue" category with him, but I think we'll just add that when the need arises.