From 293595066cea9a5c18d2ed7bf4de69cbfffe1ed1 Mon Sep 17 00:00:00 2001 From: Steven Lambert <2433219+straker@users.noreply.github.com> Date: Mon, 10 Oct 2022 15:42:00 -0600 Subject: [PATCH] fix(is-visible-on-screen): ignore elements hidden by overflow:hidden (#3676) * fix(is-visible-on-screen): ignore elements hidden by overflow:hidden * finalize * suggestions * suggestions * revert files from prettier not in scope * move test --- .../shared/svg-non-empty-title-evaluate.js | 5 +- lib/commons/dom/visibility-methods.js | 59 +++++++-- lib/commons/math/index.js | 1 + lib/commons/math/rects-overlap.js | 23 ++++ test/checks/color/color-contrast.js | 26 ++++ test/checks/shared/svg-non-empty-title.js | 33 +++-- test/commons/dom/is-visible-on-screen.js | 13 +- test/commons/dom/visibility-methods.js | 101 ++++++++------- test/commons/math/rects-overlap.js | 57 +++++++++ .../rules/color-contrast/color-contrast.html | 115 +++++++++++++----- .../rules/color-contrast/color-contrast.json | 12 +- 11 files changed, 340 insertions(+), 105 deletions(-) create mode 100644 lib/commons/math/rects-overlap.js create mode 100644 test/commons/math/rects-overlap.js diff --git a/lib/checks/shared/svg-non-empty-title-evaluate.js b/lib/checks/shared/svg-non-empty-title-evaluate.js index d537e37378..b614de2470 100644 --- a/lib/checks/shared/svg-non-empty-title-evaluate.js +++ b/lib/checks/shared/svg-non-empty-title-evaluate.js @@ -1,4 +1,4 @@ -import visibleVirtual from '../../commons/text/visible-virtual'; +import { subtreeText } from '../../commons/text'; function svgNonEmptyTitleEvaluate(node, options, virtualNode) { if (!virtualNode.children) { @@ -17,7 +17,8 @@ function svgNonEmptyTitleEvaluate(node, options, virtualNode) { } try { - if (visibleVirtual(titleNode) === '') { + const titleText = subtreeText(titleNode, { includeHidden: true }).trim(); + if (titleText === '') { this.data({ messageKey: 'emptyTitle' }); diff --git a/lib/commons/dom/visibility-methods.js b/lib/commons/dom/visibility-methods.js index db8b3ded77..9d262348a0 100644 --- a/lib/commons/dom/visibility-methods.js +++ b/lib/commons/dom/visibility-methods.js @@ -3,8 +3,10 @@ import { closest, getRootNode, querySelectorAll, - escapeSelector + escapeSelector, + memoize } from '../../core/utils'; +import rectsOverlap from '../math/rects-overlap'; const clipRegex = /rect\s*\(([0-9]+)px,?\s*([0-9]+)px,?\s*([0-9]+)px,?\s*([0-9]+)px\s*\)/; @@ -102,21 +104,58 @@ export function scrollHidden(vNode) { } /** - * Determine if an element is hidden by using overflow: hidden and dimensions + * Determine if an element is hidden by using overflow: hidden. * @param {VirtualNode} vNode * @return {Boolean} */ -export function overflowHidden(vNode) { - const elHeight = parseInt(vNode.getComputedStylePropertyValue('height')); - const elWidth = parseInt(vNode.getComputedStylePropertyValue('width')); +export function overflowHidden(vNode, { isAncestor } = {}) { + // an ancestor that is hidden outside an overflow + // does not mean that a descendant is also hidden + // since the descendant can reposition itself to be + // in view of the overflow:hidden ancestor + if (isAncestor) { + return false; + } - return ( - vNode.getComputedStylePropertyValue('position') === 'absolute' && - (elHeight < 2 || elWidth < 2) && - vNode.getComputedStylePropertyValue('overflow') === 'hidden' - ); + const rect = vNode.boundingClientRect; + const nodes = getOverflowHiddenAncestors(vNode); + + if (!nodes.length) { + return false; + } + + return nodes.some(node => { + const nodeRect = node.boundingClientRect; + + if (nodeRect.width < 2 || nodeRect.height < 2) { + return true; + } + + return !rectsOverlap(rect, nodeRect); + }); } +/** + * Get all ancestor nodes (including the passed in node) that have overflow:hidden + */ +const getOverflowHiddenAncestors = memoize( + function getOverflowHiddenAncestorsMemoized(vNode) { + const ancestors = []; + + if (!vNode) { + return ancestors; + } + + const overflow = vNode.getComputedStylePropertyValue('overflow'); + + if (overflow === 'hidden') { + ancestors.push(vNode); + } + + return ancestors.concat(getOverflowHiddenAncestors(vNode.parent)); + } +); + /** * Determines if an element is hidden with a clip or clip-path technique * @param {VirtualNode} vNode diff --git a/lib/commons/math/index.js b/lib/commons/math/index.js index 82b296f49b..91b9993ac1 100644 --- a/lib/commons/math/index.js +++ b/lib/commons/math/index.js @@ -1,3 +1,4 @@ export { default as getOffset } from './get-offset'; export { default as hasVisualOverlap } from './has-visual-overlap'; +export { default as rectsOverlap } from './rects-overlap'; export { default as splitRects } from './split-rects'; diff --git a/lib/commons/math/rects-overlap.js b/lib/commons/math/rects-overlap.js new file mode 100644 index 0000000000..50b402d340 --- /dev/null +++ b/lib/commons/math/rects-overlap.js @@ -0,0 +1,23 @@ +/** + * Determine if two rectangles touch. + * @method rectsOverlap + * @memberof axe.commons.math + * @param {Rect} rect1 + * @param {Rect} rect2 + * @returns {Boolean} + */ +export default function rectsOverlap(rect1, rect2) { + // perform an AABB (axis-aligned bounding box) check. + // account for differences in how browsers handle floating + // point precision of bounding rects + // @see https://developer.mozilla.org/en-US/docs/Games/Techniques/2D_collision_detection + + /* eslint-disable no-bitwise */ + return ( + (rect1.left | 0) < (rect2.right | 0) && + (rect1.right | 0) > (rect2.left | 0) && + (rect1.top | 0) < (rect2.bottom | 0) && + (rect1.bottom | 0) > (rect2.top | 0) + ); + /* eslint-enable no-bitwise */ +} diff --git a/test/checks/color/color-contrast.js b/test/checks/color/color-contrast.js index ba49188e37..e1edfd4f7e 100644 --- a/test/checks/color/color-contrast.js +++ b/test/checks/color/color-contrast.js @@ -399,6 +399,32 @@ describe('color-contrast', function () { }); }); + it('passes for element outside overflow:hidden', function () { + var params = checkSetup(` + +
+
+
hello
+
goodbye
+
+
+ `); + assert.isTrue(contrastEvaluate.apply(checkContext, params)); + }); + describe('with pseudo elements', function () { it('should return undefined if :before pseudo element has a background color', function () { var params = checkSetup( diff --git a/test/checks/shared/svg-non-empty-title.js b/test/checks/shared/svg-non-empty-title.js index f4c11fae2a..4ed56ac211 100644 --- a/test/checks/shared/svg-non-empty-title.js +++ b/test/checks/shared/svg-non-empty-title.js @@ -1,4 +1,4 @@ -describe('svg-non-empty-title tests', function() { +describe('svg-non-empty-title tests', function () { 'use strict'; var fixture = document.getElementById('fixture'); @@ -6,38 +6,45 @@ describe('svg-non-empty-title tests', function() { var checkSetup = axe.testUtils.checkSetup; var checkEvaluate = axe.testUtils.getCheckEvaluate('svg-non-empty-title'); - afterEach(function() { + afterEach(function () { fixture.innerHTML = ''; checkContext.reset(); }); - it('returns true if the element has a `title` child', function() { + it('returns true if the element has a `title` child', function () { var checkArgs = checkSetup( 'Time II: Party' ); assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); }); - it('returns true if the `title` child has text nested in another element', function() { + it('returns true if the `title` child has text nested in another element', function () { var checkArgs = checkSetup( 'Time II: Party' ); assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); }); - it('returns false if the element has no `title` child', function() { + it('returns true if the element has a `title` child with `display:none`', function () { + var checkArgs = checkSetup( + 'Time II: Party' + ); + assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); + }); + + it('returns false if the element has no `title` child', function () { var checkArgs = checkSetup(''); assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); assert.equal(checkContext._data.messageKey, 'noTitle'); }); - it('returns false if the `title` child is empty', function() { + it('returns false if the `title` child is empty', function () { var checkArgs = checkSetup(''); assert.isFalse(checkEvaluate.apply(checkContext, checkArgs)); assert.equal(checkContext._data.messageKey, 'emptyTitle'); }); - it('returns false if the `title` is a grandchild', function() { + it('returns false if the `title` is a grandchild', function () { var checkArgs = checkSetup( 'Time II: Party' ); @@ -45,7 +52,7 @@ describe('svg-non-empty-title tests', function() { assert.equal(checkContext._data.messageKey, 'noTitle'); }); - it('returns false if the `title` child has only whitespace', function() { + it('returns false if the `title` child has only whitespace', function () { var checkArgs = checkSetup( ' \t\r\n ' ); @@ -53,7 +60,7 @@ describe('svg-non-empty-title tests', function() { assert.equal(checkContext._data.messageKey, 'emptyTitle'); }); - it('returns false if there are multiple titles, and the first is empty', function() { + it('returns false if there are multiple titles, and the first is empty', function () { var checkArgs = checkSetup( 'Time II: Party' ); @@ -61,8 +68,8 @@ describe('svg-non-empty-title tests', function() { assert.equal(checkContext._data.messageKey, 'emptyTitle'); }); - describe('Serial Virtual Node', function() { - it('returns true if the element has a `title` child', function() { + describe('Serial Virtual Node', function () { + it('returns true if the element has a `title` child', function () { var serialNode = new axe.SerialVirtualNode({ nodeName: 'svg' }); @@ -82,7 +89,7 @@ describe('svg-non-empty-title tests', function() { assert.isTrue(checkEvaluate.apply(checkContext, checkArgs)); }); - it('returns false if the element has no `title` child', function() { + it('returns false if the element has no `title` child', function () { var serialNode = new axe.SerialVirtualNode({ nodeName: 'svg' }); @@ -93,7 +100,7 @@ describe('svg-non-empty-title tests', function() { assert.equal(checkContext._data.messageKey, 'noTitle'); }); - it('returns undefined if the element has empty children', function() { + it('returns undefined if the element has empty children', function () { var serialNode = new axe.SerialVirtualNode({ nodeName: 'svg' }); diff --git a/test/commons/dom/is-visible-on-screen.js b/test/commons/dom/is-visible-on-screen.js index 12b3f29ea3..99437949f7 100644 --- a/test/commons/dom/is-visible-on-screen.js +++ b/test/commons/dom/is-visible-on-screen.js @@ -381,9 +381,18 @@ describe('dom.isVisibleOnScreen', function () { assert.isFalse(isVisibleOnScreen(el.actualNode)); } ); - it('should return false if element is visually hidden using position absolute, overflow hidden, and a very small height', function () { + + it('should return false for screen reader only technique', function () { + var vNode = queryFixture( + '
Visually Hidden
' + ); + + assert.isFalse(isVisibleOnScreen(vNode)); + }); + + it('should return false for element outside "overflow:hidden"', function () { var vNode = queryFixture( - '
StickySticky
' + '
Visually Hidden
' ); assert.isFalse(isVisibleOnScreen(vNode)); diff --git a/test/commons/dom/visibility-methods.js b/test/commons/dom/visibility-methods.js index 3d9444a2c1..655c045aae 100644 --- a/test/commons/dom/visibility-methods.js +++ b/test/commons/dom/visibility-methods.js @@ -204,87 +204,100 @@ describe('dom.visibility-methods', () => { }); }); - describe('overflowHidden', () => { - it('should return true for element with "overflow:hidden" and "width:0`', () => { - const vNode = queryFixture( - '
overflow hidden
' + describe('overflowHidden', function () { + it('should return true for element with "overflow:hidden" and small width', function () { + var vNode = queryFixture( + '
Hello world
' ); assert.isTrue(overflowHidden(vNode)); }); - it('should return true for element with "overflow:hidden" and "height:0`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return true for element with "overflow:hidden" and small height', function () { + var vNode = queryFixture( + '
Hello world
' ); assert.isTrue(overflowHidden(vNode)); }); - it('should return true for element with "overflow:hidden" and "width:1`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return true for parent with "overflow: hidden" and element outside parent rect', function () { + var vNode = queryFixture( + '
' + + '
Hello world
' + + '
' ); assert.isTrue(overflowHidden(vNode)); }); - it('should return true for element with "overflow:hidden" and "height:1`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return true for ancestor with "overflow: hidden" and element outside ancestor rect', function () { + var vNode = queryFixture( + '
' + + '
' + + '
' + + '
Hello world
' + + '
' + + '
' + + '
' ); assert.isTrue(overflowHidden(vNode)); }); - it('should return true for element with "overflow:hidden" and width > 1', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return true for multiple ancestors with "overflow: hidden" and element outside one ancestor rect', function () { + var vNode = queryFixture( + '
' + + '
' + + '
' + + '
Hello world
' + + '
' + + '
' + + '
' ); - assert.isFalse(overflowHidden(vNode)); - }); - - it('should return true for element with "overflow:hidden" and height > 1', () => { - const vNode = queryFixture( - '
overflow hidden
' - ); - assert.isFalse(overflowHidden(vNode)); + assert.isTrue(overflowHidden(vNode)); }); - it('should return false for element with "position:fixed`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return true for element barely inside "overflow: hidden" parent using floating point', () => { + var vNode = queryFixture( + '
' + + '
Hello world
' + + '
' ); - assert.isFalse(overflowHidden(vNode)); + assert.isTrue(overflowHidden(vNode)); }); - it('should return false for element with "position:relative`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return false for element with "overflow:hidden" and width larger than 1px', function () { + var vNode = queryFixture( + '
Hello world
' ); assert.isFalse(overflowHidden(vNode)); }); - it('should return false for element without position', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return false for element with "overflow:hidden" and height larger than 1px', function () { + var vNode = queryFixture( + '
Hello world
' ); assert.isFalse(overflowHidden(vNode)); }); - it('should return false for element with "overflow:visible`', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return false for element with just "overflow:hidden"', function () { + var vNode = queryFixture( + '
Hello world
' ); assert.isFalse(overflowHidden(vNode)); }); - it('should return false for element with "overflow:auto`', () => { - const vNode = queryFixture( - '
overflow hidden
' - ); + it('should return false for element without "overflow:hidden"', function () { + var vNode = queryFixture('
Hello world
'); assert.isFalse(overflowHidden(vNode)); }); - it('should return false for element without overflow', () => { - const vNode = queryFixture( - '
overflow hidden
' + it('should return false for multiple ancestors with "overflow: hidden" and element is inside all ancestor rects', function () { + var vNode = queryFixture( + '
' + + '
' + + '
' + + '
Hello world
' + + '
' + + '
' + + '
' ); assert.isFalse(overflowHidden(vNode)); }); diff --git a/test/commons/math/rects-overlap.js b/test/commons/math/rects-overlap.js new file mode 100644 index 0000000000..1b3ef639c1 --- /dev/null +++ b/test/commons/math/rects-overlap.js @@ -0,0 +1,57 @@ +describe('math.rects-overlap', function () { + const rectsOverlap = axe.commons.math.rectsOverlap; + + let rectA, rectB; + beforeEach(() => { + rectA = { + left: 0, + right: 10, + top: 0, + bottom: 10 + }; + rectB = { + left: 5, + right: 20, + top: 0, + bottom: 5 + }; + }); + + it('returns true when rects overlap', () => { + assert.isTrue(rectsOverlap(rectA, rectB)); + }); + + it('should not matter on order', () => { + assert.isTrue(rectsOverlap(rectB, rectA)); + }); + + it('returns false when rects do not overlap (horizontally)', () => { + rectA.left = 25; + rectA.right = 40; + + assert.isFalse(rectsOverlap(rectA, rectB)); + }); + + it('returns false when rects do not overlap (vertically)', () => { + rectA.top = 15; + rectA.bottom = 25; + + assert.isFalse(rectsOverlap(rectA, rectB)); + }); + + it('returns false when rects are next to one another', () => { + rectA.left = 20; + rectA.right = 25; + + assert.isFalse(rectsOverlap(rectA, rectB)); + }); + + it('returns false when rects barely overlap due to floating point', () => { + rectA.left = 20.5; + rectA.right = 25.9; + rectB.left = 10.9; + rectB.right = 20.9; + + assert.isFalse(rectsOverlap(rectA, rectB)); + }); +}); diff --git a/test/integration/rules/color-contrast/color-contrast.html b/test/integration/rules/color-contrast/color-contrast.html index a1e2ed0997..5728c9dfbe 100644 --- a/test/integration/rules/color-contrast/color-contrast.html +++ b/test/integration/rules/color-contrast/color-contrast.html @@ -1,17 +1,22 @@
This is a pass.
-
+
This is a pass.
This is a pass.
-
+
-
+
This is a fail.But this is a pass.
-
+
Pass.
-
-
-
+
This is a fail.
-
+
This is a fail.
@@ -65,7 +67,7 @@
Hello world
@@ -99,7 +101,13 @@
text
@@ -122,44 +130,63 @@
Background image
-
+
Outside parent
-
-
+
+
Background overlap
Element overlap
-
+
-
-
Hi
+
+
Hi
Hi
-
-
+
+
₠ ₡ ₢ ₣
@@ -256,7 +283,13 @@

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Sed et sollicitudin @@ -273,3 +306,21 @@ felis ante non libero.

+ +
+ this is a span with some text inside of it + this is a span with some text inside of it + this is a span with some text inside of it + this is a span with some text inside of it +
diff --git a/test/integration/rules/color-contrast/color-contrast.json b/test/integration/rules/color-contrast/color-contrast.json index c661e1881c..a79732129f 100644 --- a/test/integration/rules/color-contrast/color-contrast.json +++ b/test/integration/rules/color-contrast/color-contrast.json @@ -1,7 +1,14 @@ { "description": "color-contrast test", "rule": "color-contrast", - "violations": [["#fail1"], ["#fail2"], ["#fail3"], ["#fail6"], ["#fail7"]], + "violations": [ + ["#fail1"], + ["#fail2"], + ["#fail3"], + ["#fail6"], + ["#fail7"], + ["#fail8"] + ], "passes": [ ["#pass1"], ["#pass2"], @@ -34,6 +41,7 @@ ["#canttell17"], ["#canttell18"], ["#canttell19"], - ["#canttell20"] + ["#canttell20"], + ["#canttell21"] ] }