fix(link-in-text-block): take into account ancestor inline element styles

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);
   }
 
@@ -29,15 +31,19 @@ export default function linkInTextBlockStyleEvaluate(node) {
   if (elementIsDistinct(node, parentBlock)) {
     return true;
   }
-  if (hasPseudoContent(node)) {
-    this.data({ messageKey: 'pseudoContent' });
-    return undefined;
+
+  for (const inlineNode of inlineNodes) {
+    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-';
 }
 

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,60 +26,78 @@ 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') {
-    return true;
-  }
+  const ancestorStyle = window.getComputedStyle(ancestorNode);
+  let curNode = node;
+  while (curNode !== ancestorNode) {
+    const curNodeStyle = window.getComputedStyle(curNode);
+
+    // Check if the link has a background
+    if (curNodeStyle.getPropertyValue('background-image') !== 'none') {
+      return true;
+    }
 
-  // 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 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 (
-        result ||
-        // or if the current border edge / outline
-        (nodeStyle.getPropertyValue(edge + '-style') !== 'none' &&
-          parseFloat(nodeStyle.getPropertyValue(edge + '-width')) > 0 &&
-          borderClr.alpha !== 0)
+        curNodeStyle.getPropertyValue(edge + '-style') !== 'none' &&
+        parseFloat(curNodeStyle.getPropertyValue(edge + '-width')) > 0 &&
+        borderClr.alpha !== 0
       );
-    },
-    false
-  );
+    });
 
-  if (hasBorder) {
-    return true;
+    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) {
+      return true;
+    }
+
+    curNode = getComposedParent(curNode);
   }
 
-  var parentStyle = window.getComputedStyle(ancestorNode);
-  // Compare fonts
-  if (_getFonts(nodeStyle)[0] !== _getFonts(parentStyle)[0]) {
+  // 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;

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 += `
+      <p id="${parId}">
+        <span id="${spanId}">
+          <a href="/" id="${linkId}">link</a>
+        </span>
+      </p>
+    `;
 
-    fixture.innerHTML +=
-      '<p id="' +
-      parId +
-      '"> Text ' +
-      '<a href="/" id="' +
-      linkId +
-      '">link</a>' +
-      '</p>';
     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(`
+        <style>
+          ${elmName}:before { content: '🔗'; }
+          a { text-decoration: none; }
+        </style>
+        <p>A <span><a href="#" id="target">link</a></span> inside a block of text</p>
+      `).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(`
+        <style>
+          ${elmName}:after { content: ""; }
+          a { text-decoration: none; }
+        </style>
+        <p>A <span><a href="#" id="target">link</a></span> inside a block of text</p>
+      `).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(`
+        <style>
+          ${elmName}:after { content: none; position: absolute; }
+          a { text-decoration: none; }
+        </style>
+        <p>A <span><a href="#" id="target">link</a></span> inside a block of text</p>
+      `).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(`
-        <style>
-          a:before { content: '🔗'; }
-          a { text-decoration: none; }
-        </style>
-        <p>A <a href="#" id="target">link</a> inside a block of text</p>
-      `).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(`
-        <style>
-          a:after { content: ""; }
-          a { text-decoration: none; }
-        </style>
-        <p>A <a href="#" id="target">link</a> inside a block of text</p>
-      `).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(`
-        <style>
-          a:after { content: none; position: absolute; }
-          a { text-decoration: none; }
-        </style>
-        <p>A <a href="#" id="target">link</a> inside a block of text</p>
-      `).actualNode;
-      const result = linkInBlockStyleCheck.call(checkContext, link);
-      assert.isFalse(result);
-    });
+    createPseudoTests('a', 'link');
+    createPseudoTests('span', 'ancestor inline element');
   });
 });