chore(text-block): suggest changes to PR #5000 (#5009)

While reviewing #5000 I found the approach wasn't working as I was
expecting. Playing around with examples I found a simpler way to do
this. IDK if this is everything we should do, but I think this gets us
close. I decided to put this in its own PR since this was too much code
to put into comments.

Author: WilcoFiers

lib/commons/dom/is-in-text-block.js

@@ -2,39 +2,11 @@ import getComposedParent from './get-composed-parent';
 import sanitize from '../text/sanitize';
 import { getNodeFromTree, nodeLookup } from '../../core/utils';
 import getRoleType from '../aria/get-role-type';
-import isFocusable from './is-focusable';
-
-function walkDomNode(node, functor) {
-  if (functor(node.actualNode) !== false) {
-    node.children.forEach(child => walkDomNode(child, functor));
-  }
-}
 
 const blockLike = ['block', 'list-item', 'table', 'flex', 'grid'];
 
 const inlineBlockLike = ['inline-block', 'inline-flex', 'inline-grid'];
 
-function isBlock(node) {
-  const { vNode } = nodeLookup(node);
-  const display = vNode.getComputedStylePropertyValue('display');
-  return blockLike.includes(display) || display.substr(0, 6) === 'table-';
-}
-
-function isInlineBlockLike(vNode) {
-  const display = vNode.getComputedStylePropertyValue('display');
-  return inlineBlockLike.includes(display);
-}
-
-function getBlockParent(node) {
-  // Find the closest parent
-  let parentBlock = getComposedParent(node);
-  while (parentBlock && !isBlock(parentBlock)) {
-    parentBlock = getComposedParent(parentBlock);
-  }
-
-  return getNodeFromTree(parentBlock);
-}
-
 /**
  * Determines if an element is within a text block
  * With `noLengthCompare` true, will return if there is any non-space text outside
@@ -43,62 +15,21 @@ function getBlockParent(node) {
  * @param  {Element} node [description]
  * @param  {Object} options Optional
  * @property {Bool} noLengthCompare
- * @property {Bool} permissive
+ * @property {Bool} includeInlineBlock
  * @return {Boolean}      [description]
  */
-function isInTextBlock(node, { noLengthCompare, permissive = false } = {}) {
+function isInTextBlock(
+  node,
+  { noLengthCompare, includeInlineBlock = false } = {}
+) {
   const { vNode, domNode } = nodeLookup(node);
-
-  if (isBlock(domNode)) {
-    // Ignore if the link is a block
+  if (isBlock(domNode) || (!includeInlineBlock && isInlineBlockLike(vNode))) {
+    // Ignore if the element is a block
     return false;
   }
 
   // Find all the text part of the parent block not in a link, and all the text in a link
   const virtualParent = getBlockParent(domNode);
-
-  // if the element is a block-like element, look at the siblings of the node and if any
-  // are inline or text nodes then return the desired permissive option value so the
-  // caller determine what to do with them.
-  // @see https://github.com/dequelabs/axe-core/issues/4392
-  if (isInlineBlockLike(vNode)) {
-    // widget parents should be ignored
-    if (getRoleType(virtualParent) === 'widget' || isFocusable(virtualParent)) {
-      return false;
-    }
-
-    for (const siblingVNode of virtualParent.children) {
-      if (
-        siblingVNode.actualNode === node ||
-        // BR and HR elements break the line
-        ['br', 'hr'].includes(siblingVNode.props.nodeName)
-      ) {
-        continue;
-      }
-
-      // if there is a widget as a sibling we break out of the loop as we won't count two
-      // widgets as being inline
-      if (getRoleType(siblingVNode) === 'widget') {
-        break;
-      }
-
-      if (siblingVNode.props.nodeType === window.Node.TEXT_NODE) {
-        if (sanitize(siblingVNode.props.nodeValue)) {
-          return permissive;
-        }
-
-        continue;
-      }
-
-      const display = siblingVNode.getComputedStylePropertyValue('display');
-      if (display === 'inline') {
-        return permissive;
-      }
-    }
-
-    return false;
-  }
-
   let parentText = '';
   let widgetText = '';
   let inBrBlock = 0;
@@ -160,3 +91,30 @@ function isInTextBlock(node, { noLengthCompare, permissive = false } = {}) {
 }
 
 export default isInTextBlock;
+
+function isBlock(node) {
+  const { vNode } = nodeLookup(node);
+  const display = vNode.getComputedStylePropertyValue('display');
+  return blockLike.includes(display) || display.substr(0, 6) === 'table-';
+}
+
+function isInlineBlockLike(vNode) {
+  const display = vNode.getComputedStylePropertyValue('display');
+  return inlineBlockLike.includes(display);
+}
+
+function walkDomNode(node, functor) {
+  if (functor(node.actualNode) !== false) {
+    node.children.forEach(child => walkDomNode(child, functor));
+  }
+}
+
+function getBlockParent(node) {
+  // Find the closest parent
+  let parentBlock = getComposedParent(node);
+  while (parentBlock && !isBlock(parentBlock)) {
+    parentBlock = getComposedParent(parentBlock);
+  }
+
+  return getNodeFromTree(parentBlock);
+}

test/commons/dom/is-in-text-block.js

@@ -315,11 +315,7 @@ describe('dom.isInTextBlock', () => {
     it('returns true if there is any text outside the link', () => {
       fixtureSetup('<p>amy <a href="" id="link">link text is longer</a></p>');
       const link = document.getElementById('link');
-      assert.isTrue(
-        isInTextBlock(link, {
-          noLengthCompare: true
-        })
-      );
+      assert.isTrue(isInTextBlock(link, { noLengthCompare: true }));
     });
 
     it('returns false if the non-widget text is only whitespace', () => {
@@ -332,33 +328,73 @@ describe('dom.isInTextBlock', () => {
           '</p>'
       );
       const link = document.getElementById('link');
-      assert.isFalse(
-        isInTextBlock(link, {
-          noLengthCompare: true
-        })
-      );
+      assert.isFalse(isInTextBlock(link, { noLengthCompare: true }));
     });
   });
 
-  describe('options.permissive', () => {
-    it('returns options.permissive if inline-block element has text sibling', () => {
+  describe('with options.permissive: true', () => {
+    it('returns true if inline-block element has text sibling', () => {
       const target = queryFixture(`
         <div>
           Hello world
           <button id="target">button</button>
         </div>
       `);
-      assert.isTrue(isInTextBlock(target, { permissive: true }));
+      assert.isTrue(isInTextBlock(target, { includeInlineBlock: true }));
     });
 
-    it('returns options.permissive if inline-block element has inline element sibling', () => {
+    it('returns true if inline-block element has inline element sibling', () => {
       const target = queryFixture(`
         <div>
           <span>Hello world</span>
           <button id="target">button</button>
         </div>
       `);
-      assert.isTrue(isInTextBlock(target, { permissive: true }));
+      assert.isTrue(isInTextBlock(target, { includeInlineBlock: true }));
+    });
+
+    it('returns true if inline-block element has text sibling after it', () => {
+      const target = queryFixture(`
+        <div>
+          <button id="target">button</button> hello world
+        </div>
+      `);
+      assert.isTrue(isInTextBlock(target, { includeInlineBlock: true }));
+    });
+
+    it('returns true if inline-block element has both inline text and a widget sibling', () => {
+      const target = queryFixture(`
+        <div>
+          <button>button 1</button>
+          <span>Hello world, goodbye mars</span>
+          <button id="target">button 2</button>
+        </div>
+      `);
+      assert.isTrue(isInTextBlock(target, { includeInlineBlock: true }));
+    });
+
+    it('returns false the inline text sibling is on a different line', () => {
+      const target = queryFixture(`
+        <div>
+          Hello
+          <br>
+          <button id="target">button</button>
+          <hr>
+          world
+        </div>
+      `);
+      assert.isFalse(isInTextBlock(target, { includeInlineBlock: true }));
+    });
+
+    it('returns true if inline-block element has a sibling on the same line', () => {
+      const target = queryFixture(`
+        <div>
+          Hello
+          <br>
+          world <button id="target">button 2</button>
+        </div>
+      `);
+      assert.isFalse(isInTextBlock(target, { includeInlineBlock: true }));
     });
   });
 });