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.

Sign up for GitHub

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

fix(no-autoplay-audio): don't timeout for preload=none media elements #4684

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
+226 −137
Open

fix(no-autoplay-audio): don't timeout for preload=none media elements #4684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions lib/checks/media/no-autoplay-audio-evaluate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
function noAutoplayAudioEvaluate(node, options) {
const hasControls = node.hasAttribute('controls');

/**
* if the media loops then we only need to know if it has controls, regardless
* of the duration
*/
if (node.hasAttribute('loop')) {
return hasControls;
}

/**
* if duration cannot be read, this means `preloadMedia` has failed
*/
Expand All @@ -12,15 +22,15 @@ function noAutoplayAudioEvaluate(node, options) {
*/
const { allowedDuration = 3 } = options;
const playableDuration = getPlayableDuration(node);
if (playableDuration <= allowedDuration && !node.hasAttribute('loop')) {
if (playableDuration <= allowedDuration) {
return true;
}

/**
* if media element does not provide controls mechanism
* -> fail
*/
if (!node.hasAttribute('controls')) {
if (!hasControls) {
return false;
}

Expand Down
21 changes: 21 additions & 0 deletions lib/core/utils/preload-media.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,27 @@ function preloadMedia({ treeRoot = axe._tree[0] }) {
treeRoot,
'video, audio',
({ actualNode }) => {
/**
* Ignore media that won't load no matter how long we wait
* @see https://github.com/dequelabs/axe-core/issues/4665
*/
if (
actualNode.preload === 'none' &&
actualNode.networkState !== actualNode.NETWORK_LOADING
Comment on lines +21 to +23
Copy link

Choose a reason for hiding this comment

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

should we ignore the preload="metadata" too? it seems like it doesn't put the media in a ready to play state either

Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to wait for the media to be playable, we just need the metadata so we can identify the duration (that's why isMediaElementReady waits for any non-zero ready state, rather than specifically waiting for a contentful ready state)

) {
return false;
}

/**
* Ignore media nodes which are `paused` or `muted`
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this comment explain why it's doing this, as opposed to what it's doing? The what is already clear from the code.

*/
if (
actualNode.hasAttribute('paused') ||
Copy link
Contributor

Choose a reason for hiding this comment

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

no-autoplay-audio also omits cases without an autoplay attribute; did you leave out that case intentionally?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm just realizing now on a fresh reading of the spec that if we do restrict to only waiting for autoplay cases, we actually might want to not add the new logic for preload="none" because of this note in the spec:

Note: The autoplay attribute can override the preload attribute (since if the media plays, it naturally has to buffer first, regardless of the hint given by the preload attribute). Including both is not an error, however.

actualNode.hasAttribute('muted')
) {
return false;
}

/**
* this is to safe-gaurd against empty `src` values which can get resolved `window.location`, thus never preloading as the URL is not a media asset
*/
Expand Down
183 changes: 95 additions & 88 deletions test/checks/media/no-autoplay-audio.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,119 +1,126 @@
describe('no-autoplay-audio', function () {
'use strict';
describe('no-autoplay-audio', () => {
const check = checks['no-autoplay-audio'];
const checkSetup = axe.testUtils.checkSetup;
const checkContext = axe.testUtils.MockCheckContext();
const preloadOptions = { preload: { assets: ['media'] } };

var check;
var fixture = document.getElementById('fixture');
var checkSetup = axe.testUtils.checkSetup;
var checkContext = axe.testUtils.MockCheckContext();
var preloadOptions = { preload: { assets: ['media'] } };

before(function () {
check = checks['no-autoplay-audio'];
});

afterEach(function () {
fixture.innerHTML = '';
axe._tree = undefined;
afterEach(() => {
checkContext.reset();
});

it('returns undefined when <audio> has no source (duration cannot be interpreted)', function (done) {
var checkArgs = checkSetup('<audio id="target"></audio>');
axe.utils.preload(preloadOptions).then(function () {
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
done();
});
it('returns undefined when <audio> has no source (duration cannot be interpreted)', async () => {
const checkArgs = checkSetup('<audio id="target"></audio>');
await axe.utils.preload(preloadOptions);
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
});

it('returns undefined when <video> has no source (duration cannot be interpreted)', function (done) {
var checkArgs = checkSetup('<video id="target"><source src=""/></video>');
axe.utils.preload(preloadOptions).then(function () {
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
done();
});
it('returns undefined when <video> has no source (duration cannot be interpreted)', async () => {
const checkArgs = checkSetup('<video id="target"><source src=""/></video>');
await axe.utils.preload(preloadOptions);
assert.isUndefined(check.evaluate.apply(checkContext, checkArgs));
});

it('returns false when <audio> can autoplay and has no controls mechanism', function (done) {
var checkArgs = checkSetup(
it('returns false when <audio> can autoplay and has no controls mechanism', async () => {
const checkArgs = checkSetup(
'<audio id="target" src="/test/assets/moon-speech.mp3" autoplay="true"></audio>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
done();
});
await axe.utils.preload(preloadOptions);
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
});

it('returns false when <video> can autoplay and has no controls mechanism', function (done) {
var checkArgs = checkSetup(
'<video id="target" autoplay="true">' +
'<source src="/test/assets/video.webm" type="video/webm" />' +
'<source src="/test/assets/video.mp4" type="video/mp4" />' +
'</video>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
done();
});
it('returns false when <video> can autoplay and has no controls mechanism', async () => {
const checkArgs = checkSetup(`
<video id="target" autoplay="true">
Copy link

Choose a reason for hiding this comment

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

why are we explicitly specifying the value of the autoplay? it's it just a boolean attribute? E.g. autoplay = "false" is equal to autoplay="true" and will still autoplay.

<source src="/test/assets/video.webm" type="video/webm" />
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>
`);
await axe.utils.preload(preloadOptions);
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
});

it('returns false when <audio> plays less than allowed dutation but loops', function (done) {
var checkArgs = checkSetup(
it('returns false when <audio> plays less than allowed dutation but loops', async () => {
const checkArgs = checkSetup(
'<audio id="target" src="/test/assets/moon-speech.mp3#t=2,4" autoplay="true" loop="true"></audio>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
done();
});
await axe.utils.preload(preloadOptions);
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <video> can autoplay and duration is below allowed duration (by passing options)', function (done) {
var checkArgs = checkSetup(
'<video id="target" autoplay="true">' +
'<source src="/test/assets/video.webm" type="video/webm" />' +
'<source src="/test/assets/video.mp4" type="video/mp4" />' +
'</video>',
{ allowedDuration: 15 }
it('returns false when <video> loops and has no controls mechanism when duration is unknown', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the duration actually unknown for this (and all the other test cases meant to exercise unknown duration paths?) It looks like this is using the same media sources as the other test cases for known durations

const checkArgs = checkSetup(`
<video id="target" loop>
<source src="/test/assets/video.webm#t=7,9" type="video/webm" />
<source src="/test/assets/video.mp4#t=7,9" type="video/mp4" />
</video>
`);
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
});

it('returns false when <audio> loops and has no controls mechanism when duration is unknown', () => {
const checkArgs = checkSetup(
'<audio id="target" src="/test/assets/moon-speech.mp3#t=2,4" loop="true"></audio>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
done();
});
assert.isFalse(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <video> can autoplay and duration is below allowed duration (by setting playback range)', function (done) {
var checkArgs = checkSetup(
'<video id="target" autoplay="true">' +
'<source src="/test/assets/video.webm#t=7,9" type="video/webm" />' +
'<source src="/test/assets/video.mp4#t=7,9" type="video/mp4" />' +
'</video>'
// Note: default allowed duration is 3s
it('returns true when <video> can autoplay and duration is below allowed duration (by passing options)', async () => {
const checkArgs = checkSetup(
`
<video id="target" autoplay="true">
<source src="/test/assets/video.webm" type="video/webm" />
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>`,
{ allowedDuration: 15 }
);
axe.utils.preload(preloadOptions).then(function () {
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
done();
});
await axe.utils.preload(preloadOptions);
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <video> can autoplay and duration is below allowed duration (by setting playback range)', async () => {
const checkArgs = checkSetup(`
<video id="target" autoplay="true">
<source src="/test/assets/video.webm#t=7,9" type="video/webm" />
<source src="/test/assets/video.mp4#t=7,9" type="video/mp4" />
</video>`);
// Note: default allowed duration is 3s
await axe.utils.preload(preloadOptions);
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <audio> can autoplay but has controls mechanism', function (done) {
var checkArgs = checkSetup(
it('returns true when <audio> can autoplay but has controls mechanism', async () => {
const checkArgs = checkSetup(
'<audio id="target" src="/test/assets/moon-speech.mp3" autoplay="true" controls></audio>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
done();
});
await axe.utils.preload(preloadOptions);
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <video> can autoplay and has controls mechanism', async () => {
const checkArgs = checkSetup(`
<video id="target" autoplay="true" controls>
<source src="/test/assets/video.webm" type="video/webm" />
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>
`);
await axe.utils.preload(preloadOptions);
assert.isTrue(check.evaluate.apply(null, checkArgs));
});

it('returns true when <video> loops and has controls mechanism when duration is unknown', () => {
const checkArgs = checkSetup(`
<video id="target" loop controls>
<source src="/test/assets/video.webm#t=7,9" type="video/webm" />
<source src="/test/assets/video.mp4#t=7,9" type="video/mp4" />
</video>
`);
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});

it('returns true when <video> can autoplay and has controls mechanism', function (done) {
var checkArgs = checkSetup(
'<video id="target" autoplay="true" controls>' +
'<source src="/test/assets/video.webm" type="video/webm" />' +
'<source src="/test/assets/video.mp4" type="video/mp4" />' +
'</video>'
it('returns true when <audio> loops and has controls mechanism when duration is unknown', () => {
const checkArgs = checkSetup(
'<audio id="target" src="/test/assets/moon-speech.mp3#t=2,4" controls="true" loop="true"></audio>'
);
axe.utils.preload(preloadOptions).then(function () {
assert.isTrue(check.evaluate.apply(null, checkArgs));
done();
});
assert.isTrue(check.evaluate.apply(checkContext, checkArgs));
});
});
95 changes: 53 additions & 42 deletions test/core/utils/preload-media.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,54 +1,68 @@
describe('axe.utils.preloadMedia', function () {
'use strict';
describe('axe.utils.preloadMedia', () => {
const fixtureSetup = axe.testUtils.fixtureSetup;

var fixture = document.getElementById('fixture');
var fixtureSetup = axe.testUtils.fixtureSetup;

afterEach(function () {
fixture.innerHTML = '';
});

it('returns empty array when there are no media nodes to be preloaded', function (done) {
it('returns empty array when there are no media nodes to be preloaded', async () => {
axe._tree = axe.utils.getFlattenedTree(document);

axe.utils.preloadMedia({ treeRoot: axe._tree[0] }).then(function (result) {
assert.equal(result.length, 0);
done();
});
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns empty array when <audio> has no source', function (done) {
it('returns empty array when <audio> has no source', async () => {
fixtureSetup('<audio autoplay="true" controls></audio>');

axe.utils.preloadMedia({ treeRoot: axe._tree[0] }).then(function (result) {
assert.equal(result.length, 0);
done();
});
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns empty array when <video> has no source', function (done) {
it('returns empty array when <video> has no source', async () => {
fixtureSetup('<video id="target"><source src=""/></video>');
axe.utils.preloadMedia({ treeRoot: axe._tree[0] }).then(function (result) {
assert.equal(result.length, 0);
done();
});
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns media node (audio) after their metadata has been preloaded', function (done) {
it('returns empty array when media node does not preload', async () => {
fixtureSetup(`
<video id="target" preload="none">
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>
`);
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns empty array when media node is muted', async () => {
fixtureSetup(`
<video id="target" muted>
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>
`);
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns empty array when media node is paused', async () => {
fixtureSetup(`
<video id="target" paused>
<source src="/test/assets/video.mp4" type="video/mp4" />
</video>
`);
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 0);
});

it('returns media node (audio) after their metadata has been preloaded', async () => {
fixtureSetup(
'<audio src="/test/assets/moon-speech.mp3" autoplay="true" controls></audio>'
);

axe.utils.preloadMedia({ treeRoot: axe._tree[0] }).then(function (result) {
assert.equal(result.length, 1);
assert.isTrue(result[0].readyState > 0);
assert.equal(Math.round(result[0].duration), 27);

done();
});
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 1);
assert.isTrue(result[0].readyState > 0);
assert.equal(Math.round(result[0].duration), 27);
});

it('returns media nodes (audio, video) after their metadata has been preloaded', function (done) {
it('returns media nodes (audio, video) after their metadata has been preloaded', async () => {
fixtureSetup(
// 1 audio elm
'<audio src="/test/assets/moon-speech.mp3"></audio>' +
Expand All @@ -59,15 +73,12 @@ describe('axe.utils.preloadMedia', function () {
'</video>'
);

axe.utils.preloadMedia({ treeRoot: axe._tree[0] }).then(function (result) {
assert.equal(result.length, 2);
assert.isTrue(result[0].readyState > 0);
assert.equal(Math.round(result[0].duration), 27);

assert.isTrue(result[1].readyState > 0);
assert.equal(Math.round(result[1].duration), 14);
const result = await axe.utils.preloadMedia({ treeRoot: axe._tree[0] });
assert.equal(result.length, 2);
assert.isTrue(result[0].readyState > 0);
assert.equal(Math.round(result[0].duration), 27);

done();
});
assert.isTrue(result[1].readyState > 0);
assert.equal(Math.round(result[1].duration), 14);
});
});
Loading