From bc4bf0918cb9e82ad017911905eb2b3f4a5e7fa4 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Aug 2023 09:31:08 -0600 Subject: [PATCH 1/9] fix(link-in-text-block): take into account ancestor inline element styles --- .../link-in-text-block-style-evaluate.js | 22 ++- test/checks/color/link-in-text-block-style.js | 138 +++++++++++------- 2 files changed, 97 insertions(+), 63 deletions(-) diff --git a/lib/checks/color/link-in-text-block-style-evaluate.js b/lib/checks/color/link-in-text-block-style-evaluate.js index e07929ea41..f51101493d 100644 --- a/lib/checks/color/link-in-text-block-style-evaluate.js +++ b/lib/checks/color/link-in-text-block-style-evaluate.js @@ -15,8 +15,10 @@ export default function linkInTextBlockStyleEvaluate(node) { return false; } - var parentBlock = getComposedParent(node); + let parentBlock = getComposedParent(node); + const inlineNodes = [node]; while (parentBlock && parentBlock.nodeType === 1 && !isBlock(parentBlock)) { + inlineNodes.push(parentBlock); parentBlock = getComposedParent(parentBlock); } @@ -26,18 +28,22 @@ export default function linkInTextBlockStyleEvaluate(node) { this.relatedNodes([parentBlock]); - if (elementIsDistinct(node, parentBlock)) { - return true; - } - if (hasPseudoContent(node)) { - this.data({ messageKey: 'pseudoContent' }); - return undefined; + for (const inlineNode of inlineNodes) { + if (elementIsDistinct(inlineNode, parentBlock)) { + return true; + } + + if (hasPseudoContent(inlineNode)) { + this.data({ messageKey: 'pseudoContent' }); + return undefined; + } } + return false; } function isBlock(elm) { - var display = window.getComputedStyle(elm).getPropertyValue('display'); + const display = window.getComputedStyle(elm).getPropertyValue('display'); return blockLike.indexOf(display) !== -1 || display.substr(0, 6) === 'table-'; } diff --git a/test/checks/color/link-in-text-block-style.js b/test/checks/color/link-in-text-block-style.js index cc8e3d812d..cede991c94 100644 --- a/test/checks/color/link-in-text-block-style.js +++ b/test/checks/color/link-in-text-block-style.js @@ -71,24 +71,71 @@ describe('link-in-text-block-style', () => { styleElm.innerHTML += selector + ' {\n' + cssLines + '\n}\n'; } - function getLinkElm(linkStyle) { + function getLinkElm(linkStyle, spanStyle = {}) { // Get a random id and build the style strings const linkId = 'linkid-' + Math.floor(Math.random() * 100000); const parId = 'parid-' + Math.floor(Math.random() * 100000); + const spanId = 'spanid-' + Math.floor(Math.random() * 100000); createStyleString('#' + linkId, linkStyle); createStyleString('#' + parId, {}); + createStyleString('#' + spanId, spanStyle); + + fixture.innerHTML += ` +

+ + link + +

+ `; - fixture.innerHTML += - '

Text ' + - 'link' + - '

'; axe.testUtils.flatTreeSetup(fixture); - return document.getElementById(linkId); + return { + parentElm: document.getElementById(parId), + linkElm: document.getElementById(linkId) + }; + } + + function createPseudoTests(elmName, desc) { + it(`returns undefined if ${desc} has a :before pseudo element`, () => { + const link = queryFixture(` + +

A link inside a block of text

+ `).actualNode; + const result = linkInBlockStyleCheck.call(checkContext, link); + assert.isUndefined(result); + assert.deepEqual(checkContext._data, { messageKey: 'pseudoContent' }); + assert.equal(checkContext._relatedNodes[0], link.parentNode.parentNode); + }); + + it(`returns undefined if ${desc} has a :after pseudo element`, () => { + const link = queryFixture(` + +

A link inside a block of text

+ `).actualNode; + const result = linkInBlockStyleCheck.call(checkContext, link); + assert.isUndefined(result); + assert.deepEqual(checkContext._data, { messageKey: 'pseudoContent' }); + assert.equal(checkContext._relatedNodes[0], link.parentNode.parentNode); + }); + + it(`does not return undefined if ${desc} pseudo content is none`, () => { + const link = queryFixture(` + +

A link inside a block of text

+ `).actualNode; + const result = linkInBlockStyleCheck.call(checkContext, link); + assert.isFalse(result); + }); } describe('link default state', () => { @@ -148,60 +195,41 @@ describe('link-in-text-block-style', () => { }); describe('links distinguished through style', () => { + const testSuites = { + underline: { textDecoration: 'underline' }, + border: { 'border-bottom': '1px solid' }, + outline: { outline: '1px solid' }, + 'font-weight': { 'font-weight': 'bold' }, + 'font-size': { 'font-size': '2rem' }, + 'background-image': { 'background-image': 'url()' } + }; + it('returns false if link style matches parent', () => { - const linkElm = getLinkElm({}); + const { linkElm, parentElm } = getLinkElm({}); assert.isFalse(linkInBlockStyleCheck.call(checkContext, linkElm)); - assert.equal(checkContext._relatedNodes[0], linkElm.parentNode); + assert.equal(checkContext._relatedNodes[0], parentElm); assert.isNull(checkContext._data); }); - it('returns true if link has underline', () => { - const linkElm = getLinkElm({ - textDecoration: 'underline' + Object.entries(testSuites).forEach(([property, styles]) => { + it(`returns true if link has ${property}`, () => { + const { linkElm, parentElm } = getLinkElm(styles); + assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); + assert.equal(checkContext._relatedNodes[0], parentElm); + assert.isNull(checkContext._data); }); - assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); - assert.equal(checkContext._relatedNodes[0], linkElm.parentNode); - assert.isNull(checkContext._data); - }); - - it('returns undefined when the link has a :before pseudo element', () => { - const link = queryFixture(` - -

A link inside a block of text

- `).actualNode; - const result = linkInBlockStyleCheck.call(checkContext, link); - assert.isUndefined(result); - assert.deepEqual(checkContext._data, { messageKey: 'pseudoContent' }); - assert.equal(checkContext._relatedNodes[0], link.parentNode); }); - it('returns undefined when the link has a :after pseudo element', () => { - const link = queryFixture(` - -

A link inside a block of text

- `).actualNode; - const result = linkInBlockStyleCheck.call(checkContext, link); - assert.isUndefined(result); - assert.deepEqual(checkContext._data, { messageKey: 'pseudoContent' }); - assert.equal(checkContext._relatedNodes[0], link.parentNode); + Object.entries(testSuites).forEach(([property, styles]) => { + it(`returns true if ancestor inline element has ${property}`, () => { + const { linkElm, parentElm } = getLinkElm({}, styles); + assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); + assert.equal(checkContext._relatedNodes[0], parentElm); + assert.isNull(checkContext._data); + }); }); - it('does not return undefined when the pseudo element content is none', () => { - const link = queryFixture(` - -

A link inside a block of text

- `).actualNode; - const result = linkInBlockStyleCheck.call(checkContext, link); - assert.isFalse(result); - }); + createPseudoTests('a', 'link'); + createPseudoTests('span', 'ancestor inline element'); }); }); From 47bf2b518d1707ce22cf51aa9bbf8d75388f6754 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Aug 2023 09:35:42 -0600 Subject: [PATCH 2/9] integration tests --- .../link-in-text-block.html | 26 +++++++++++++++++++ .../link-in-text-block.json | 2 ++ 2 files changed, 28 insertions(+) diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.html b/test/integration/rules/link-in-text-block/link-in-text-block.html index 9d69d819d0..b02d5c5469 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.html +++ b/test/integration/rules/link-in-text-block/link-in-text-block.html @@ -74,6 +74,32 @@

Non-color change tests

>

+

+ paragraph of text (pass: ancestor span is bolder than paragraph) + + + Link text + +

+ +

+ paragraph of text (pass: ancestor span uses border) + + + Link text + +

+

Color change tests

Note that these tests limit changes to one variable per test. No attempt is diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.json b/test/integration/rules/link-in-text-block/link-in-text-block.json index e4e2c7dfaa..a008073f7e 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.json +++ b/test/integration/rules/link-in-text-block/link-in-text-block.json @@ -10,6 +10,8 @@ ["#pass-default-styling-aria-hidden"], ["#pass-different-weight"], ["#pass-different-size"], + ["#pass-ancestor-different-weight"], + ["#pass-ancestor-border"], ["#pass-background-color"], ["#pass-text-color"], ["#pass-same-colors"] From 859ffcf7e8841763466a14dad69652ba15f8cafa Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Aug 2023 09:36:53 -0600 Subject: [PATCH 3/9] typo --- .../rules/link-in-text-block/link-in-text-block.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.html b/test/integration/rules/link-in-text-block/link-in-text-block.html index b02d5c5469..b00abba6c0 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.html +++ b/test/integration/rules/link-in-text-block/link-in-text-block.html @@ -89,7 +89,7 @@

Non-color change tests

paragraph of text (pass: ancestor span uses border) - + Date: Thu, 24 Aug 2023 09:39:34 -0600 Subject: [PATCH 4/9] fix same color issue --- .../rules/link-in-text-block/link-in-text-block.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.html b/test/integration/rules/link-in-text-block/link-in-text-block.html index b00abba6c0..65800b9665 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.html +++ b/test/integration/rules/link-in-text-block/link-in-text-block.html @@ -55,7 +55,7 @@

Non-color change tests

paragraph of text (pass: link is bolder than paragraph) @@ -66,7 +66,7 @@

Non-color change tests

paragraph of text (pass: link is larger that paragraph) @@ -78,7 +78,7 @@

Non-color change tests

paragraph of text (pass: ancestor span is bolder than paragraph) @@ -89,9 +89,9 @@

Non-color change tests

paragraph of text (pass: ancestor span uses border) - + From 470b462c6e4323f6e73de0f915408b3cf0bc5396 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 24 Aug 2023 09:41:16 -0600 Subject: [PATCH 5/9] add u example --- .../rules/link-in-text-block/link-in-text-block.html | 9 +++++++++ .../rules/link-in-text-block/link-in-text-block.json | 1 + 2 files changed, 10 insertions(+) diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.html b/test/integration/rules/link-in-text-block/link-in-text-block.html index 65800b9665..4d8882ac85 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.html +++ b/test/integration/rules/link-in-text-block/link-in-text-block.html @@ -100,6 +100,15 @@

Non-color change tests

+

+ paragraph of text (pass: ancestor is a u element) + + + Link text + +

+

Color change tests

Note that these tests limit changes to one variable per test. No attempt is diff --git a/test/integration/rules/link-in-text-block/link-in-text-block.json b/test/integration/rules/link-in-text-block/link-in-text-block.json index a008073f7e..ed6472dc99 100644 --- a/test/integration/rules/link-in-text-block/link-in-text-block.json +++ b/test/integration/rules/link-in-text-block/link-in-text-block.json @@ -12,6 +12,7 @@ ["#pass-different-size"], ["#pass-ancestor-different-weight"], ["#pass-ancestor-border"], + ["#pass-ancestor-u"], ["#pass-background-color"], ["#pass-text-color"], ["#pass-same-colors"] From 2f100c10fed98132d3cd8da65093b6aa2ff3fb18 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 30 Aug 2023 16:50:07 -0600 Subject: [PATCH 6/9] figure out text-decoration --- .../link-in-text-block-style-evaluate.js | 8 +- lib/commons/color/element-is-distinct.js | 85 ++-- test/commons/color/element-is-distinct.js | 404 +++++++++++------- 3 files changed, 301 insertions(+), 196 deletions(-) diff --git a/lib/checks/color/link-in-text-block-style-evaluate.js b/lib/checks/color/link-in-text-block-style-evaluate.js index f51101493d..5758323cb7 100644 --- a/lib/checks/color/link-in-text-block-style-evaluate.js +++ b/lib/checks/color/link-in-text-block-style-evaluate.js @@ -28,11 +28,11 @@ export default function linkInTextBlockStyleEvaluate(node) { this.relatedNodes([parentBlock]); - for (const inlineNode of inlineNodes) { - if (elementIsDistinct(inlineNode, parentBlock)) { - return true; - } + if (elementIsDistinct(node, parentBlock)) { + return true; + } + for (const inlineNode of inlineNodes) { if (hasPseudoContent(inlineNode)) { this.data({ messageKey: 'pseudoContent' }); return undefined; diff --git a/lib/commons/color/element-is-distinct.js b/lib/commons/color/element-is-distinct.js index a3ff6a6fae..2028f6adb4 100644 --- a/lib/commons/color/element-is-distinct.js +++ b/lib/commons/color/element-is-distinct.js @@ -1,4 +1,5 @@ import Color from './color'; +import { getComposedParent } from '../dom'; /** * Creates a string array of fonts for given CSSStyleDeclaration object @@ -25,7 +26,7 @@ function _getFonts(style) { * @return {Boolean} */ function elementIsDistinct(node, ancestorNode) { - var nodeStyle = window.getComputedStyle(node); + const nodeStyle = window.getComputedStyle(node); // Check if the link has a background if (nodeStyle.getPropertyValue('background-image') !== 'none') { @@ -33,52 +34,72 @@ function elementIsDistinct(node, ancestorNode) { } // Check if the link has a border or outline - var hasBorder = ['border-bottom', 'border-top', 'outline'].reduce( - (result, edge) => { - var borderClr = new Color(); - borderClr.parseString(nodeStyle.getPropertyValue(edge + '-color')); - - // Check if a border/outline was specified - return ( - result || - // or if the current border edge / outline - (nodeStyle.getPropertyValue(edge + '-style') !== 'none' && - parseFloat(nodeStyle.getPropertyValue(edge + '-width')) > 0 && - borderClr.alpha !== 0) - ); - }, - false - ); + const hasBorder = ['border-bottom', 'border-top', 'outline'].some(edge => { + const borderClr = new Color(); + borderClr.parseString(nodeStyle.getPropertyValue(edge + '-color')); + + // Check if a border/outline was specified + return ( + nodeStyle.getPropertyValue(edge + '-style') !== 'none' && + parseFloat(nodeStyle.getPropertyValue(edge + '-width')) > 0 && + borderClr.alpha !== 0 + ); + }); if (hasBorder) { return true; } - var parentStyle = window.getComputedStyle(ancestorNode); - // Compare fonts - if (_getFonts(nodeStyle)[0] !== _getFonts(parentStyle)[0]) { + const ancestorStyle = window.getComputedStyle(ancestorNode); + let curNode = node; + while (curNode !== ancestorNode) { + const curNodeStyle = window.getComputedStyle(curNode); + + // text decoration styles are not inherited so we need + // to check each node from the target node to the + // ancestor node + const hasStyle = ['text-decoration-line', 'text-decoration-style'].some( + cssProp => { + const ancestorValue = ancestorStyle.getPropertyValue(cssProp); + const curNodeValue = curNodeStyle.getPropertyValue(cssProp); + + /* + For logic purposes we can treat the target node and all inline ancestors as a single logic point since if any of them define a text-decoration style it will visually apply that style to the target. + + A target node is distinct if it defines a text-decoration and either the ancestor does not define one or the target's text-decoration is different than the ancestor. A target that does not define a text-decoration can never be distinct from the ancestor. + */ + return ( + curNodeValue !== 'none' && + (ancestorValue === 'none' || curNodeValue !== ancestorValue) + ); + } + ); + + if (hasStyle) { + return true; + } + + curNode = getComposedParent(curNode); + } + + // font styles are inherited so we only need to check + // the target node + if (_getFonts(nodeStyle)[0] !== _getFonts(ancestorStyle)[0]) { return true; } - var hasStyle = [ - 'text-decoration-line', - 'text-decoration-style', - 'font-weight', - 'font-style', - 'font-size' - ].reduce((result, cssProp) => { + let hasStyle = ['font-weight', 'font-style', 'font-size'].some(cssProp => { return ( - result || nodeStyle.getPropertyValue(cssProp) !== - parentStyle.getPropertyValue(cssProp) + ancestorStyle.getPropertyValue(cssProp) ); - }, false); + }); - var tDec = nodeStyle.getPropertyValue('text-decoration'); + const tDec = nodeStyle.getPropertyValue('text-decoration'); if (tDec.split(' ').length < 3) { // old style CSS text decoration hasStyle = - hasStyle || tDec !== parentStyle.getPropertyValue('text-decoration'); + hasStyle || tDec !== ancestorStyle.getPropertyValue('text-decoration'); } return hasStyle; diff --git a/test/commons/color/element-is-distinct.js b/test/commons/color/element-is-distinct.js index 413482bb30..b326983092 100644 --- a/test/commons/color/element-is-distinct.js +++ b/test/commons/color/element-is-distinct.js @@ -1,271 +1,355 @@ describe('color.elementIsDistinct', () => { - let styleElm; - let elementIsDistinct; + const elementIsDistinct = axe.commons.color.elementIsDistinct; + const fixtureSetup = axe.testUtils.fixtureSetup; + const fixture = document.querySelector('#fixture'); + + function convertStylePropsToString(props = {}) { + return Object.entries(props) + .map(([name, value]) => `${name}: ${value}`) + .join(';'); + } - const fixture = document.getElementById('fixture'); + function createTestFixture({ target, root, inline } = {}) { + const linkStyles = convertStylePropsToString(target); + const paragraphStyles = convertStylePropsToString(root); + const spanStyles = convertStylePropsToString(inline); - before(() => { - styleElm = document.createElement('style'); - document.head.appendChild(styleElm); - }); + fixtureSetup(` +

+ + Hello World + +

+ `); - const defaultStyle = { - color: '#000', - textDecoration: 'none' - }; - - function createStyleString(selector, outerStyle) { - // Merge style with the default - const styleObj = {}; - for (const prop in defaultStyle) { - if (defaultStyle.hasOwnProperty(prop)) { - styleObj[prop] = defaultStyle[prop]; - } - } - for (const prop in outerStyle) { - if (outerStyle.hasOwnProperty(prop)) { - styleObj[prop] = outerStyle[prop]; - } - } - - const cssLines = Object.keys(styleObj) - .map(prop => { - // Make camelCase prop dash separated - const cssPropName = prop - .trim() - .split(/(?=[A-Z])/g) - .reduce((name, propPiece) => { - if (!name) { - return propPiece; - } else { - return name + '-' + propPiece.toLowerCase(); - } - }, null); - - // Return indented line of style code - return ' ' + cssPropName + ':' + styleObj[prop] + ';'; - }) - .join('\n'); - - // Add to the style element - styleElm.innerHTML += selector + ' {\n' + cssLines + '\n}\n'; - } - - function getLinkElm(linkStyle, paragraphStyle) { - // Get a random id and build the style string - const linkId = 'linkid-' + Math.floor(Math.random() * 100000); - const parId = 'parid-' + Math.floor(Math.random() * 100000); - - createStyleString('#' + linkId, linkStyle); - createStyleString('#' + parId, paragraphStyle); - - fixture.innerHTML += - '

Text ' + - 'link' + - '

'; return { - link: document.getElementById(linkId), - par: document.getElementById(parId) + root: fixture.querySelector('p'), + inline: fixture.querySelector('span'), + target: fixture.querySelector('a') }; } - beforeEach(() => { - createStyleString('p', defaultStyle); - elementIsDistinct = axe.commons.color.elementIsDistinct; - }); - - afterEach(() => { - fixture.innerHTML = ''; - styleElm.innerHTML = ''; - }); - - after(() => { - styleElm.parentNode.removeChild(styleElm); - }); - it('returns false without style adjustments', () => { - const elms = getLinkElm({}); - const result = elementIsDistinct(elms.link, elms.par); + const elms = createTestFixture(); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns true with background-image set', () => { - const elms = getLinkElm({ - background: 'url(icon.png) no-repeat' + const elms = createTestFixture({ + target: { background: 'url(icon.png) no-repeat' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns true with border: dashed 1px black', () => { - const elms = getLinkElm({ - border: 'dashed 1px black' + const elms = createTestFixture({ + target: { border: 'dashed 1px black' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns true with border-bottom: dashed 1px black', () => { - const elms = getLinkElm({ - borderBottom: 'dashed 1px black' + const elms = createTestFixture({ + target: { 'border-bottom': 'dashed 1px black' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns false with border: solid 0px black', () => { - const elms = getLinkElm({ - border: 'solid 0px black' + const elms = createTestFixture({ + target: { border: 'solid 0px black' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns false with border: none 1px black', () => { - const elms = getLinkElm({ - border: 'none 1px black' + const elms = createTestFixture({ + target: { border: 'none 1px black' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns false with border: solid 1px transparent', () => { - const elms = getLinkElm({ - border: 'solid 1px transparent' + const elms = createTestFixture({ + target: { border: 'solid 1px transparent' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns true with outline: solid 1px black', () => { - const elms = getLinkElm({ - outline: 'solid 1px black' + const elms = createTestFixture({ + target: { outline: 'solid 1px black' } }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns true if font-weight is different', () => { - const elms = getLinkElm( - { - fontWeight: 'bold' + const elms = createTestFixture({ + target: { + 'font-weight': 'bold' }, - { - fontWeight: 'normal' + root: { + 'font-weight': 'normal' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns false if font-weight is the same', () => { - const elms = getLinkElm( - { - fontWeight: 'bold' + const elms = createTestFixture({ + target: { + 'font-weight': 'bold' }, - { - fontWeight: 'bold' + root: { + 'font-weight': 'bold' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('compares font numbers and labels correctly', () => { - const elms = getLinkElm( - { - fontWeight: 'bold' + const elms = createTestFixture({ + target: { + 'font-weight': 'bold' }, - { - fontWeight: '700' + root: { + 'font-weight': '700' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns true if text-decoration is different', () => { - const elms = getLinkElm( - { - textDecoration: 'underline' + const elms = createTestFixture({ + target: { + 'text-decoration': 'underline' }, - { - textDecoration: 'none' + root: { + 'text-decoration': 'none' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns false if text-decoration is the same', () => { - const elms = getLinkElm( - { - textDecoration: 'underline' + const elms = createTestFixture({ + target: { + 'text-decoration': 'underline' }, - { - textDecoration: 'underline' + root: { + 'text-decoration': 'underline' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); it('returns true if font-size is different', () => { - const elms = getLinkElm( - { - fontSize: '14px' + const elms = createTestFixture({ + target: { + 'font-size': '14px' }, - { - fontSize: '12px' + root: { + 'font-size': '12px' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns true if font-family is different', () => { - const elms = getLinkElm( - { - fontFamily: 'Arial' + const elms = createTestFixture({ + target: { + 'font-family': 'Arial' }, - { - fontFamily: 'Arial-black' + root: { + 'font-family': 'Arial-black' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isTrue(result); }); it('returns false if the first font-family is identical', () => { - const elms = getLinkElm( - { - fontFamily: 'Arial-black, Arial' + const elms = createTestFixture({ + target: { + 'font-family': 'Arial-black, Arial' }, - { - fontFamily: 'Arial-black, sans-serif' + root: { + 'font-family': 'Arial-black, sans-serif' } - ); + }); - const result = elementIsDistinct(elms.link, elms.par); + const result = elementIsDistinct(elms.target, elms.root); assert.isFalse(result); }); + + describe('inline element', () => { + describe('text-decoration-line', () => { + it('returns true if inline adds', () => { + const elms = createTestFixture({ + inline: { + 'text-decoration': 'underline' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns true if target has text-decoration:none and inline adds', () => { + const elms = createTestFixture({ + target: { + 'text-decoration': 'none' + }, + inline: { + 'text-decoration': 'underline' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns false if inline and root use same value', () => { + const elms = createTestFixture({ + root: { + 'text-decoration': 'underline' + }, + inline: { + 'text-decoration': 'underline' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + + it('returns true if inline and root use different value', () => { + const elms = createTestFixture({ + root: { + 'text-decoration': 'underline' + }, + inline: { + 'text-decoration': 'overline' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + }); + + describe('text-decoration-style', () => { + it('returns true if inline adds', () => { + const elms = createTestFixture({ + inline: { + 'text-decoration': 'underline solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns true if target has text-decoration:none and inline adds', () => { + const elms = createTestFixture({ + target: { + 'text-decoration': 'none' + }, + inline: { + 'text-decoration': 'underline solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns false if inline and root use same value', () => { + const elms = createTestFixture({ + root: { + 'text-decoration': 'underline solid' + }, + inline: { + 'text-decoration': 'underline solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + + it('returns true if inline and root use different value', () => { + const elms = createTestFixture({ + root: { + 'text-decoration': 'underline solid' + }, + inline: { + 'text-decoration': 'underline wavy' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + }); + + describe('font', () => { + it('returns true if inline adds', () => { + const elms = createTestFixture({ + inline: { + 'font-weight': 'bold' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns false if target has same font as root and inline adds', () => { + const elms = createTestFixture({ + target: { + 'font-weight': 'normal' + }, + root: { + 'font-weight': 'normal' + }, + inline: { + 'font-weight': 'bold' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + }); + }); }); From 014585f85459eed8e44968e4ddd94888b08cc6ed Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Thu, 31 Aug 2023 08:26:20 -0600 Subject: [PATCH 7/9] fix tests --- lib/commons/color/element-is-distinct.js | 48 ++++++------- test/commons/color/element-is-distinct.js | 87 ++++++++++++++++++++++- 2 files changed, 108 insertions(+), 27 deletions(-) diff --git a/lib/commons/color/element-is-distinct.js b/lib/commons/color/element-is-distinct.js index 2028f6adb4..763fb29180 100644 --- a/lib/commons/color/element-is-distinct.js +++ b/lib/commons/color/element-is-distinct.js @@ -28,36 +28,34 @@ function _getFonts(style) { function elementIsDistinct(node, ancestorNode) { const nodeStyle = window.getComputedStyle(node); - // Check if the link has a background - if (nodeStyle.getPropertyValue('background-image') !== 'none') { - return true; - } - - // Check if the link has a border or outline - const hasBorder = ['border-bottom', 'border-top', 'outline'].some(edge => { - const borderClr = new Color(); - borderClr.parseString(nodeStyle.getPropertyValue(edge + '-color')); - - // Check if a border/outline was specified - return ( - nodeStyle.getPropertyValue(edge + '-style') !== 'none' && - parseFloat(nodeStyle.getPropertyValue(edge + '-width')) > 0 && - borderClr.alpha !== 0 - ); - }); - - if (hasBorder) { - return true; - } - const ancestorStyle = window.getComputedStyle(ancestorNode); let curNode = node; while (curNode !== ancestorNode) { const curNodeStyle = window.getComputedStyle(curNode); - // text decoration styles are not inherited so we need - // to check each node from the target node to the - // ancestor node + // Check if the link has a background + if (curNodeStyle.getPropertyValue('background-image') !== 'none') { + return true; + } + + // Check if the link has a border or outline + const hasBorder = ['border-bottom', 'border-top', 'outline'].some(edge => { + const borderClr = new Color(); + borderClr.parseString(curNodeStyle.getPropertyValue(edge + '-color')); + + // Check if a border/outline was specified + return ( + curNodeStyle.getPropertyValue(edge + '-style') !== 'none' && + parseFloat(curNodeStyle.getPropertyValue(edge + '-width')) > 0 && + borderClr.alpha !== 0 + ); + }); + + if (hasBorder) { + return true; + } + + // Check if the link has text-decoration styles const hasStyle = ['text-decoration-line', 'text-decoration-style'].some( cssProp => { const ancestorValue = ancestorStyle.getPropertyValue(cssProp); diff --git a/test/commons/color/element-is-distinct.js b/test/commons/color/element-is-distinct.js index b326983092..fa37f1b907 100644 --- a/test/commons/color/element-is-distinct.js +++ b/test/commons/color/element-is-distinct.js @@ -212,8 +212,91 @@ describe('color.elementIsDistinct', () => { }); describe('inline element', () => { + describe('background-image', () => { + it('returns true if inline adds background-image', () => { + const elms = createTestFixture({ + inline: { + background: 'url(icon.png) no-repeat' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + }); + + describe('border', () => { + it('returns true if inline adds border', () => { + const elms = createTestFixture({ + inline: { + 'border-bottom': '1px solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns false if inline adds border with 0 width', () => { + const elms = createTestFixture({ + inline: { + 'border-top': '0 solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + + it('returns false if inline adds border with 0 alpha', () => { + const elms = createTestFixture({ + inline: { + 'border-bottom': '2px solid rgba(255,255,255,0)' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + }); + + describe('outline', () => { + it('returns true if inline adds outline', () => { + const elms = createTestFixture({ + inline: { + outline: '1px solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isTrue(result); + }); + + it('returns false if inline adds outline with 0 width', () => { + const elms = createTestFixture({ + inline: { + outline: '0 solid' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + + it('returns false if inline adds outline with 0 alpha', () => { + const elms = createTestFixture({ + inline: { + outline: '2px solid rgba(255,255,255,0)' + } + }); + + const result = elementIsDistinct(elms.target, elms.root); + assert.isFalse(result); + }); + }); + describe('text-decoration-line', () => { - it('returns true if inline adds', () => { + it('returns true if inline adds text-decoration-line', () => { const elms = createTestFixture({ inline: { 'text-decoration': 'underline' @@ -268,7 +351,7 @@ describe('color.elementIsDistinct', () => { }); describe('text-decoration-style', () => { - it('returns true if inline adds', () => { + it('returns true if inline adds text-decoration-style', () => { const elms = createTestFixture({ inline: { 'text-decoration': 'underline solid' From 4d839b9209ad1a58936ffc548efc48c0ee91cf48 Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 20 Sep 2023 16:07:49 -0600 Subject: [PATCH 8/9] suggestions --- lib/commons/color/element-is-distinct.js | 117 ++++++++------- test/checks/color/link-in-text-block-style.js | 28 ++-- test/commons/color/element-is-distinct.js | 141 +++++++++++++----- 3 files changed, 180 insertions(+), 106 deletions(-) diff --git a/lib/commons/color/element-is-distinct.js b/lib/commons/color/element-is-distinct.js index 763fb29180..bb538f3243 100644 --- a/lib/commons/color/element-is-distinct.js +++ b/lib/commons/color/element-is-distinct.js @@ -1,21 +1,7 @@ import Color from './color'; +import { sanitize } from '../text'; import { getComposedParent } from '../dom'; -/** - * Creates a string array of fonts for given CSSStyleDeclaration object - * @private - * @param {Object} style CSSStyleDeclaration object - * @return {Array} - */ -function _getFonts(style) { - return style - .getPropertyValue('font-family') - .split(/[,;]/g) - .map(font => { - return font.trim().toLowerCase(); - }); -} - /** * Determine if the text content of two nodes is styled in a way that they can be distinguished without relying on color * @method elementIsDistinct @@ -25,7 +11,7 @@ function _getFonts(style) { * @param {HTMLElement} ancestorNode The ancestor node element to check * @return {Boolean} */ -function elementIsDistinct(node, ancestorNode) { +export default function elementIsDistinct(node, ancestorNode) { const nodeStyle = window.getComputedStyle(node); const ancestorStyle = window.getComputedStyle(ancestorNode); @@ -38,42 +24,11 @@ function elementIsDistinct(node, ancestorNode) { return true; } - // Check if the link has a border or outline - const hasBorder = ['border-bottom', 'border-top', 'outline'].some(edge => { - const borderClr = new Color(); - borderClr.parseString(curNodeStyle.getPropertyValue(edge + '-color')); - - // Check if a border/outline was specified - return ( - curNodeStyle.getPropertyValue(edge + '-style') !== 'none' && - parseFloat(curNodeStyle.getPropertyValue(edge + '-width')) > 0 && - borderClr.alpha !== 0 - ); - }); - - if (hasBorder) { - return true; - } - - // Check if the link has text-decoration styles - const hasStyle = ['text-decoration-line', 'text-decoration-style'].some( - cssProp => { - const ancestorValue = ancestorStyle.getPropertyValue(cssProp); - const curNodeValue = curNodeStyle.getPropertyValue(cssProp); - - /* - For logic purposes we can treat the target node and all inline ancestors as a single logic point since if any of them define a text-decoration style it will visually apply that style to the target. - - A target node is distinct if it defines a text-decoration and either the ancestor does not define one or the target's text-decoration is different than the ancestor. A target that does not define a text-decoration can never be distinct from the ancestor. - */ - return ( - curNodeValue !== 'none' && - (ancestorValue === 'none' || curNodeValue !== ancestorValue) - ); - } - ); - - if (hasStyle) { + if ( + hasDifferentText(node, curNode) || + hasVisibleBorder(curNodeStyle) || + hasVisibleTextDecortation(curNodeStyle, ancestorStyle) + ) { return true; } @@ -82,7 +37,7 @@ function elementIsDistinct(node, ancestorNode) { // font styles are inherited so we only need to check // the target node - if (_getFonts(nodeStyle)[0] !== _getFonts(ancestorStyle)[0]) { + if (getFonts(nodeStyle)[0] !== getFonts(ancestorStyle)[0]) { return true; } @@ -103,4 +58,58 @@ function elementIsDistinct(node, ancestorNode) { return hasStyle; } -export default elementIsDistinct; +/** + * Creates a string array of fonts for given CSSStyleDeclaration object + * @private + * @param {Object} style CSSStyleDeclaration object + * @return {Array} + */ +function getFonts(style) { + return style + .getPropertyValue('font-family') + .split(/[,;]/g) + .map(font => { + return font.trim().toLowerCase(); + }); +} + +function hasDifferentText(node, ancestorNode) { + // 3 is an arbitrary number that helps account for punctuation + // and emoji that takes up two characters + return ancestorNode.textContent + .split(node.textContent) + .some(text => sanitize(text).length > 3); +} + +function hasVisibleBorder(nodeStyle) { + // Check if the link has a border or outline + return ['border-bottom', 'border-top', 'outline'].some(edge => { + const borderClr = new Color(); + borderClr.parseString(nodeStyle.getPropertyValue(edge + '-color')); + + // Check if a border/outline was specified + return ( + nodeStyle.getPropertyValue(edge + '-style') !== 'none' && + parseFloat(nodeStyle.getPropertyValue(edge + '-width')) > 0 && + borderClr.alpha !== 0 + ); + }); +} + +function hasVisibleTextDecortation(nodeStyle, ancestorStyle) { + // Check if the link has text-decoration styles + return ['text-decoration-line', 'text-decoration-style'].some(cssProp => { + const ancestorValue = ancestorStyle.getPropertyValue(cssProp); + const nodeValue = nodeStyle.getPropertyValue(cssProp); + + /* + For logic purposes we can treat the target node and all inline ancestors as a single logic point since if any of them define a text-decoration style it will visually apply that style to the target. + + A target node is distinct if it defines a text-decoration and either the ancestor does not define one or the target's text-decoration is different than the ancestor. A target that does not define a text-decoration can never be distinct from the ancestor. + */ + return ( + nodeValue !== 'none' && + (ancestorValue === 'none' || nodeValue !== ancestorValue) + ); + }); +} diff --git a/test/checks/color/link-in-text-block-style.js b/test/checks/color/link-in-text-block-style.js index cede991c94..2242cb396c 100644 --- a/test/checks/color/link-in-text-block-style.js +++ b/test/checks/color/link-in-text-block-style.js @@ -212,20 +212,20 @@ describe('link-in-text-block-style', () => { }); Object.entries(testSuites).forEach(([property, styles]) => { - it(`returns true if link has ${property}`, () => { - const { linkElm, parentElm } = getLinkElm(styles); - assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); - assert.equal(checkContext._relatedNodes[0], parentElm); - assert.isNull(checkContext._data); - }); - }); - - Object.entries(testSuites).forEach(([property, styles]) => { - it(`returns true if ancestor inline element has ${property}`, () => { - const { linkElm, parentElm } = getLinkElm({}, styles); - assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); - assert.equal(checkContext._relatedNodes[0], parentElm); - assert.isNull(checkContext._data); + describe(`with ${property}`, () => { + it(`returns true if link has ${property}`, () => { + const { linkElm, parentElm } = getLinkElm(styles); + assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); + assert.equal(checkContext._relatedNodes[0], parentElm); + assert.isNull(checkContext._data); + }); + + it(`returns true if ancestor inline element has ${property}`, () => { + const { linkElm, parentElm } = getLinkElm({}, styles); + assert.isTrue(linkInBlockStyleCheck.call(checkContext, linkElm)); + assert.equal(checkContext._relatedNodes[0], parentElm); + assert.isNull(checkContext._data); + }); }); }); diff --git a/test/commons/color/element-is-distinct.js b/test/commons/color/element-is-distinct.js index fa37f1b907..0f74068e72 100644 --- a/test/commons/color/element-is-distinct.js +++ b/test/commons/color/element-is-distinct.js @@ -9,10 +9,10 @@ describe('color.elementIsDistinct', () => { .join(';'); } - function createTestFixture({ target, root, inline } = {}) { + function createTestFixture({ target, root, targetWrap } = {}) { const linkStyles = convertStylePropsToString(target); const paragraphStyles = convertStylePropsToString(root); - const spanStyles = convertStylePropsToString(inline); + const spanStyles = convertStylePropsToString(targetWrap); fixtureSetup(`

@@ -24,7 +24,7 @@ describe('color.elementIsDistinct', () => { return { root: fixture.querySelector('p'), - inline: fixture.querySelector('span'), + targetWrap: fixture.querySelector('span'), target: fixture.querySelector('a') }; } @@ -211,11 +211,11 @@ describe('color.elementIsDistinct', () => { assert.isFalse(result); }); - describe('inline element', () => { + describe('targetWrap element', () => { describe('background-image', () => { - it('returns true if inline adds background-image', () => { + it('returns true if targetWrap adds background-image', () => { const elms = createTestFixture({ - inline: { + targetWrap: { background: 'url(icon.png) no-repeat' } }); @@ -225,10 +225,75 @@ describe('color.elementIsDistinct', () => { }); }); + describe('text', () => { + it('returns true if the targetWrap has more than 3 characters before target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `Hows ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + + it('returns true if the targetWrap has more than 3 characters after target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `${elms.target.outerHTML} Hows`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + + it('returns false if the targetWrap has 3 characters before target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `the ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('returns false if the targetWrap has 3 characters after target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `${elms.target.outerHTML} the`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('returns false if the targetWrap has less than 3 characters before target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `: ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('returns false if the targetWrap has less than 3 characters after target', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `${elms.target.outerHTML}: `; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('ignores whitespace', () => { + const elms = createTestFixture(); + elms.targetWrap.innerHTML = `\n\t ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + }); + describe('border', () => { - it('returns true if inline adds border', () => { + it('returns true if targetWrap adds border', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'border-bottom': '1px solid' } }); @@ -237,9 +302,9 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns false if inline adds border with 0 width', () => { + it('returns false if targetWrap adds border with 0 width', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'border-top': '0 solid' } }); @@ -248,9 +313,9 @@ describe('color.elementIsDistinct', () => { assert.isFalse(result); }); - it('returns false if inline adds border with 0 alpha', () => { + it('returns false if targetWrap adds border with 0 alpha', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'border-bottom': '2px solid rgba(255,255,255,0)' } }); @@ -261,9 +326,9 @@ describe('color.elementIsDistinct', () => { }); describe('outline', () => { - it('returns true if inline adds outline', () => { + it('returns true if targetWrap adds outline', () => { const elms = createTestFixture({ - inline: { + targetWrap: { outline: '1px solid' } }); @@ -272,9 +337,9 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns false if inline adds outline with 0 width', () => { + it('returns false if targetWrap adds outline with 0 width', () => { const elms = createTestFixture({ - inline: { + targetWrap: { outline: '0 solid' } }); @@ -283,9 +348,9 @@ describe('color.elementIsDistinct', () => { assert.isFalse(result); }); - it('returns false if inline adds outline with 0 alpha', () => { + it('returns false if targetWrap adds outline with 0 alpha', () => { const elms = createTestFixture({ - inline: { + targetWrap: { outline: '2px solid rgba(255,255,255,0)' } }); @@ -296,9 +361,9 @@ describe('color.elementIsDistinct', () => { }); describe('text-decoration-line', () => { - it('returns true if inline adds text-decoration-line', () => { + it('returns true if targetWrap adds text-decoration-line', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'text-decoration': 'underline' } }); @@ -307,12 +372,12 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns true if target has text-decoration:none and inline adds', () => { + it('returns true if target has text-decoration:none and targetWrap adds', () => { const elms = createTestFixture({ target: { 'text-decoration': 'none' }, - inline: { + targetWrap: { 'text-decoration': 'underline' } }); @@ -321,12 +386,12 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns false if inline and root use same value', () => { + it('returns false if targetWrap and root use same value', () => { const elms = createTestFixture({ root: { 'text-decoration': 'underline' }, - inline: { + targetWrap: { 'text-decoration': 'underline' } }); @@ -335,12 +400,12 @@ describe('color.elementIsDistinct', () => { assert.isFalse(result); }); - it('returns true if inline and root use different value', () => { + it('returns true if targetWrap and root use different value', () => { const elms = createTestFixture({ root: { 'text-decoration': 'underline' }, - inline: { + targetWrap: { 'text-decoration': 'overline' } }); @@ -351,9 +416,9 @@ describe('color.elementIsDistinct', () => { }); describe('text-decoration-style', () => { - it('returns true if inline adds text-decoration-style', () => { + it('returns true if targetWrap adds text-decoration-style', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'text-decoration': 'underline solid' } }); @@ -362,12 +427,12 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns true if target has text-decoration:none and inline adds', () => { + it('returns true if target has text-decoration:none and targetWrap adds', () => { const elms = createTestFixture({ target: { 'text-decoration': 'none' }, - inline: { + targetWrap: { 'text-decoration': 'underline solid' } }); @@ -376,12 +441,12 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns false if inline and root use same value', () => { + it('returns false if targetWrap and root use same value', () => { const elms = createTestFixture({ root: { 'text-decoration': 'underline solid' }, - inline: { + targetWrap: { 'text-decoration': 'underline solid' } }); @@ -390,12 +455,12 @@ describe('color.elementIsDistinct', () => { assert.isFalse(result); }); - it('returns true if inline and root use different value', () => { + it('returns true if targetWrap and root use different value', () => { const elms = createTestFixture({ root: { 'text-decoration': 'underline solid' }, - inline: { + targetWrap: { 'text-decoration': 'underline wavy' } }); @@ -406,9 +471,9 @@ describe('color.elementIsDistinct', () => { }); describe('font', () => { - it('returns true if inline adds', () => { + it('returns true if targetWrap adds', () => { const elms = createTestFixture({ - inline: { + targetWrap: { 'font-weight': 'bold' } }); @@ -417,7 +482,7 @@ describe('color.elementIsDistinct', () => { assert.isTrue(result); }); - it('returns false if target has same font as root and inline adds', () => { + it('returns false if target has same font as root and targetWrap adds', () => { const elms = createTestFixture({ target: { 'font-weight': 'normal' @@ -425,7 +490,7 @@ describe('color.elementIsDistinct', () => { root: { 'font-weight': 'normal' }, - inline: { + targetWrap: { 'font-weight': 'bold' } }); From 25b3001d0293fad4d55bc3da386a299d5dc5b73a Mon Sep 17 00:00:00 2001 From: Steven Lambert Date: Wed, 20 Sep 2023 16:37:03 -0600 Subject: [PATCH 9/9] fix test --- lib/commons/color/element-is-distinct.js | 10 +- test/commons/color/element-is-distinct.js | 158 +++++++++++++--------- 2 files changed, 98 insertions(+), 70 deletions(-) diff --git a/lib/commons/color/element-is-distinct.js b/lib/commons/color/element-is-distinct.js index bb538f3243..ff0a6df548 100644 --- a/lib/commons/color/element-is-distinct.js +++ b/lib/commons/color/element-is-distinct.js @@ -25,9 +25,9 @@ export default function elementIsDistinct(node, ancestorNode) { } if ( - hasDifferentText(node, curNode) || - hasVisibleBorder(curNodeStyle) || - hasVisibleTextDecortation(curNodeStyle, ancestorStyle) + hasSameText(node, curNode) && + (hasVisibleBorder(curNodeStyle) || + hasVisibleTextDecortation(curNodeStyle, ancestorStyle)) ) { return true; } @@ -73,12 +73,12 @@ function getFonts(style) { }); } -function hasDifferentText(node, ancestorNode) { +function hasSameText(node, ancestorNode) { // 3 is an arbitrary number that helps account for punctuation // and emoji that takes up two characters return ancestorNode.textContent .split(node.textContent) - .some(text => sanitize(text).length > 3); + .every(text => sanitize(text).length <= 3); } function hasVisibleBorder(nodeStyle) { diff --git a/test/commons/color/element-is-distinct.js b/test/commons/color/element-is-distinct.js index 0f74068e72..b9e22f2d3c 100644 --- a/test/commons/color/element-is-distinct.js +++ b/test/commons/color/element-is-distinct.js @@ -225,71 +225,6 @@ describe('color.elementIsDistinct', () => { }); }); - describe('text', () => { - it('returns true if the targetWrap has more than 3 characters before target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `Hows ${elms.target.outerHTML}`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isTrue(result); - }); - - it('returns true if the targetWrap has more than 3 characters after target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `${elms.target.outerHTML} Hows`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isTrue(result); - }); - - it('returns false if the targetWrap has 3 characters before target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `the ${elms.target.outerHTML}`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isFalse(result); - }); - - it('returns false if the targetWrap has 3 characters after target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `${elms.target.outerHTML} the`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isFalse(result); - }); - - it('returns false if the targetWrap has less than 3 characters before target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `: ${elms.target.outerHTML}`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isFalse(result); - }); - - it('returns false if the targetWrap has less than 3 characters after target', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `${elms.target.outerHTML}: `; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isFalse(result); - }); - - it('ignores whitespace', () => { - const elms = createTestFixture(); - elms.targetWrap.innerHTML = `\n\t ${elms.target.outerHTML}`; - const target = elms.targetWrap.querySelector('a'); - - const result = elementIsDistinct(target, elms.root); - assert.isFalse(result); - }); - }); - describe('border', () => { it('returns true if targetWrap adds border', () => { const elms = createTestFixture({ @@ -470,6 +405,99 @@ describe('color.elementIsDistinct', () => { }); }); + describe('text', () => { + it('returns true if targetWrap adds border and has <= 3 characters before target', () => { + const elms = createTestFixture({ + targetWrap: { + 'border-bottom': '1px solid' + } + }); + elms.targetWrap.innerHTML = `the ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + + it('returns true if targetWrap adds outline and has <= 3 characters after target', () => { + const elms = createTestFixture({ + targetWrap: { + outline: '1px solid' + } + }); + elms.targetWrap.innerHTML = `${elms.target.outerHTML} the`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + + it('returns true if targetWrap adds text-decoration and has <= 3 characters before target', () => { + const elms = createTestFixture({ + targetWrap: { + 'text-decoration': 'underline' + } + }); + elms.targetWrap.innerHTML = `the ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + + it('returns false if targetWrap adds border and has > 3 characters before target', () => { + const elms = createTestFixture({ + targetWrap: { + 'border-bottom': '1px solid' + } + }); + elms.targetWrap.innerHTML = `World ${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('returns false if targetWrap adds outline and has <= 3 characters after target', () => { + const elms = createTestFixture({ + targetWrap: { + outline: '1px solid' + } + }); + elms.targetWrap.innerHTML = `${elms.target.outerHTML} World`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('returns false if targetWrap adds text-decoration and has > 3 characters after target', () => { + const elms = createTestFixture({ + targetWrap: { + 'text-decoration': 'underline' + } + }); + elms.targetWrap.innerHTML = `${elms.target.outerHTML} World`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isFalse(result); + }); + + it('ignores whitespace differences', () => { + const elms = createTestFixture({ + targetWrap: { + 'text-decoration': 'underline' + } + }); + elms.targetWrap.innerHTML = `\t \n${elms.target.outerHTML}`; + const target = elms.targetWrap.querySelector('a'); + + const result = elementIsDistinct(target, elms.root); + assert.isTrue(result); + }); + }); + describe('font', () => { it('returns true if targetWrap adds', () => { const elms = createTestFixture({