Skip to content
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(audits): Disable video-description #10128

Closed
wants to merge 2 commits into from

Conversation

Malvoz
Copy link
Contributor

@Malvoz Malvoz commented Dec 18, 2019

axe deprecated their video-description rule in dequelabs/axe-core#1737 due to poor implementations of the descriptions keyword for the <track kind=""> attribute. I'm unsure whether their deprecation warrants a complete removal, this PR simply disables the audit.

(I reordered the audits alphabetically for readability.)

@patrickhulce
Copy link
Collaborator

cc @robdodson do you agree?

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @Malvoz! would you mind preserving the order of we're specifically enabling these, but skip these alphabetical within those two groups is a great idea

we probably need a comment for that implied ordering of enabled: true/enabled: false...

@robdodson
Copy link
Contributor

I recall @mfriesenhahn was working on this. I don't quite recall where we last left things, but I do seem to remember that no browser really supports this at the moment—I think it requires users to install an extension to get the descriptions to play. Michael, what do you think about having LH disable the audit or moving it to manual audit?

@mfriesenhahn
Copy link
Collaborator

We're waiting on the LH folks to confirm that the audit is being switched to a manual check before publishing the update :)

@patrickhulce
Copy link
Collaborator

Oh was there already a conversation about this I don't know about and/or have forgotten about @mfriesenhahn? Any chance you know the link? :)

@mfriesenhahn
Copy link
Collaborator

Here's the web.dev PR where @kaycebasques mentions that y'all expect to have audits converted to manual in Q2.

@patrickhulce
Copy link
Collaborator

Gotcha thanks for that @mfriesenhahn! I don't have access to either thread that @kaycebasques links to in that PR for what we're waiting on, so I'll defer to someone internal on this one.

@robdodson
Copy link
Contributor

This is what @paulirish says in the first thread:

so first, we'll be removing video-description when we update axe next (as they just deprecated it).

and for audio-caption, video-caption:
if axe passes it. we'll pass it.
if axe marks them as incomplete, we'll switch them to manual, to tell the user that they have to review their videos individually to verify they have captions delivered via JavaScript.
This requires a bit more internals refactoring so it's not going to happen in our next release, however.

More likely it's a Q2 thing.

@mfriesenhahn
Copy link
Collaborator

Thanks, @robdodson! I actually didn't have access to those threads either, so this is the first time I'm seeing the details. Very helpful.

@patrickhulce
Copy link
Collaborator

so first, we'll be removing video-description when we update axe next

Alright that sounds like a go-ahead to me!

@Malvoz we can delete the video-description audit code, remove it from the config, and eliminate any tests if you're up for it :)

@paulirish
Copy link
Member

@Malvoz are you still up for removing this audit? patrick's comment covers what is needed.

@Malvoz
Copy link
Contributor Author

Malvoz commented Sep 28, 2020

@paulirish and others, sorry for stalling. I am quite busy at the moment so if anyone can take this on (by any means) that'd be great.

@connorjclark
Copy link
Collaborator

Closing, will do as part of #11207

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants