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

Jump to bottom

fix: Do not require media captions / descriptions #1075

Merged
merged 4 commits into from
Aug 23, 2018
Merged

fix: Do not require media captions / descriptions #1075

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c7ca83b
fix: Do not require media captions / descriptions
WilcoFiers Aug 15, 2018
ad112f9
chore: Resolve PR feedback
WilcoFiers Aug 22, 2018
c3bb01b
Merge branch 'develop' into media-no-fails
WilcoFiers Aug 22, 2018
10daa8d
Merge branch 'develop' into media-no-fails
dylanb Aug 23, 2018
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
43 changes: 34 additions & 9 deletions build/tasks/validate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,27 @@ function hasUniqueId() {
};
}

function hasTwoOutcomes(messages) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like this is normalising multiple outcomes, should change function name to something generic like normaliseOutcomes or hasMultipleOutcomes rather than exactly Two?

const keys = Object.keys(messages);
if (keys.length < 2) {
return false;
}

return keys.every(key => {
switch (key) {
case 'pass':
case 'fail':
return typeof messages[key] === 'string';

case 'incomplete':
return ['string', 'object'].includes(typeof messages[key]);

default:
return false;
}
});
}

function createSchemas() {
var schemas = {};

Expand Down Expand Up @@ -84,16 +105,20 @@ function createSchemas() {
messages: {
required: true,
type: 'object',
properties: {
fail: {
required: true,
type: 'string'
},
pass: {
required: true,
type: 'string'
}
conform: hasTwoOutcomes,
messages: {
conform: 'Must have at least two valid messages'
}
// propferties: {
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: 🤣 propferties, know it is commented.... delete instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah, delete this comment

// fail: {
// required: true,
// type: 'string'
// },
// pass: {
// required: true,
// type: 'string'
// }
// }
},
impact: {
required: true,
Expand Down
17 changes: 7 additions & 10 deletions lib/checks/media/caption.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,8 @@
var tracks = axe.utils.querySelectorAll(virtualNode, 'track');
const tracks = axe.utils.querySelectorAll(virtualNode, 'track');
const hasCaptions = tracks.some(
({ actualNode }) =>
(actualNode.getAttribute('kind') || '').toLowerCase() === 'captions'
);

if (tracks.length) {
// return false if any track has kind === 'caption'
return !tracks.some(
({ actualNode }) =>
(actualNode.getAttribute('kind') || '').toLowerCase() === 'captions'
);
}
// Undefined if there are no tracks - media may be decorative
return undefined;
// Undefined if there are no tracks - media may use another caption method
return hasCaptions ? false : undefined;
1 change: 0 additions & 1 deletion lib/checks/media/caption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"impact": "critical",
"messages": {
"pass": "The multimedia element has a captions track",
"fail": "The multimedia element does not have a captions track",
"incomplete": "A captions track for this element could not be found"
Copy link
Contributor

Choose a reason for hiding this comment

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

Could make this message more explicit about what to do to check whether an alternative exists

}
}
Expand Down
18 changes: 7 additions & 11 deletions lib/checks/media/description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
var tracks = axe.utils.querySelectorAll(virtualNode, 'track');
const tracks = axe.utils.querySelectorAll(virtualNode, 'track');
const hasDescriptions = tracks.some(
({ actualNode }) =>
(actualNode.getAttribute('kind') || '').toLowerCase() === 'descriptions'
);

if (tracks.length) {
// return false if any track has kind === 'description'
var out = !tracks.some(
({ actualNode }) =>
(actualNode.getAttribute('kind') || '').toLowerCase() === 'descriptions'
);
return out;
}
// Undefined if there are no tracks - media may be decorative
return undefined;
// Undefined if there are no tracks - media may have another description method
return hasDescriptions ? false : undefined;
1 change: 0 additions & 1 deletion lib/checks/media/description.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
"impact": "critical",
"messages": {
"pass": "The multimedia element has an audio description track",
"fail": "The multimedia element does not have an audio description track",
"incomplete": "An audio description track for this element could not be found"
}
}
Expand Down
12 changes: 6 additions & 6 deletions test/checks/media/caption.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,17 @@ describe('caption', function() {
assert.isUndefined(checks.caption.evaluate.apply(null, checkArgs));
});

it('should fail if there is no kind=captions attribute', function() {
it('should return undefined if there is no kind=captions attribute', function() {
var checkArgs = checkSetup(
'<audio><track kind=descriptions></audio>',
'audio'
);
assert.isTrue(checks.caption.evaluate.apply(null, checkArgs));
assert.isUndefined(checks.caption.evaluate.apply(null, checkArgs));
});

it('should fail if there is no kind attribute', function() {
it('should return undefined if there is no kind attribute', function() {
var checkArgs = checkSetup('<video><track></video>', 'video');
assert.isTrue(checks.description.evaluate.apply(null, checkArgs));
assert.isUndefined(checks.description.evaluate.apply(null, checkArgs));
});

it('should pass if there is a kind=captions attribute', function() {
Expand All @@ -36,12 +36,12 @@ describe('caption', function() {
'should get track from composed tree',
function() {
var node = document.createElement('div');
node.innerHTML = '<track kind=descriptions>';
node.innerHTML = '<track kind=captions>';
var shadow = node.attachShadow({ mode: 'open' });
shadow.innerHTML = '<audio><slot></slot></audio>';

var checkArgs = checkSetup(node, {}, 'audio');
assert.isTrue(checks.caption.evaluate.apply(null, checkArgs));
assert.isFalse(checks.caption.evaluate.apply(null, checkArgs));
}
);
});
8 changes: 4 additions & 4 deletions test/checks/media/description.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ describe('description', function() {
assert.isUndefined(checks.description.evaluate.apply(null, checkArgs));
});

it('should fail if there is no kind=captions attribute', function() {
it('should return undefined if there is no kind=captions attribute', function() {
var checkArgs = checkSetup('<video><track kind=captions></video>', 'video');
assert.isTrue(checks.description.evaluate.apply(null, checkArgs));
assert.isUndefined(checks.description.evaluate.apply(null, checkArgs));
});

it('should fail if there is no kind attribute', function() {
it('should return undefined if there is no kind attribute', function() {
var checkArgs = checkSetup('<video><track></video>', 'video');
assert.isTrue(checks.description.evaluate.apply(null, checkArgs));
assert.isUndefined(checks.description.evaluate.apply(null, checkArgs));
});

it('should pass if there is a kind=descriptions attribute', function() {
Expand Down
2 changes: 1 addition & 1 deletion test/integration/rules/video-caption/video-caption.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<video id="incomplete1"></video>
<video id="fail1"><track kind="descriptions"></video>
<video id="incomplete2"><track kind="descriptions"></video>
<video id="pass1"><track kind="captions"></video>
<video id="pass2"><track kind="descriptions"><track kind="captions"></video>
3 changes: 1 addition & 2 deletions test/integration/rules/video-caption/video-caption.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"description": "video-caption test",
"rule": "video-caption",
"incomplete": [["#incomplete1"]],
"violations": [["#fail1"]],
"incomplete": [["#incomplete1"], ["#incomplete2"]],
"passes": [["#pass1"], ["#pass2"]]
}
2 changes: 1 addition & 1 deletion test/integration/rules/video-description/video-description.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@

<video id="incomplete1"></video>
<video id="fail1"><track kind="captions"></video>
<video id="incomplete2"><track kind="captions"></video>
<video id="pass1"><track kind="descriptions"></video>
<video id="pass2"><track kind="descriptions"><track kind="captions"></video>
3 changes: 1 addition & 2 deletions test/integration/rules/video-description/video-description.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
{
"description": "video-description test",
"rule": "video-description",
"incomplete": [["#incomplete1"]],
"violations": [["#fail1"]],
"incomplete": [["#incomplete1"], ["#incomplete2"]],
"passes": [["#pass1"], ["#pass2"]]
}