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

fix(object-alt): ignore unloaded objects #3680

Merged
merged 2 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions lib/rules/object-alt.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"id": "object-alt",
"selector": "object",
"matches": "no-explicit-name-required-matches",
"selector": "object[data]",
"matches": "object-is-loaded-matches",
"tags": [
"cat.text-alternatives",
"wcag2a",
Expand Down
22 changes: 22 additions & 0 deletions lib/rules/object-is-loaded-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
import noExplicitNameRequired from './no-explicit-name-required-matches';

export default (node, vNode) =>
[noExplicitNameRequired, objectHasLoaded].every(fn => fn(node, vNode));

/**
* Test if an object loaded content; assume yes if we can't prove otherwise
*
* @param {Element} node
* @param {VirtualNode} vNode
* @returns {boolean}
*/
function objectHasLoaded(node) {
if (!node?.ownerDocument?.createRange) {
return true; // Assume it did
}
// There's no ready
const range = node.ownerDocument.createRange();
range.setStart(node, 0);
range.setEnd(node, node.childNodes.length);
return range.getClientRects().length === 0;
}
5 changes: 4 additions & 1 deletion test/integration/full/isolated-env/isolated-env.html
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,10 @@ <h2>Ok</h2>
<circle cx="50" cy="50" r="40" fill="yellow"></circle>
</svg>
<div lang="en">English</div>
<object title="This object has text"></object>
<object
title="This object has text"
data="data:text/html,Object%20content"
></object>
<li role="presentation" aria-label="My Heading">Hello</li>
<div role="img" aria-label="blah"></div>
<div style="overflow-y: scroll; height: 5px">
Expand Down
89 changes: 73 additions & 16 deletions test/integration/rules/object-alt/object-alt.html
Original file line number Diff line number Diff line change
@@ -1,18 +1,75 @@
<object id="pass1" title="This object has text"></object>
<object id="pass2" aria-label="this object has text"></object>
<span id="label1">this object has text</span>
<object id="pass3" aria-labelledby="label1"></object>
<object id="pass4" role="presentation"></object>
<object id="pass5" role="none"></object>
<object
id="pass1"
title="This object has text"
data="data:text/html,Object%20content"
></object>
<object
id="pass2"
aria-label="this object has text"
data="data:text/html,Object%20content"
></object>
<span id="label1" data="data:text/html,Object%20content"
>this object has text</span
>
<object
id="pass3"
aria-labelledby="label1"
data="data:text/html,Object%20content"
></object>
<object
id="pass4"
role="presentation"
data="data:text/html,Object%20content"
></object>
<object id="pass5" role="none" data="data:text/html,Object%20content"></object>

<object id="violation1"></object>
<object id="violation2"><div></div></object>
<object id="violation3">This object has text.</object>
<object id="violation4" role="none" tabindex="0"></object>
<object id="violation5" role="presentation" tabindex="0"></object>
<object id="violation6" role="none" aria-live="assertive"></object>
<object id="violation7" role="presentation" aria-live="assertive"></object>
<object id="violation1" data="data:text/html,Object%20content"></object>
<object id="violation2" data="data:text/html,Object%20content">
<div></div>
</object>
<object id="violation3" data="data:text/html,Object%20content">
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ACT doesn't cover this, but this is still an issue.

This object has text.
</object>
<object
id="violation4"
role="none"
tabindex="0"
data="data:text/html,Object%20content"
></object>
<object
id="violation5"
role="presentation"
tabindex="0"
data="data:text/html,Object%20content"
></object>
<object
id="violation6"
role="none"
aria-live="assertive"
data="data:text/html,Object%20content"
></object>
<object
id="violation7"
role="presentation"
aria-live="assertive"
data="data:text/html,Object%20content"
></object>
<object
id="violation8"
role="img"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ACT Covers this in a different rule. We don't.

data="data:text/html,Object%20content"
></object>
<object
id="violation9"
role="separator"
tabindex="0"
data="data:text/html,Object%20content"
></object>

<object id="violation8" role="img"></object>
<object id="violation9" role="separator" tabindex="0"></object>
<object id="inapplicable1" role="separator"></object>
<object id="inapplicable1"><!-- no data attribute --></object>
<object id="inapplicable2" data="">Fallback content</object>
<object
id="inapplicable3"
role="separator"
data="data:text/html,Object%20content"
></object>
66 changes: 43 additions & 23 deletions test/integration/virtual-rules/object-alt.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
describe('object-alt virtual-rule', function() {
it('should pass for aria-label', function() {
describe('object-alt virtual-rule', function () {
const data = `data:text/html,Object%20content`;

it('is inapplicable when the object has no data attribute', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {}
});
assert.lengthOf(results.inapplicable, 1);
});

it('should pass for aria-label', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {
'aria-label': 'foobar'
'aria-label': 'foobar',
data
}
});

Expand All @@ -12,11 +23,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should incomplete for aria-labelledby', function() {
it('should incomplete for aria-labelledby', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {
'aria-labelledby': 'foobar'
'aria-labelledby': 'foobar',
data
}
});

Expand All @@ -25,11 +37,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 1);
});

it('should pass for title', function() {
it('should pass for title', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {
title: 'foobar'
title: 'foobar',
data
}
});

Expand All @@ -38,11 +51,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should pass for role=presentation', function() {
it('should pass for role=presentation', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {
role: 'presentation'
role: 'presentation',
data
}
});

Expand All @@ -51,11 +65,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should pass for role=none', function() {
it('should pass for role=none', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {
role: 'none'
role: 'none',
data
}
});

Expand All @@ -64,9 +79,10 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should fail for visible text content', function() {
it('should fail for visible text content', function () {
var node = new axe.SerialVirtualNode({
nodeName: 'object'
nodeName: 'object',
attributes: { data }
});
var child = new axe.SerialVirtualNode({
nodeName: '#text',
Expand All @@ -82,20 +98,21 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should fail when alt and children are missing', function() {
it('should fail when alt and children are missing', function () {
var results = axe.runVirtualRule('object-alt', {
nodeName: 'object',
attributes: {}
attributes: { data }
});

assert.lengthOf(results.passes, 0);
assert.lengthOf(results.violations, 1);
assert.lengthOf(results.incomplete, 0);
});

it('should fail children contain no visible text', function() {
it('should fail children contain no visible text', function () {
var node = new axe.SerialVirtualNode({
nodeName: 'object'
nodeName: 'object',
attributes: { data }
});
node.children = [];

Expand All @@ -106,11 +123,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should fail when alt contains only whitespace', function() {
it('should fail when alt contains only whitespace', function () {
var node = new axe.SerialVirtualNode({
nodeName: 'object',
attributes: {
alt: ' \t \n '
alt: ' \t \n ',
data
}
});
node.children = [];
Expand All @@ -122,11 +140,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should fail when aria-label is empty', function() {
it('should fail when aria-label is empty', function () {
var node = new axe.SerialVirtualNode({
nodeName: 'object',
attributes: {
'aria-label': ''
'aria-label': '',
data
}
});
node.children = [];
Expand All @@ -138,11 +157,12 @@ describe('object-alt virtual-rule', function() {
assert.lengthOf(results.incomplete, 0);
});

it('should fail when title is empty', function() {
it('should fail when title is empty', function () {
var node = new axe.SerialVirtualNode({
nodeName: 'object',
attributes: {
title: ''
title: '',
data
}
});
node.children = [];
Expand Down
56 changes: 56 additions & 0 deletions test/rule-matches/object-is-loaded-matches.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
describe('object-is-loaded-matches', () => {
let rule, fixture;
const data = `data:text/html,Object%20content`;

// Give enough time for the browser to load / decide it cannot load objects
async function delayedQueryFixture(html, delay = 50) {
fixture.innerHTML = html;
await new Promise(r => setTimeout(r, delay));
const tree = axe.setup();
return axe.utils.querySelectorAll(tree, '#target')[0];
}

before(() => {
fixture = document.querySelector('#fixture');
rule = axe.utils.getRule('object-alt');
});

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

it(`returns true objects with hidden fallback content`, async () => {
const vNode = await delayedQueryFixture(
`<object data="${data}" height="30" id="target">
Fallback content
</object>`
);
assert.isTrue(rule.matches(vNode.actualNode, vNode));
});

it(`returns false if the object shows any content`, async () => {
const vNode = await delayedQueryFixture(
`<object data="invalid" height="30" id="target">
Fallback content
</object>`
);
assert.isFalse(rule.matches(vNode.actualNode, vNode));
});

it(`returns true if the object shows no content`, async () => {
const vNode = await delayedQueryFixture(
`<object data="invalid" height="30" id="target"></object>`
);
// Ideally, this should be false, don't know it can be done
Copy link
Contributor

Choose a reason for hiding this comment

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

Very helpful comment, thanks.

assert.isTrue(rule.matches(vNode.actualNode, vNode));
});

it(`returns false if the object has a role that doesn't require a name`, async () => {
const vNode = await delayedQueryFixture(
`<object data="${data}" height="30" id="target" role="grid">
Fallback content
</object>`
);
assert.isFalse(rule.matches(vNode.actualNode, vNode));
});
});