-
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
deps: update axe-core to 3.5.1 #10344
deps: update axe-core to 3.5.1 #10344
Conversation
Thanks very much @jayaddison! Looks like we have a few nice changes in that 3.5.0 update (though
|
This comment has been minimized.
This comment has been minimized.
I'm not really sure why yet, but following a more successful attempt at |
This comment has been minimized.
This comment has been minimized.
Ok, I'm gradually gaining more context here. The failing test is an accessibility self-test over the HTML output produced by the The axe-core 3.5.1 rule which flags the failure is designed to ensure that perceivable elements in a rendered page belong to accessibility landmarks, as documented in ARIA's authoring practices. From examining the situation locally, it looks like the updated CSS selector for the I've not yet been able to work out why the new selector changes the test behaviour -- it still looks valid in theory against the NB: A wildcard selector is only used in one other axe-core 3.5.1 rule - the |
In dequelabs/axe-core#2047, @straker helped determine that the test failures here appeared valid. Commit 14f0454 modifies the rendering test to ensure that content is rendered inside a My working theory is that the test in question may not have been working as expected in the past, and that the |
This comment has been minimized.
This comment has been minimized.
You can add the new audits to lighthouse/lighthouse-core/gather/gatherers/accessibility.js Lines 35 to 46 in 53a8619
with |
This comment has been minimized.
This comment has been minimized.
'duplicate-id-aria': {enabled: false}, | ||
'landmark-no-duplicate-contentinfo': {enabled: false}, |
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.
I'm not completely certain whether it's fair to associate both of these disabled tests with #9432. duplicate-id-aria
seems likely to be related; I'm not 100% sure on whether landmark-no-duplicate-contentinfo
is.
This comment has been minimized.
This comment has been minimized.
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.
sorry for the delay @jayaddison, paul is traveling so I'll take over this PR
I think the diff will become much, much smaller after these changes, might be easier to start fresh with a git reset origin/master && git add -A && git commit -m 'reset'
and cherry-pick back in your accessibility gatherer changes
This comment has been minimized.
This comment has been minimized.
This reverts commit 0818a41.
Yes, diff looks beautiful now! 👍 Not sure what is up with that travis failure, looks like it was caused by a previous failure of firefox extension build crashing and we didn't get a chance to build all assets? My best guess make sure it's merged with master and we'll retry. |
(confirmed up-to-date with master) |
hm weird, just cleared the cache for that PR and we'll see if it fixes things. |
So close :) |
This reverts commit b8d9fc6.
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 very much @jayaddison!
Summary
Updates axe-core==3.5.1 following its release on 2020-02-13 (release tag).