diff --git a/lib/checks/tables/td-has-header.js b/lib/checks/tables/td-has-header.js index 0de8785296..f1bfda68a5 100644 --- a/lib/checks/tables/td-has-header.js +++ b/lib/checks/tables/td-has-header.js @@ -1,6 +1,7 @@ -var tableUtils = axe.commons.table; -var badCells = []; -var cells = tableUtils.getAllCells(node); +const tableUtils = axe.commons.table; +const badCells = []; +const cells = tableUtils.getAllCells(node); +const tableGrid = tableUtils.toGrid(node); cells.forEach(cell => { // For each non-empty data cell that doesn't have an aria label @@ -10,7 +11,7 @@ cells.forEach(cell => { !axe.commons.aria.label(cell) ) { // Check if it has any headers - const hasHeaders = tableUtils.getHeaders(cell).some(header => { + const hasHeaders = tableUtils.getHeaders(cell, tableGrid).some(header => { return header !== null && !!axe.commons.dom.hasContent(header); }); diff --git a/lib/commons/table/get-cell-position.js b/lib/commons/table/get-cell-position.js index 4e6bf17a79..7d06ea389e 100644 --- a/lib/commons/table/get-cell-position.js +++ b/lib/commons/table/get-cell-position.js @@ -8,7 +8,7 @@ * @param {HTMLTableCellElement} cell The table cell of which to get the position * @return {Object} Object with `x` and `y` properties of the coordinates */ -table.getCellPosition = function(cell, tableGrid) { +table.getCellPosition = axe.utils.memoize(function(cell, tableGrid) { var rowIndex, index; if (!tableGrid) { tableGrid = table.toGrid(dom.findUp(cell, 'table')); @@ -25,4 +25,4 @@ table.getCellPosition = function(cell, tableGrid) { } } } -}; +}); diff --git a/lib/commons/table/get-headers.js b/lib/commons/table/get-headers.js index 56b6f11983..9aa4e93261 100644 --- a/lib/commons/table/get-headers.js +++ b/lib/commons/table/get-headers.js @@ -1,29 +1,73 @@ /* global table */ +/** + * Loop through the table grid looking for headers and caching the result. + * @param {String} headerType The type of header to look for ("row" or "col") + * @param {Object} position The position of the cell to start looking + * @param {Array} tablegrid A matrix of the table obtained using axe.commons.table.toGrid + * @return {Array} Array of HTMLTableCellElements that are headers + */ +function traverseForHeaders(headerType, position, tableGrid) { + const property = headerType === 'row' ? '_rowHeaders' : '_colHeaders'; + const predicate = + headerType === 'row' ? table.isRowHeader : table.isColumnHeader; + const rowEnd = headerType === 'row' ? position.y : 0; + const colEnd = headerType === 'row' ? 0 : position.x; + + let headers; + const cells = []; + for (let row = position.y; row >= rowEnd && !headers; row--) { + for (let col = position.x; col >= colEnd; col--) { + const cell = tableGrid[row] ? tableGrid[row][col] : undefined; + + if (!cell) { + continue; + } + + // stop traversing once we've found a cache + const vNode = axe.utils.getNodeFromTree(cell); + if (vNode[property]) { + headers = vNode[property]; + break; + } + + cells.push(cell); + } + } + + // need to check that the cells we've traversed are headers + headers = (headers || []).concat(cells.filter(predicate)); + + // cache results + cells.forEach(tableCell => { + const vNode = axe.utils.getNodeFromTree(tableCell); + vNode[property] = headers; + }); + + return headers; +} + /** * Get any associated table headers for a `HTMLTableCellElement` * @method getHeaders * @memberof axe.commons.table * @instance * @param {HTMLTableCellElement} cell The cell of which to get headers + * @param {Array} [tablegrid] A matrix of the table obtained using axe.commons.table.toGrid * @return {Array} Array of headers associated to the table cell */ -table.getHeaders = function(cell) { +table.getHeaders = function(cell, tableGrid) { if (cell.hasAttribute('headers')) { return commons.dom.idrefs(cell, 'headers'); } - - var tableGrid = commons.table.toGrid(commons.dom.findUp(cell, 'table')); - var position = commons.table.getCellPosition(cell, tableGrid); + if (!tableGrid) { + tableGrid = commons.table.toGrid(commons.dom.findUp(cell, 'table')); + } + const position = commons.table.getCellPosition(cell, tableGrid); // TODO: RTL text - var rowHeaders = table - .traverse('left', position, tableGrid) - .filter(cell => table.isRowHeader(cell)); - - var colHeaders = table - .traverse('up', position, tableGrid) - .filter(cell => table.isColumnHeader(cell)); + const rowHeaders = traverseForHeaders('row', position, tableGrid); + const colHeaders = traverseForHeaders('col', position, tableGrid); return [].concat(rowHeaders, colHeaders).reverse(); }; diff --git a/lib/commons/table/get-scope.js b/lib/commons/table/get-scope.js index 51b006c051..f088854b0a 100644 --- a/lib/commons/table/get-scope.js +++ b/lib/commons/table/get-scope.js @@ -29,7 +29,7 @@ table.getScope = function(cell) { return false; } var tableGrid = table.toGrid(dom.findUp(cell, 'table')); - var pos = table.getCellPosition(cell); + var pos = table.getCellPosition(cell, tableGrid); // The element is in a row with all th elements, that makes it a column header var headerRow = tableGrid[pos.y].reduce((headerRow, cell) => { diff --git a/lib/commons/table/to-grid.js b/lib/commons/table/to-grid.js index 640bdcc4b3..f20b127792 100644 --- a/lib/commons/table/to-grid.js +++ b/lib/commons/table/to-grid.js @@ -8,7 +8,7 @@ * @param {HTMLTableElement} node The table to convert * @return {Array} Array of HTMLTableCellElements */ -table.toGrid = function(node) { +table.toGrid = axe.utils.memoize(function(node) { var table = []; var rows = node.rows; for (var i = 0, rowLength = rows.length; i < rowLength; i++) { @@ -32,7 +32,7 @@ table.toGrid = function(node) { } return table; -}; +}); // This was the old name table.toArray = table.toGrid; diff --git a/lib/core/imports/index.js b/lib/core/imports/index.js index 3eea57cff2..2c9e29d477 100644 --- a/lib/core/imports/index.js +++ b/lib/core/imports/index.js @@ -46,5 +46,6 @@ axe.imports = { axios: require('axios'), CssSelectorParser: require('css-selector-parser').CssSelectorParser, doT: require('@deque/dot'), - emojiRegexText: require('emoji-regex') + emojiRegexText: require('emoji-regex'), + memoize: require('memoizee') }; diff --git a/lib/core/public/run-rules.js b/lib/core/public/run-rules.js index 32be4c952b..14cd826669 100644 --- a/lib/core/public/run-rules.js +++ b/lib/core/public/run-rules.js @@ -3,6 +3,7 @@ // Clean up after resolve / reject function cleanup() { + axe._memoizedFns.forEach(fn => fn.clear()); axe._cache.clear(); axe._tree = undefined; axe._selectorData = undefined; diff --git a/lib/core/utils/memoize.js b/lib/core/utils/memoize.js new file mode 100644 index 0000000000..4dd52ebe63 --- /dev/null +++ b/lib/core/utils/memoize.js @@ -0,0 +1,17 @@ +/** + * Memoize a function. + * @method memoize + * @memberof axe.utils + * @param {Function} fn Function to memoize + * @return {Function} + */ +axe._memoizedFns = []; +axe.utils.memoize = function(fn) { + // keep track of each function that is memoized so it can be cleared at + // the end of a run. each memoized function has its own cache, so there is + // no method to clear all memoized caches. instead, we have to clear each + // individual memoized function ourselves. + const memoized = axe.imports.memoize(fn); + axe._memoizedFns.push(memoized); + return memoized; +}; diff --git a/package.json b/package.json index cc39948cd5..2979bad372 100644 --- a/package.json +++ b/package.json @@ -115,6 +115,7 @@ "lint-staged": "^9.2.1", "make-dir": "^3.0.0", "markdown-table": "^1.1.2", + "memoizee": "^0.4.14", "minami": "^1.2.3", "mkdirp": "^0.5.1", "mocha": "^6.1.2", diff --git a/test/commons/table/get-headers.js b/test/commons/table/get-headers.js index 26829b8a57..2b9f30af02 100644 --- a/test/commons/table/get-headers.js +++ b/test/commons/table/get-headers.js @@ -5,10 +5,14 @@ describe('table.getHeaders', function() { } var fixture = $id('fixture'); + var origToGrid = axe.commons.table.toGrid; + var origCellPosition = axe.commons.table.getCellPosition; afterEach(function() { fixture.innerHTML = ''; axe._tree = undefined; + axe.commons.table.toGrid = origToGrid; + axe.commons.table.getCellPosition = origCellPosition; }); it('should work with scope=auto', function() { @@ -136,4 +140,38 @@ describe('table.getHeaders', function() { axe.testUtils.flatTreeSetup(fixture.firstChild); assert.deepEqual(axe.commons.table.getHeaders(target), []); }); + + it('should not call toGrid if a tableGrid is passed in', function() { + fixture.innerHTML = + '' + + '' + + '' + + '
12
2ok
'; + + var target = $id('target'); + var table = $id('table'); + + axe.testUtils.flatTreeSetup(fixture.firstChild); + var tableGrid = axe.commons.table.toGrid(table); + + // getCellPosition will eventually call into toGrid when checking for + // scope but we're only interested if the toGrid call was called before + // cellPosition + var called = false; + axe.commons.table.toGrid = function(table) { + called = true; + return origToGrid(table); + }; + axe.commons.table.getCellPosition = function(cell, tableGrid) { + axe.commons.table.toGrid = origToGrid; + return origCellPosition(cell, tableGrid); + }; + + axe.commons.table.getHeaders(target, tableGrid); + assert.isFalse(called); + assert.deepEqual(axe.commons.table.getHeaders(target), [ + $id('t1'), + $id('t2') + ]); + }); }); diff --git a/test/core/public/run-rules.js b/test/core/public/run-rules.js index a3c2a766eb..a7ba070e5d 100644 --- a/test/core/public/run-rules.js +++ b/test/core/public/run-rules.js @@ -48,9 +48,11 @@ describe('runRules', function() { } var fixture = document.getElementById('fixture'); + var memoizedFns; var isNotCalled; beforeEach(function() { + memoizedFns = axe._memoizedFns.slice(); isNotCalled = function(err) { throw err || new Error('Reject should not be called'); }; @@ -60,6 +62,7 @@ describe('runRules', function() { fixture.innerHTML = ''; axe._audit = null; axe._tree = undefined; + axe._memoizedFns = memoizedFns; }); it('should work', function(done) { @@ -914,4 +917,46 @@ describe('runRules', function() { }, 100); }); }); + + it('should clear the memoized cache for each function', function(done) { + axe._load({ + rules: [ + { + id: 'html', + selector: 'html', + any: ['html'] + } + ], + checks: [ + { + id: 'html', + evaluate: function() { + return true; + } + } + ], + messages: {} + }); + + runRules( + document, + {}, + function resolve(out, cleanup) { + var called = false; + axe._memoizedFns = [ + { + clear: function() { + called = true; + } + } + ]; + + cleanup(); + assert.isTrue(called); + + done(); + }, + isNotCalled + ); + }); }); diff --git a/test/core/utils/memoize.js b/test/core/utils/memoize.js new file mode 100644 index 0000000000..162c5e40d2 --- /dev/null +++ b/test/core/utils/memoize.js @@ -0,0 +1,32 @@ +describe('axe.utils.memoize', function() { + 'use strict'; + + var orig = axe.imports.memoize; + var memoizedFns; + + beforeEach(function() { + memoizedFns = axe._memoizedFns.slice(); + }); + + afterEach(function() { + axe.imports.memoize = orig; + axe._memoizedFns = memoizedFns; + }); + + it('should call imports.memoize', function() { + var called = false; + axe.imports.memoize = function() { + called = true; + }; + + axe.utils.memoize(function() {}); + assert.isTrue(called); + }); + + it('should add the function to axe._memoizedFns', function() { + axe._memoizedFns.length = 0; + + axe.utils.memoize(function myFn() {}); + assert.equal(axe._memoizedFns.length, 1); + }); +});