-
Notifications
You must be signed in to change notification settings - Fork 1
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(target-size): do not crash for nodes with many overlapping widget…
…s (#4373) Decided that instead of returning `null`, `splitRects` should return an empty array to signify the bail out state. `splitRects` returns the original node if there is no overlap, so an empty array allows us to still signify the desired state as well as still allow all the code that used it to treat it as an array. Otherwise I would need to propagate a `null` check through 4 different functions in `target-offset-evaluate` (`getOffset`, `getTargetSize`, `getTargetRects`, etc.). In trying to catch the new state in `target-size-evaluate` it became difficult to figure out the logic of all the different things that needed to be checked to know what to return. I decided to refactor the entire function to make the flow easier by eliminating possibilities higher up so the if statements only needed to check a single condition rather than multiple conditions. The two key parts of the refactor was to move the overflow content check to the top. For overflowing content the return would always be undefined unless the target had sufficient size, in which case it would return true. This meant we can check those states first and not have to check for it again, which greatly simplifies all the logic of the if statements. In terms of the magic number for when to exit early, it was based on running performance tests against the code in #4359 and logging how long the inner loop took as `uniqueRects` size grew. Once the time got to ~2 seconds to complete I took that number then reduced it by a factor of 10x for slower machines. ``` uniqueRects: 4 time: 0.10000014305114746 uniqueRects: 7 time: 0 uniqueRects: 10 time: 0 uniqueRects: 13 time: 0 uniqueRects: 16 time: 0 uniqueRects: 19 time: 0.09999990463256836 uniqueRects: 22 time: 0 uniqueRects: 25 time: 0 uniqueRects: 34 time: 0.10000014305114746 uniqueRects: 43 time: 0.09999990463256836 uniqueRects: 55 time: 0.09999990463256836 uniqueRects: 75 time: 0 uniqueRects: 90 time: 0.09999990463256836 uniqueRects: 122 time: 0.09999990463256836 uniqueRects: 140 time: 0.10000014305114746 uniqueRects: 187 time: 0.20000004768371582 uniqueRects: 208 time: 0.09999990463256836 uniqueRects: 273 time: 0.10000014305114746 uniqueRects: 297 time: 0.8999998569488525 uniqueRects: 383 time: 0.2999999523162842 uniqueRects: 410 time: 0.20000004768371582 uniqueRects: 520 time: 0.2999999523162842 uniqueRects: 548 time: 0.2999999523162842 uniqueRects: 683 time: 0.2999999523162842 uniqueRects: 692 time: 0.5 uniqueRects: 720 time: 0.5999999046325684 uniqueRects: 775 time: 0.7000000476837158 uniqueRects: 872 time: 1.1999998092651367 uniqueRects: 1029 time: 0.5999999046325684 uniqueRects: 1267 time: 1 uniqueRects: 1610 time: 1.2000000476837158 uniqueRects: 2083 time: 1.3999998569488525 uniqueRects: 2089 time: 1.8999998569488525 uniqueRects: 2095 time: 1.9000000953674316 uniqueRects: 2101 time: 1.7000000476837158 uniqueRects: 2107 time: 1.6000001430511475 uniqueRects: 2113 time: 1.7999999523162842 uniqueRects: 2119 time: 1.6000001430511475 uniqueRects: 2125 time: 1.7000000476837158 uniqueRects: 2131 time: 1.6000001430511475 uniqueRects: 2164 time: 1.6999998092651367 uniqueRects: 2329 time: 1.7000000476837158 uniqueRects: 2365 time: 1.7000000476837158 uniqueRects: 2565 time: 2 uniqueRects: 2604 time: 2.0999999046325684 uniqueRects: 2840 time: 2.5 uniqueRects: 2882 time: 2.6000001430511475 uniqueRects: 3157 time: 2.5 uniqueRects: 3202 time: 2.5999999046325684 uniqueRects: 3519 time: 2.700000047683716 uniqueRects: 3567 time: 3.0999999046325684 uniqueRects: 3929 time: 3.4000000953674316 uniqueRects: 3980 time: 15.5 uniqueRects: 4390 time: 6.599999904632568 uniqueRects: 4442 time: 4.200000047683716 uniqueRects: 4901 time: 4.299999952316284 uniqueRects: 5534 time: 5.299999952316284 uniqueRects: 6366 time: 6.3999998569488525 uniqueRects: 7429 time: 7.8999998569488525 uniqueRects: 8762 time: 10.600000143051147 uniqueRects: 10407 time: 12.5 uniqueRects: 12409 time: 16.100000143051147 uniqueRects: 14816 time: 21.5 uniqueRects: 17677 time: 30.199999809265137 uniqueRects: 17686 time: 33.200000047683716 uniqueRects: 17695 time: 33.200000047683716 uniqueRects: 17704 time: 34.5 uniqueRects: 17713 time: 34.09999990463257 uniqueRects: 17722 time: 34.299999952316284 uniqueRects: 17731 time: 34.59999990463257 uniqueRects: 17740 time: 34.09999990463257 uniqueRects: 17749 time: 35.5 uniqueRects: 17806 time: 34.89999985694885 uniqueRects: 18319 time: 35.09999990463257 uniqueRects: 18379 time: 36.10000014305115 uniqueRects: 18951 time: 37.200000047683716 uniqueRects: 19014 time: 38.59999990463257 uniqueRects: 19646 time: 39.700000047683716 uniqueRects: 19712 time: 41.40000009536743 uniqueRects: 20407 time: 43.09999990463257 uniqueRects: 20476 time: 43.69999980926514 uniqueRects: 21237 time: 44.5 uniqueRects: 21309 time: 47.200000047683716 uniqueRects: 22139 time: 48.5 uniqueRects: 22214 time: 50.799999952316284 uniqueRects: 23116 time: 51.799999952316284 uniqueRects: 23192 time: 55.299999952316284 uniqueRects: 24167 time: 57 uniqueRects: 27536 time: 66.39999985694885 uniqueRects: 31476 time: 85.89999985694885 uniqueRects: 36043 time: 131.5 uniqueRects: 41300 time: 230.20000004768372 ``` Closes: #4359 Closes: #4360 --------- Co-authored-by: Wilco Fiers <WilcoFiers@users.noreply.github.com>
- Loading branch information
1 parent
0f8a9af
commit 1dbea83
Showing
9 changed files
with
203 additions
and
20 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
<!doctype html> | ||
<html lang="en"> | ||
<head> | ||
<title>Target-size too many rects Test</title> | ||
<link | ||
rel="stylesheet" | ||
type="text/css" | ||
href="/node_modules/mocha/mocha.css" | ||
/> | ||
<script src="/node_modules/mocha/mocha.js"></script> | ||
<script src="/node_modules/chai/chai.js"></script> | ||
<script src="/axe.js"></script> | ||
<script> | ||
mocha.setup({ | ||
timeout: 10000, | ||
ui: 'bdd' | ||
}); | ||
var assert = chai.assert; | ||
</script> | ||
</head> | ||
<body> | ||
<main id="mocha"></main> | ||
|
||
<section id="incomplete"> | ||
<div id="incomplete-many-rects" role="tabpanel" tabindex="0"> | ||
<table id="tab-table"></table> | ||
</div> | ||
</section> | ||
|
||
<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> | ||
<script src="/test/testutils.js"></script> | ||
<script src="too-many-rects.js"></script> | ||
<script src="/test/integration/adapter.js"></script> | ||
</body> | ||
</html> |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
describe('target-size too many rects test', () => { | ||
'use strict'; | ||
let results; | ||
|
||
before(done => { | ||
axe.testUtils.awaitNestedLoad(() => { | ||
const options = { | ||
runOnly: ['target-size'], | ||
elementRef: true | ||
}; | ||
const context = { | ||
// ignore the incomplete table results | ||
exclude: 'table tr' | ||
}; | ||
axe.run(context, options, (err, r) => { | ||
if (err) { | ||
done(err); | ||
} | ||
results = r; | ||
console.log(results); | ||
done(); | ||
}); | ||
}); | ||
}); | ||
|
||
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') | ||
); | ||
}); | ||
}); |