fix(valid-lang): ignore lang on elements with no text (#3523)

* fix(valid-lang): ignore lang on elements with no text

* Delete broken / unnecessary test

* Fix failing tests

* fix broken test

* Address feedback

* Disable flakey IE test

* Make hasLangText part of valid-lang check

lib/checks/language/valid-lang-evaluate.js

@@ -1,5 +1,6 @@
 import { isValidLang, getBaseLang } from '../../core/utils';
 import { sanitize } from '../../commons/text';
+import { hasLangText } from '../../commons/dom'
 
 function validLangEvaluate(node, options, virtualNode) {
   const invalid = [];
@@ -25,12 +26,17 @@ function validLangEvaluate(node, options, virtualNode) {
     }
   });
 
-  if (invalid.length) {
-    this.data(invalid);
-    return true;
+  if (!invalid.length) {
+    return false;
   }
-
-  return false;
+  if ( // Except for `html`, ignore elements with no text
+    virtualNode.props.nodeName !== 'html' &&
+    !hasLangText(virtualNode)
+  ) {
+    return false;
+  }
+  this.data(invalid);
+  return true;
 }
 
 export default validLangEvaluate;

lib/commons/dom/has-content-virtual.js

@@ -2,25 +2,30 @@ import isVisualContent from './is-visual-content';
 import labelVirtual from '../aria/label-virtual';
 
 const hiddenTextElms = [
-  'HEAD',
-  'TITLE',
-  'TEMPLATE',
-  'SCRIPT',
-  'STYLE',
-  'IFRAME',
-  'OBJECT',
-  'VIDEO',
-  'AUDIO',
-  'NOSCRIPT'
+  'head',
+  'title',
+  'template',
+  'script',
+  'style',
+  'iframe',
+  'object',
+  'video',
+  'audio',
+  'noscript'
 ];
 
-function hasChildTextNodes(elm) {
-  if (!hiddenTextElms.includes(elm.actualNode.nodeName.toUpperCase())) {
-    return elm.children.some(
-      ({ actualNode }) =>
-        actualNode.nodeType === 3 && actualNode.nodeValue.trim()
-    );
+/**
+ * Test if the element has child nodes that are non-empty text nodes
+ * @param {VirtualNode} elm 
+ * @returns boolean
+ */
+export function hasChildTextNodes(elm) {
+  if (hiddenTextElms.includes(elm.props.nodeName)) {
+    return false
   }
+  return elm.children.some(({ props }) => {
+    return props.nodeType === 3 && props.nodeValue.trim()
+  })
 }
 
 /**

lib/commons/dom/has-lang-text.js

@@ -0,0 +1,24 @@
+import { hasChildTextNodes } from './has-content-virtual';
+import isVisualContent from './is-visual-content';
+import isVisible from './is-visible';
+
+/**
+ * Check that a node has text, or an accessible name which language is defined by the 
+ * nearest ancestor's lang attribute.
+ * @param {VirtualNode} virtualNode 
+ * @return boolean
+ */
+export default function hasLangText(virtualNode) {
+  if (typeof virtualNode.children === 'undefined' || hasChildTextNodes(virtualNode)) {
+    return true;
+  }
+  if (virtualNode.props.nodeType === 1 && isVisualContent(virtualNode)) {
+    // See: https://github.com/dequelabs/axe-core/issues/3281
+    return !!axe.commons.text.accessibleTextVirtual(virtualNode);
+  }
+  return virtualNode.children.some(child => (
+    !child.attr('lang') && // non-empty lang
+    hasLangText(child) && // has text
+    isVisible(child, true) // Not hidden for AT
+  ));
+}

lib/commons/dom/index.js

@@ -17,6 +17,7 @@ export { default as getTextElementStack } from './get-text-element-stack';
 export { default as getViewportSize } from './get-viewport-size';
 export { default as hasContentVirtual } from './has-content-virtual';
 export { default as hasContent } from './has-content';
+export { default as hasLangText } from './has-lang-text';
 export { default as idrefs } from './idrefs';
 export { default as insertedIntoFocusOrder } from './inserted-into-focus-order';
 export { default as isCurrentPageLink } from './is-current-page-link';

lib/commons/dom/is-visual-content.js

@@ -1,8 +1,13 @@
+import { getNodeFromTree } from '../../core/utils';
+import AbstractVirtualNode from '../../core/base/virtual-node/abstract-virtual-node';
+
 const visualRoles = [
   'checkbox',
   'img',
+  'meter',
+  'progressbar',
+  'scrollbar',
   'radio',
-  'range',
   'slider',
   'spinbutton',
   'textbox'
@@ -13,34 +18,34 @@ const visualRoles = [
  * @method isVisualContent
  * @memberof axe.commons.dom
  * @instance
- * @param  {Element} element The element to check
+ * @param  {Element|VirtualNode} element The element to check
  * @return {Boolean}
  */
-function isVisualContent(element) {
-  /*eslint indent: 0*/
-  const role = element.getAttribute('role');
+function isVisualContent(el) {
+  const vNode = el instanceof AbstractVirtualNode ? el : getNodeFromTree(el);
+  const role = axe.commons.aria.getExplicitRole(vNode);
   if (role) {
     return visualRoles.indexOf(role) !== -1;
   }
 
-  switch (element.nodeName.toUpperCase()) {
-    case 'IMG':
-    case 'IFRAME':
-    case 'OBJECT':
-    case 'VIDEO':
-    case 'AUDIO':
-    case 'CANVAS':
-    case 'SVG':
-    case 'MATH':
-    case 'BUTTON':
-    case 'SELECT':
-    case 'TEXTAREA':
-    case 'KEYGEN':
-    case 'PROGRESS':
-    case 'METER':
+  switch (vNode.props.nodeName) {
+    case 'img':
+    case 'iframe':
+    case 'object':
+    case 'video':
+    case 'audio':
+    case 'canvas':
+    case 'svg':
+    case 'math':
+    case 'button':
+    case 'select':
+    case 'textarea':
+    case 'keygen':
+    case 'progress':
+    case 'meter':
       return true;
-    case 'INPUT':
-      return element.type !== 'hidden';
+    case 'input':
+      return vNode.props.type !== 'hidden';
     default:
       return false;
   }

lib/rules/valid-lang.json

@@ -1,7 +1,6 @@
 {
   "id": "valid-lang",
-  "selector": "[lang], [xml\\:lang]",
-  "matches": "not-html-matches",
+  "selector": "[lang]:not(html), [xml\\:lang]:not(html)",
   "tags": ["cat.language", "wcag2aa", "wcag312", "ACT"],
   "actIds": ["de46e4"],
   "metadata": {

test/checks/language/valid-lang.js

@@ -12,55 +12,55 @@ describe('valid-lang', function() {
   });
 
   it('should return false if a lang attribute is present in options', function() {
-    var params = checkSetup('<div id="target" lang="woohoo"></div>', {
+    var params = checkSetup('<div id="target" lang="woohoo">text</div>', {
       value: ['blah', 'blah', 'woohoo']
     });
 
     assert.isFalse(validLangEvaluate.apply(checkContext, params));
   });
 
   it('should lowercase options and attribute first', function() {
-    var params = checkSetup('<div id="target" lang="wooHOo"></div>', {
+    var params = checkSetup('<div id="target" lang="wooHOo">text</div>', {
       value: ['blah', 'blah', 'wOohoo']
     });
 
     assert.isFalse(validLangEvaluate.apply(checkContext, params));
   });
 
   it('should return true if a lang attribute is not present in options', function() {
-    var params = checkSetup('<div id="target" lang="FOO"></div>');
+    var params = checkSetup('<div id="target" lang="FOO">text</div>');
 
     assert.isTrue(validLangEvaluate.apply(checkContext, params));
     assert.deepEqual(checkContext._data, ['lang="FOO"']);
   });
 
   it('should return false (and not throw) when given no present in options', function() {
-    var params = checkSetup('<div id="target" lang="en"></div>');
+    var params = checkSetup('<div id="target" lang="en">text</div>');
 
     assert.isFalse(validLangEvaluate.apply(checkContext, params));
   });
 
   it('should return true if the language is badly formatted', function() {
-    var params = checkSetup('<div id="target" lang="en_US"></div>');
+    var params = checkSetup('<div id="target" lang="en_US">text</div>');
 
     assert.isTrue(validLangEvaluate.apply(checkContext, params));
     assert.deepEqual(checkContext._data, ['lang="en_US"']);
   });
 
   it('should return false if it matches a substring proceeded by -', function() {
-    var params = checkSetup('<div id="target" lang="en-LOL"></div>');
+    var params = checkSetup('<div id="target" lang="en-LOL">text</div>');
 
     assert.isFalse(validLangEvaluate.apply(checkContext, params));
   });
 
   it('should work with xml:lang', function() {
-    var params = checkSetup('<div id="target" xml:lang="en-LOL"></div>');
+    var params = checkSetup('<div id="target" xml:lang="en-LOL">text</div>');
 
     assert.isFalse(validLangEvaluate.apply(checkContext, params));
   });
 
   it('should accept options.attributes', function() {
-    var params = checkSetup('<div id="target" custom-lang="en_US"></div>', {
+    var params = checkSetup('<div id="target" custom-lang="en_US">text</div>', {
       attributes: ['custom-lang']
     });
 
@@ -69,8 +69,15 @@ describe('valid-lang', function() {
   });
 
   it('should return true if lang value is just whitespace', function() {
-    var params = checkSetup('<div id="target" lang="  "></div>');
+    var params = checkSetup('<div id="target" lang="  ">text</div>');
 
     assert.isTrue(validLangEvaluate.apply(checkContext, params));
   });
+
+  it('should return false if a lang attribute element has no content', function() {
+    var params = checkSetup('<div id="target" lang="FOO"></div>');
+
+    assert.isFalse(validLangEvaluate.apply(checkContext, params));
+    assert.deepEqual(checkContext._data, null);
+  });
 });

test/commons/color/get-background-color.js

@@ -4,6 +4,7 @@ describe('color.getBackgroundColor', function() {
   var fixture = document.getElementById('fixture');
 
   var shadowSupported = axe.testUtils.shadowSupport.v1;
+  var isIE11 = axe.testUtils.isIE11;
   var origBodyBg;
   var origHtmlBg;
 
@@ -480,11 +481,11 @@ describe('color.getBackgroundColor', function() {
     assert.equal(actual.alpha, expected.alpha);
   });
 
-  it('handles nested inline elements in the middle of a text', function () {
+  it('handles nested inline elements in the middle of a text', function() {
     fixture.innerHTML =
       '<div style="height: 1em; overflow:auto; background: cyan">' +
       '  <br>' +
-      '  <b id="target">Text <i style="display: inline-block">'+
+      '  <b id="target">Text <i style="display: inline-block">' +
       '    <s><img width="100" height="16"></s>' +
       '  </i></b>' +
       '</div>';
@@ -499,7 +500,7 @@ describe('color.getBackgroundColor', function() {
     assert.equal(actual.alpha, 1);
   });
 
-  it('should return null for inline elements with position:absolute', function () {
+  it('should return null for inline elements with position:absolute', function() {
     fixture.innerHTML =
       '<div style="height: 1em; overflow:auto; position: relative">' +
       '  <br>' +
@@ -725,21 +726,25 @@ describe('color.getBackgroundColor', function() {
     assert.isNull(actual);
   });
 
-  it('should return background color for inline elements that do not fit the viewport', function() {
-    var html = '';
-    for (var i = 0; i < 300; i++) {
-      html += 'foo<br />';
-    }
-    fixture.innerHTML = '<em>' + html + '</em>';
-    axe.testUtils.flatTreeSetup(fixture);
-    var actual = axe.commons.color.getBackgroundColor(fixture, []);
-    var expected = new axe.commons.color.Color(255, 255, 255, 1);
+  // Test is flakey in IE11, timing out regularly
+  (isIE11 ? xit : it)(
+    'should return background color for inline elements that do not fit the viewport',
+    function() {
+      var html = '';
+      for (var i = 0; i < 300; i++) {
+        html += 'foo<br />';
+      }
+      fixture.innerHTML = '<em>' + html + '</em>';
+      axe.testUtils.flatTreeSetup(fixture);
+      var actual = axe.commons.color.getBackgroundColor(fixture, []);
+      var expected = new axe.commons.color.Color(255, 255, 255, 1);
 
-    assert.closeTo(actual.red, expected.red, 0.5);
-    assert.closeTo(actual.green, expected.green, 0.5);
-    assert.closeTo(actual.blue, expected.blue, 0.5);
-    assert.closeTo(actual.alpha, expected.alpha, 0.1);
-  });
+      assert.closeTo(actual.red, expected.red, 0.5);
+      assert.closeTo(actual.green, expected.green, 0.5);
+      assert.closeTo(actual.blue, expected.blue, 0.5);
+      assert.closeTo(actual.alpha, expected.alpha, 0.1);
+    }
+  );
 
   it('should return the body bgColor when content does not overlap', function() {
     fixture.innerHTML =