From 1b6ab42f72b1ea0d2ed223c6fd63b9b1e54cfa9b Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Fri, 31 Jan 2020 09:50:52 -0700 Subject: [PATCH] fix(color-contrast): improve speed and accuracy of code blocks with syntax highlighting (#2003) * fix(color-contrast): improve speed and accuracy of code block with syntax highlighting * add integration test * figure out why firefox fails on opacity, but just in circle (works in localhost) * try again * fix tests * comment --- lib/commons/color/get-background-color.js | 10 +-- lib/commons/dom/get-element-stack.js | 49 +++++++++--- test/commons/color/get-foreground-color.js | 18 ++--- test/commons/dom/get-element-stack.js | 35 ++++++--- .../full/contrast/code-highlighting.html | 76 +++++++++++++++++++ .../full/contrast/code-highlighting.js | 52 +++++++++++++ 6 files changed, 205 insertions(+), 35 deletions(-) create mode 100644 test/integration/full/contrast/code-highlighting.html create mode 100644 test/integration/full/contrast/code-highlighting.js diff --git a/lib/commons/color/get-background-color.js b/lib/commons/color/get-background-color.js index 4fc036cee0..54919b607d 100644 --- a/lib/commons/color/get-background-color.js +++ b/lib/commons/color/get-background-color.js @@ -140,15 +140,15 @@ color.filteredRectStack = function filteredRectStack(elm) { */ color.getRectStack = function(elm) { const boundingStack = axe.commons.dom.getElementStack(elm); - let rects = Array.from(elm.getClientRects()); + + // Handle inline elements spanning multiple lines to be evaluated + const filteredArr = axe.commons.dom.getTextElementStack(elm); + // If the element does not have multiple rects, like for display:block, return a single stack - if (!rects || rects.length <= 1) { + if (!filteredArr || filteredArr.length <= 1) { return [boundingStack]; } - // Handle inline elements spanning multiple lines to be evaluated - let filteredArr = axe.commons.dom.getClientElementStack(elm); - if (filteredArr.some(stack => stack === undefined)) { // Can be happen when one or more of the rects sits outside the viewport return null; diff --git a/lib/commons/dom/get-element-stack.js b/lib/commons/dom/get-element-stack.js index f6c3a7bc45..02d8ba390e 100644 --- a/lib/commons/dom/get-element-stack.js +++ b/lib/commons/dom/get-element-stack.js @@ -337,7 +337,10 @@ function addNodeToGrid(grid, vNode) { for (let col = startCol; col <= endCol; col++) { grid.cells[row][col] = grid.cells[row][col] || []; - grid.cells[row][col].push(vNode); + + if (!grid.cells[row][col].includes(vNode)) { + grid.cells[row][col].push(vNode); + } } } }); @@ -452,10 +455,10 @@ function getRectStack(grid, rect, recursed = false) { // perform an AABB (axis-aligned bounding box) collision check for the // point inside the rect return ( - x < rectX + clientRect.width && - x > rectX && - y < rectY + clientRect.height && - y > rectY + x <= rectX + clientRect.width && + x >= rectX && + y <= rectY + clientRect.height && + y >= rectY ); }); }); @@ -506,13 +509,13 @@ dom.getElementStack = function(node) { }; /** - * Return all elements that are at the center of each client rect of the passed in node. - * @method getClientElementStack + * Return all elements that are at the center of each text client rect of the passed in node. + * @method getTextElementStack * @memberof axe.commons.dom * @param {Node} node * @return {Array} */ -dom.getClientElementStack = function(node) { +dom.getTextElementStack = function(node) { if (!axe._cache.get('gridCreated')) { createGrid(); axe._cache.set('gridCreated', true); @@ -525,7 +528,35 @@ dom.getClientElementStack = function(node) { return []; } - const clientRects = vNode.clientRects; + // for code blocks that use syntax highlighting, you can get a ton of client + // rects (See https://github.com/dequelabs/axe-core/issues/1985). they use + // a mixture of text nodes and other nodes (which will contain their own text + // nodes), but all we care about is checking the direct text nodes as the + // other nodes will have their own client rects checked. doing this speeds up + // color contrast significantly for large syntax highlighted code blocks + const clientRects = []; + Array.from(node.childNodes).forEach(elm => { + if ( + elm.nodeType === 3 && + axe.commons.text.sanitize(elm.textContent) !== '' + ) { + const range = document.createRange(); + range.selectNodeContents(elm); + const rects = range.getClientRects(); + + for (let i = 0; i < rects.length; i++) { + const rect = rects[i]; + + // filter out 0 width and height rects (newline characters) + // ie11 has newline characters return 0.00998, so we'll say if the + // line is < 1 it shouldn't be counted + if (rect.width >= 1 && rect.height >= 1) { + clientRects.push(rect); + } + } + } + }); + return clientRects.map(rect => { return getRectStack(grid, rect); }); diff --git a/test/commons/color/get-foreground-color.js b/test/commons/color/get-foreground-color.js index e11da14546..eed11fd9a4 100644 --- a/test/commons/color/get-foreground-color.js +++ b/test/commons/color/get-foreground-color.js @@ -14,8 +14,8 @@ describe('color.getForegroundColor', function() { it('should return the blended color if it has alpha set', function() { fixture.innerHTML = - '
' + - '
' + + '
' + 'This is my text' + '
'; @@ -31,8 +31,8 @@ describe('color.getForegroundColor', function() { it('should return the blended color if it has opacity set', function() { fixture.innerHTML = - '
' + - '
' + + '
' + 'This is my text' + '
'; @@ -49,8 +49,8 @@ describe('color.getForegroundColor', function() { it('should take into account parent opacity tree', function() { fixture.innerHTML = '
' + - '
' + - '
' + + '
' + + '
' + 'This is my text' + '
'; axe.testUtils.flatTreeSetup(fixture); @@ -66,9 +66,9 @@ describe('color.getForegroundColor', function() { it('should take into account entire parent opacity tree', function() { fixture.innerHTML = '
' + - '
' + - '
' + - '
' + + '
' + + '
' + + '
' + 'This is my text' + '
'; axe.testUtils.flatTreeSetup(fixture); diff --git a/test/commons/dom/get-element-stack.js b/test/commons/dom/get-element-stack.js index f1aad360ad..eded436b44 100644 --- a/test/commons/dom/get-element-stack.js +++ b/test/commons/dom/get-element-stack.js @@ -3,7 +3,7 @@ describe('dom.getElementStack', function() { var fixture = document.getElementById('fixture'); var getElementStack = axe.commons.dom.getElementStack; - var getClientElementStack = axe.commons.dom.getClientElementStack; + var getTextElementStack = axe.commons.dom.getTextElementStack; var isIE11 = axe.testUtils.isIE11; var shadowSupported = axe.testUtils.shadowSupport.v1; @@ -247,7 +247,7 @@ describe('dom.getElementStack', function() { it('should not return elements that do not fully cover the target', function() { fixture.innerHTML = '
' + - '
' + + '
' + '

Text oh heyyyy and here\'s
a link

' + '
'; axe.testUtils.flatTreeSetup(fixture); @@ -491,21 +491,32 @@ describe('dom.getElementStack', function() { }); }); - describe('dom.getClientElementStack', function() { - it('should return array of client rects', function() { + describe('dom.getTextElementStack', function() { + it('should return array of client text rects', function() { fixture.innerHTML = '
' + - '' + - 'Hello
World' + - '' + + '
' + + 'Hello
World' + + '
' + + '
'; + axe.testUtils.flatTreeSetup(fixture); + var target = fixture.querySelector('#target'); + var stacks = getTextElementStack(target).map(mapToIDs); + assert.deepEqual(stacks, [['target', '1', 'fixture']]); + }); + + it('should ignore newline characters', function() { + fixture.innerHTML = + '
' + + '
' + + 'Hello
\n' + + 'World' + + '
' + '
'; axe.testUtils.flatTreeSetup(fixture); var target = fixture.querySelector('#target'); - var stacks = getClientElementStack(target).map(mapToIDs); - assert.deepEqual(stacks, [ - ['2', 'target', '1', 'fixture'], - ['3', 'target', '1', 'fixture'] - ]); + var stacks = getTextElementStack(target).map(mapToIDs); + assert.deepEqual(stacks, [['target', '1', 'fixture']]); }); }); }); diff --git a/test/integration/full/contrast/code-highlighting.html b/test/integration/full/contrast/code-highlighting.html new file mode 100644 index 0000000000..ee94611280 --- /dev/null +++ b/test/integration/full/contrast/code-highlighting.html @@ -0,0 +1,76 @@ + + + + Test Page + + + + + + + + + + + + +
+ 
+        <!DOCTYPE html>
+          <html lang="en">
+            <head>
+              <title>Test Page</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>
+              <script src="/test/integration/no-ui-reporter.js"></script>
+              <style></style>
+            </head>
+
+            <body>
+              <pre>
+                <code>
+
+                </code>
+              </pre>
+              <script src="/test/testutils.js"></script>
+              <script src="code-highlighting.js"></script>
+              <script src="/test/integration/adapter.js"></script>
+            </body>
+          </html>
+
+
+ + + + + + diff --git a/test/integration/full/contrast/code-highlighting.js b/test/integration/full/contrast/code-highlighting.js new file mode 100644 index 0000000000..93ccf0a681 --- /dev/null +++ b/test/integration/full/contrast/code-highlighting.js @@ -0,0 +1,52 @@ +describe('color-contrast code highlighting test', function() { + 'use strict'; + + describe('violations', function() { + it('should find issues', function(done) { + axe.run( + '#fixture', + { runOnly: { type: 'rule', values: ['color-contrast'] } }, + function(err, results) { + assert.isNull(err); + assert.lengthOf(results.violations, 1); + assert.lengthOf(results.violations[0].nodes, 32); + done(); + } + ); + }); + }); + + describe('passes', function() { + it('should find passes', function(done) { + axe.run( + '#fixture', + { runOnly: { type: 'rule', values: ['color-contrast'] } }, + function(err, results) { + assert.isNull(err); + assert.lengthOf(results.passes, 1); + assert.lengthOf(results.passes[0].nodes, 27); + done(); + } + ); + }); + }); + + describe('incomplete', function() { + it('should find just the code block', function(done) { + axe.run( + '#fixture', + { runOnly: { type: 'rule', values: ['color-contrast'] } }, + function(err, results) { + assert.isNull(err); + assert.lengthOf(results.incomplete, 1); + assert.lengthOf(results.incomplete[0].nodes, 1); + assert.equal( + results.incomplete[0].nodes[0].html, + '' + ); + done(); + } + ); + }); + }); +});