-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core(a11y): update scoring weights based on severity #8823
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aria- ones got big upgrades. so did meta-refresh
bypass got de-prioritized a bit.
and label/image-alt/link-name just arent as big deals as they were.
all fine with me just calling it out.
can you leave a comment somewhere that indicates where we can pull these severity weights from? i feel like it is in the axe response somewhere.. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems like there's not a ton of differentiation anymore, should the gap between critical and minor issues be wider?
originally it was also supposed to include how common it was, is that not much of a concern anymore?
Yeah so I'm not 100% happy with this scoring, though it is based on how axe
ranks items from minor, serious, critical.
Downranking something because it is uncommon in hindsight feels wrong
because it means getting an Aria role wrong--which is critically bad--is
considered low impact in current lighthouse.
Another option would be to boost the Aria scores. That's actually what got
me started on this idea.
*From: *Patrick Hulce <notifications@github.com>
*Date: *Thu, May 2, 2019, 6:48 PM
*To: *GoogleChrome/lighthouse
*Cc: *Rob Dodson, Author
*@patrickhulce* commented on this pull request.
…
seems like there's not a ton of differentiation anymore, should the gap
between critical and minor issues be wider?
originally it was also supposed to include how common it was, is that not
much of a concern anymore?
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#8823 (review)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAIEKDIQL6YNRX36KWJNVKDPTOKXXANCNFSM4HKNW2YQ>
.
|
OK I staggered things a bit more so minor is 1, serious is 3, and critical is 10. In testing this seems to produce a result that feels about right. This change is tricky because I think a lot of sites will see their scores go up but that's because we were penalizing them for issues that were common but not necessarily critical failures. On the flip side, there were critical failures that we were severely undercounting because they were uncommon. For example, having a button without a label—if it's the only failure on the page—is currently worth ~21 points. But misspelling the In the new scheme they are both worth something like 10 or 11 points. Things which are common, but not necessarily show stoppers, like color contrast are downranked from critical to serious. A lot of sites will benefit from this, but it really didn't make sense to say it was "more severe" than having uncaptioned video or audio. Those are legitimate barriers to access, even if they are less common. One thing that does make me feel better is the W3C bad accessibility example pages all score lower in the new scheme than in current Lighthouse. That's because they have more legitimate barriers to access so they get penalized more.
@paulirish in the json or here on github? |
sorry, I changed some stuff. You may need to |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @robdodson! New scores LGTM after we sort out this sample_v2 business
@brendankenny I have done the thing. |
in the default-config.js right at the top of the auditRefs block. |
Added |
@@ -79,7 +79,8 @@ const UIStrings = { | |||
seoCategoryManualDescription: 'Run these additional validators on your site to check additional SEO best practices.', | |||
/* Title of the navigation section within the Search Engine Optimization (SEO) category. Within this section are audits with descriptive titles that highlight opportunities to make a page more usable on mobile devices. */ | |||
seoMobileGroupTitle: 'Mobile Friendly', | |||
/* Description of the navigation section within the Search Engine Optimization (SEO) category. Within this section are audits with descriptive titles that highlight opportunities to make a page more usable on mobile devices. */ | |||
/* Description of the navigation section within the Search Engine Optimization (SEO) category. Within this section are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
errant hit of the return key? :) this is breaking the strings test (I think maybe we aren't set up to extract multi-line comments into message descriptions) and linting (no trailing spaces)
thanks for getting this in so fast, @robdodson! |
sure thing! Thanks for being so accommodating y'all |
Summary
Re-weight a11y audits using the same severity scale that axe uses for their docs.
Items which really should be critical failures were weighted too low.
https://dequeuniversity.com/rules/axe/3.2/
Related Issues/PRs
#3444