fix(td-has-headers): greatly improve performance of td-has-headers ru…

…le (#1887)

* fix(td-has-headers): greatly improve performance of td-has-headers rule

* remove console.log

* add tests

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);
 		});
 

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) {
 			}
 		}
 	}
-};
+});

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<HTMLTableCellElement>} 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<HTMLTableCellElement>} 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();
 };

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) => {

lib/commons/table/to-grid.js

@@ -8,7 +8,7 @@
  * @param  {HTMLTableElement} node The table to convert
  * @return {Array<HTMLTableCellElement>} 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;

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')
 };

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;

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;
+};

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",

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 =
+			'<table id="table">' +
+			'<tr><td></td><th id="t1">1</th><th>2</th></tr>' +
+			'<tr><th id="t2">2</th><td id="target">ok</td><td></td></tr>' +
+			'</table>';
+
+		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')
+		]);
+	});
 });

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
+		);
+	});
 });

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);
+	});
+});