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(target-size): do not crash for nodes with many overlapping widgets #4373

Merged
merged 9 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
15 changes: 7 additions & 8 deletions lib/checks/mobile/target-size-evaluate.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,6 @@ export default function targetSizeEvaluate(node, options, vNode) {
vNode,
nearbyElms
);
const hasOverflowingContent = () => {
this.data({ minSize, messageKey: 'contentOverflow' });
this.relatedNodes(mapActualNodes(overflowingContent));
return undefined;
};

// Target has overflowing content;
// and is either not fully obscured (so may not pass),
Expand All @@ -33,7 +28,9 @@ export default function targetSizeEvaluate(node, options, vNode) {
overflowingContent.length &&
(fullyObscuringElms.length || !hasMinimumSize(nodeRect))
) {
return hasOverflowingContent();
this.data({ minSize, messageKey: 'contentOverflow' });
this.relatedNodes(mapActualNodes(overflowingContent));
return undefined;
}

// Target is fully obscured and no overflowing content (which may not be obscured)
Expand Down Expand Up @@ -70,7 +67,9 @@ export default function targetSizeEvaluate(node, options, vNode) {
// Target is obscured, and insufficient space is left
if (!hasMinimumSize(largestInnerRect)) {
if (overflowingContent.length) {
return hasOverflowingContent();
this.data({ minSize, messageKey: 'contentOverflow' });
this.relatedNodes(mapActualNodes(overflowingContent));
return undefined;
}

const allTabbable = obscuredWidgets.every(isInTabOrder);
Expand Down Expand Up @@ -126,7 +125,7 @@ function getLargestUnobscuredArea(vNode, obscuredNodes) {
({ boundingClientRect: rect }) => rect
);
const unobscuredRects = splitRects(nodeRect, obscuringRects);
if (!unobscuredRects.length) {
if (unobscuredRects.length === 0) {
return null;
}

Expand Down
2 changes: 1 addition & 1 deletion lib/checks/mobile/target-size.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"contentOverflow": "Element size could not be accurately determined due to overflow content",
"partiallyObscured": "Element with negative tabindex has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
"partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?",
"tooManyRects": "Could not calculate calculate largest obscured rectangle as there are too many overlapping widgets"
"tooManyRects": "Could not get the target size because there are too many overlapping elements"
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion locales/_template.json
Original file line number Diff line number Diff line change
Expand Up @@ -886,7 +886,7 @@
"contentOverflow": "Element size could not be accurately determined due to overflow content",
"partiallyObscured": "Element with negative tabindex has insufficient size because it is partially obscured (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is this a target?",
"partiallyObscuredNonTabbable": "Target has insufficient size because it is partially obscured by a neighbor with negative tabindex (smallest space is ${data.width}px by ${data.height}px, should be at least ${data.minSize}px by ${data.minSize}px). Is the neighbor a target?",
"tooManyRects": "Could not calculate calculate largest obscured rectangle as there are too many overlapping widgets"
"tooManyRects": "Could not get the target size because there are too many overlapping elements"
}
},
"header-present": {
Expand Down
26 changes: 26 additions & 0 deletions test/integration/full/target-size/target-size.html
Original file line number Diff line number Diff line change
Expand Up @@ -651,6 +651,32 @@
</div>
</section>

<section id="incomplete">
straker marked this conversation as resolved.
Show resolved Hide resolved
<div id="incomplete-many-rects" role="tabpanel" tabindex="0">
<table id="tab-table"></table>
</div>
<script>
let html = '';
for (let i = 0; i < 100; i++) {
html += `
<tr>
<td><a href="#">Hello</a></td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td>Hello</td>
<td><button>view</button></td>
<td><button>download</button></td>
<td><button>expand</button></td>
</tr>
`;
}
document.querySelector('#tab-table').innerHTML = html;
</script>
</section>

<script src="/test/testutils.js"></script>
<script src="target-size.js"></script>
<script src="/test/integration/adapter.js"></script>
Expand Down
54 changes: 36 additions & 18 deletions test/integration/full/target-size/target-size.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,37 @@
describe('target-size test', function () {
describe('target-size test', () => {
'use strict';
var results;
let results;

before(function (done) {
axe.testUtils.awaitNestedLoad(function () {
before(done => {
axe.testUtils.awaitNestedLoad(() => {
// Add necessary markup for axe to recognize these as components:
document.querySelectorAll('section span').forEach(function (link) {
document.querySelectorAll('section span').forEach(link => {
link.setAttribute('role', 'link');
link.setAttribute('tabindex', '0');
});

var options = {
const options = {
runOnly: ['target-size'],
elementRef: true
};
axe.run('section', options, function (err, r) {
const context = {
include: 'section',
// ignore the incomplete table results
exclude: 'table tr'
};
axe.run(context, options, (err, r) => {
if (err) {
done(err);
}
results = r;
// Add some highlighting for visually identifying issues.
// There are too many test cases to just do this by selector.
results.violations[0] &&
results.violations[0].nodes.forEach(function (node) {
results.violations[0].nodes.forEach(node => {
node.element.className += ' violations';
});
results.passes[0] &&
results.passes[0].nodes.forEach(function (node) {
results.passes[0].nodes.forEach(node => {
node.element.className += ' passes';
});
console.log(results);
Expand All @@ -35,25 +40,38 @@ describe('target-size test', function () {
});
});

it('finds all passing nodes', function () {
var passResults = results.passes[0] ? results.passes[0].nodes : [];
var passedElms = document.querySelectorAll(
it('finds all passing nodes', () => {
const passResults = results.passes[0] ? results.passes[0].nodes : [];
const passedElms = document.querySelectorAll(
'section:not([hidden]) div:not([hidden]) .passed'
);
passResults.forEach(function (result) {
passResults.forEach(result => {
assert.include(passedElms, result.element);
});
assert.lengthOf(passResults, passedElms.length);
});

it('finds all failed nodes', function () {
var failResults = results.violations[0] ? results.violations[0].nodes : [];
var failedElms = document.querySelectorAll(
'section:not([hidden]) div:not([hidden]) .failed'
it('finds all failed nodes', () => {
const failResults = results.violations[0]
? results.violations[0].nodes
: [];
const failedElms = document.querySelectorAll(
'section:not([hidden]) div:not([hidden]) .failed'
);
failResults.forEach(function (result) {
failResults.forEach(result => {
assert.include(failedElms, result.element);
});
assert.lengthOf(failResults, failedElms.length);
});

it('incompletes with too many rects', () => {
const incompleteNodes = results.incomplete[0]
? results.incomplete[0].nodes
: [];
assert.lengthOf(incompleteNodes, 1);
assert.include(
incompleteNodes[0].element,
document.querySelector('#incomplete-many-rects')
);
});
});
Loading