From 9e3ca45ef0378adabc659755775be137abc12818 Mon Sep 17 00:00:00 2001 From: Romain Deltour Date: Fri, 15 Dec 2017 17:29:07 +0100 Subject: [PATCH] feat: Improve generated selectors for namespaced elements in XHTML (#582) * feat(utils): add function `isXHTML` to test if a document node is XHTML * test(utils): add a test for `axe.utils.isXHTML` on an XHTML document * fix(getSelector): improve selectors for namespaced elements - by default, ensure the nodename is escaped - for XHTML documents, only use the local name Replaces #566 Closes #563 * test(getSelector): add a test for `axe.utils.getSelector` on a namespaced XHTML element --- lib/core/utils/get-selector.js | 13 +++++++--- lib/core/utils/is-xhtml.js | 16 ++++++++++++ test/core/utils/get-selector.js | 14 +++++++++++ test/core/utils/is-xhtml.js | 21 ++++++++++++++++ .../full/get-selector/get-selector.js | 12 +++++++++ .../full/get-selector/get-selector.xhtml | 25 +++++++++++++++++++ test/integration/full/is-xhtml/is-xhtml.js | 9 +++++++ test/integration/full/is-xhtml/is-xhtml.xhtml | 24 ++++++++++++++++++ 8 files changed, 130 insertions(+), 4 deletions(-) create mode 100644 lib/core/utils/is-xhtml.js create mode 100644 test/core/utils/is-xhtml.js create mode 100644 test/integration/full/get-selector/get-selector.js create mode 100644 test/integration/full/get-selector/get-selector.xhtml create mode 100644 test/integration/full/is-xhtml/is-xhtml.js create mode 100644 test/integration/full/is-xhtml/is-xhtml.xhtml diff --git a/lib/core/utils/get-selector.js b/lib/core/utils/get-selector.js index d94531910a..03b247690a 100644 --- a/lib/core/utils/get-selector.js +++ b/lib/core/utils/get-selector.js @@ -1,4 +1,5 @@ const escapeSelector = axe.utils.escapeSelector; +let isXHTML; function isUncommonClassName (className) { return ![ @@ -27,7 +28,7 @@ function getDistinctClassList (elm) { } const commonNodes = [ - 'div', 'span', 'p', + 'div', 'span', 'p', 'b', 'i', 'u', 'strong', 'em', 'h2', 'h3' ]; @@ -79,7 +80,6 @@ const createSelector = { // Get uncommon node names getUncommonElm (elm, { isCommonElm, isCustomElm, nodeName }) { if (!isCommonElm && !isCustomElm) { - nodeName = escapeSelector(nodeName); // Add [type] if nodeName is an input element if (nodeName === 'input' && elm.hasAttribute('type')) { nodeName += '[type="' + elm.type + '"]'; @@ -130,7 +130,12 @@ const createSelector = { * recognize the element by (IDs, aria roles, custom element names, etc.) */ function getElmFeatures (elm, featureCount) { - const nodeName = elm.nodeName.toLowerCase(); + if (typeof isXHTML === 'undefined') { + isXHTML = axe.utils.isXHTML(document); + } + const nodeName = escapeSelector(isXHTML? + elm.localName + :elm.nodeName.toLowerCase()); const classList = Array.from(elm.classList) || []; // Collect some props we need to build the selector const props = { @@ -174,7 +179,7 @@ function generateSelector (elm, options, doc) { let { isUnique = false } = options; const idSelector = createSelector.getElmId(elm); const { - featureCount = 2, + featureCount = 2, minDepth = 1, toRoot = false, childSelectors = [] diff --git a/lib/core/utils/is-xhtml.js b/lib/core/utils/is-xhtml.js new file mode 100644 index 0000000000..9e764982ed --- /dev/null +++ b/lib/core/utils/is-xhtml.js @@ -0,0 +1,16 @@ + +/** + * Determines if a document node is XHTML + * @method isXHTML + * @memberof axe.utils + * @instance + * @param {Node} doc a document node + * @return {Boolean} + */ +axe.utils.isXHTML = function (doc) { + 'use strict'; + if (!doc.createElement) { + return false; + } + return doc.createElement('A').localName === 'A'; +}; diff --git a/test/core/utils/get-selector.js b/test/core/utils/get-selector.js index d7eb4b89c4..540b77f2c0 100644 --- a/test/core/utils/get-selector.js +++ b/test/core/utils/get-selector.js @@ -194,6 +194,20 @@ describe('axe.utils.getSelector', function () { assert.equal(result[0], node); }); + it('should work on complex namespaced elements', function () { + fixture.innerHTML = '' + + 'x' + + '' + + 'x' + + '' + + ''; + var node = fixture.querySelector('m\\:ci'); + var sel = axe.utils.getSelector(node); + var result = document.querySelectorAll(sel); + assert.lengthOf(result, 1); + assert.equal(result[0], node); + }); + it('shouldn\'t fail if the node\'s parentNode doesnt have children, somehow (Firefox bug)', function () { var sel = axe.utils.getSelector({ nodeName: 'a', diff --git a/test/core/utils/is-xhtml.js b/test/core/utils/is-xhtml.js new file mode 100644 index 0000000000..abe78610aa --- /dev/null +++ b/test/core/utils/is-xhtml.js @@ -0,0 +1,21 @@ +describe('axe.utils.isXHTML', function () { + 'use strict'; + + it('should be a function', function () { + assert.isFunction(axe.utils.isXHTML); + }); + + it('should return true on any document that is XHTML', function () { + var doc = document.implementation.createDocument('http://www.w3.org/1999/xhtml', 'html', null); + assert.isTrue(axe.utils.isXHTML(doc)); + }); + + it('should return false on any document that is HTML', function () { + var doc = document.implementation.createHTMLDocument('Monkeys'); + assert.isFalse(axe.utils.isXHTML(doc)); + }); + + it('should return false on any document that is HTML - fixture', function () { + assert.isFalse(axe.utils.isXHTML(document)); + }); +}); diff --git a/test/integration/full/get-selector/get-selector.js b/test/integration/full/get-selector/get-selector.js new file mode 100644 index 0000000000..ab9eb1ef04 --- /dev/null +++ b/test/integration/full/get-selector/get-selector.js @@ -0,0 +1,12 @@ + +describe('axe.utils.getSelector', function () { + 'use strict'; + it('should work on namespaced elements', function () { + var fixture = document.querySelector('#fixture'); + var node = fixture.firstElementChild; + var sel = axe.utils.getSelector(node); + var result = document.querySelectorAll(sel); + assert.lengthOf(result, 1); + assert.equal(result[0], node); + }); +}); diff --git a/test/integration/full/get-selector/get-selector.xhtml b/test/integration/full/get-selector/get-selector.xhtml new file mode 100644 index 0000000000..d9139cb2e3 --- /dev/null +++ b/test/integration/full/get-selector/get-selector.xhtml @@ -0,0 +1,25 @@ + + + axe.utils.getSelector test + + + + + + + + +
+ +
+
+ + + + diff --git a/test/integration/full/is-xhtml/is-xhtml.js b/test/integration/full/is-xhtml/is-xhtml.js new file mode 100644 index 0000000000..1837bdc1ab --- /dev/null +++ b/test/integration/full/is-xhtml/is-xhtml.js @@ -0,0 +1,9 @@ + +describe('axe.utils.isXHTML', function () { + 'use strict'; + + it('should return true on any document that is XHTML', function () { + assert.isTrue(axe.utils.isXHTML(document)); + }); + +}); diff --git a/test/integration/full/is-xhtml/is-xhtml.xhtml b/test/integration/full/is-xhtml/is-xhtml.xhtml new file mode 100644 index 0000000000..c0b6e4f70a --- /dev/null +++ b/test/integration/full/is-xhtml/is-xhtml.xhtml @@ -0,0 +1,24 @@ + + + axe.utils.isXHTML test + + + + + + + + +
+
+
+ + + +