Skip to content

C
coopwire-hypothesis
Commit f2b25403 authored by Robert Knight's avatar Robert Knight
Browse files
Options

Simplify XPath generation code 

For reasons that AFAIK are no longer relevant, there were two
implementations of XPath generation for use in serializing range
selectors. Since we're removing jQuery from the Hypothesis client, this
removes the jQuery implementation and simplifies the remaining DOM-only
one.

 - Remove jQuery XPath generation

 - Change the non-jQuery implementation to take DOM Nodes as input
   rather than jQuery collections

 - Simplify and add type documentation for the remaining implementation

 - Correct the documentation for `xpathFromNode`. The previous comments
   said that it _evaluated_ XPaths but actually it _generates_ them.
parent 2414a08b
Hide whitespace changes
Inline Side-by-side
Showing with 88 additions and 215 deletions
src/annotator/anchoring/range.js
View file @ f2b25403
......@@ -249,7 +249,7 @@ export class NormalizedRange {
} else {
origParent = $(node).parent();
}
const xpath = xpathFromNode(origParent, root)[0];
const xpath = xpathFromNode(origParent[0], root ?? document);
const textNodes = getTextNodes(origParent);
// Calculate real offset as the combined length of all the
// preceding textNode siblings. We include the length of the
......
src/annotator/anchoring/test/xpath-test.js
View file @ f2b25403
import $ from 'jquery';
import { nodeFromXPath, xpathFromNode, $imports } from '../xpath';
import { nodeFromXPath } from '../xpath';
describe('annotator/anchoring/xpath', () => {
describe('xpathFromNode', () => {
let container;
let fakeSimpleXPathJQuery;
let fakeSimpleXPathPure;
beforeEach(() => {
container = document.createElement('div');
document.body.appendChild(container);
fakeSimpleXPathJQuery = sinon.stub().returns('/div[1]');
fakeSimpleXPathPure = sinon.stub().returns('/div[1]');
$imports.$mock({
'./xpath-util': {
simpleXPathJQuery: fakeSimpleXPathJQuery,
simpleXPathPure: fakeSimpleXPathPure,
},
});
});
afterEach(() => {
container.remove();
$imports.$restore();
});
it('calls `simpleXPathJQuery`', () => {
const xpath = xpathFromNode($(container), document.body);
assert.called(fakeSimpleXPathJQuery);
assert.equal(xpath, '/div[1]');
});
it('calls `simpleXPathPure` if `simpleXPathJQuery` throws an exception', () => {
sinon.stub(console, 'log');
fakeSimpleXPathJQuery.throws(new Error());
const xpath = xpathFromNode($(container), document.body);
assert.called(fakeSimpleXPathPure);
assert.equal(xpath, '/div[1]');
assert.isTrue(
// eslint-disable-next-line no-console
console.log.calledWith(
'jQuery-based XPath construction failed! Falling back to manual.'
)
);
// eslint-disable-next-line no-console
console.log.restore();
});
});
describe('nodeFromXPath', () => {
let container;
const html = `
......
src/annotator/anchoring/test/xpath-util-test.js
View file @ f2b25403
......@@ -5,8 +5,7 @@ import {
getTextNodes,
getLastTextNodeUpTo,
getFirstTextNodeNotBefore,
simpleXPathPure,
simpleXPathJQuery,
xpathFromNode,
} from '../xpath-util';
describe('annotator/anchoring/xpath-util', () => {
......@@ -175,7 +174,7 @@ describe('annotator/anchoring/xpath-util', () => {
});
});
describe('xpath', () => {
describe('xpathFromNode', () => {
let container;
const html = `
<h1 id="h1-1">text</h1>
......@@ -199,110 +198,70 @@ describe('annotator/anchoring/xpath-util', () => {
container.remove();
});
describe('xpathFromNode', () => {
[
{
id: 'a-1',
xpath: '/div[1]/p[1]/a[1]',
},
{
id: 'h1-1',
xpath: '/div[1]/h1[1]',
},
{
id: 'p-1',
xpath: '/div[1]/p[1]',
},
{
id: 'a-1',
xpath: '/div[1]/p[1]/a[1]',
},
{
id: 'p-2',
xpath: '/div[1]/p[2]',
},
{
id: 'em-1',
xpath: '/div[1]/p[2]/em[1]',
},
{
id: 'li-3',
xpath: '/div[1]/span[1]/ul[1]/li[3]',
},
].forEach(test => {
it('produces the correct xpath for the provided nodes', () => {
let node = document.getElementById(test.id);
assert.equal(simpleXPathJQuery($(node), document.body), test.xpath);
});
});
it('throws an error if the provided node is not a descendant of the root node', () => {
const node = document.createElement('p'); // not attached to DOM
assert.throws(() => {
xpathFromNode(node, document.body);
}, 'Node is not a descendant of root');
});
describe('simpleXPathPure', () => {
it('throws an error if the provided node is not a descendant of `relativeRoot`', () => {
const node = document.createElement('p'); // not attached to DOM
assert.throws(() => {
simpleXPathPure($(node), document.body);
}, 'Called getPathTo on a node which was not a descendant of @rootNode. [object HTMLBodyElement]');
[
{
id: 'a-1',
xpaths: ['/div[1]/p[1]/a[1]', '/div[1]/p[1]/a[1]/text()[1]'],
},
{
id: 'h1-1',
xpaths: ['/div[1]/h1[1]', '/div[1]/h1[1]/text()[1]'],
},
{
id: 'p-1',
xpaths: ['/div[1]/p[1]', '/div[1]/p[1]/text()[1]'],
},
{
id: 'a-1',
xpaths: ['/div[1]/p[1]/a[1]', '/div[1]/p[1]/a[1]/text()[1]'],
},
{
id: 'p-2',
xpaths: [
'/div[1]/p[2]',
'/div[1]/p[2]/text()[1]',
'/div[1]/p[2]/text()[2]',
],
},
{
id: 'em-1',
xpaths: ['/div[1]/p[2]/em[1]', '/div[1]/p[2]/em[1]/text()[1]'],
},
{
id: 'li-3',
xpaths: [
'/div[1]/span[1]/ul[1]/li[3]',
'/div[1]/span[1]/ul[1]/li[3]/text()[1]',
],
},
].forEach(test => {
it('produces the correct xpath for the provided node', () => {
let node = document.getElementById(test.id);
assert.equal(xpathFromNode(node, document.body), test.xpaths[0]);
});
[
{
id: 'a-1',
xpaths: ['/div[1]/p[1]/a[1]', '/div[1]/p[1]/a[1]/text()[1]'],
},
{
id: 'h1-1',
xpaths: ['/div[1]/h1[1]', '/div[1]/h1[1]/text()[1]'],
},
{
id: 'p-1',
xpaths: ['/div[1]/p[1]', '/div[1]/p[1]/text()[1]'],
},
{
id: 'a-1',
xpaths: ['/div[1]/p[1]/a[1]', '/div[1]/p[1]/a[1]/text()[1]'],
},
{
id: 'p-2',
xpaths: [
'/div[1]/p[2]',
'/div[1]/p[2]/text()[1]',
'/div[1]/p[2]/text()[2]',
],
},
{
id: 'em-1',
xpaths: ['/div[1]/p[2]/em[1]', '/div[1]/p[2]/em[1]/text()[1]'],
},
{
id: 'li-3',
xpaths: [
'/div[1]/span[1]/ul[1]/li[3]',
'/div[1]/span[1]/ul[1]/li[3]/text()[1]',
],
},
].forEach(test => {
it('produces the correct xpath for the provided node', () => {
let node = document.getElementById(test.id);
assert.equal(simpleXPathPure($(node), document.body), test.xpaths[0]);
});
it('produces the correct xpath for the provided text node(s)', () => {
let node = document.getElementById(test.id).firstChild;
// collect all text nodes after the target queried node.
const textNodes = [];
while (node) {
if (node.nodeType === Node.TEXT_NODE) {
textNodes.push(node);
}
node = node.nextSibling;
it('produces the correct xpath for the provided text node(s)', () => {
let node = document.getElementById(test.id).firstChild;
// collect all text nodes after the target queried node.
const textNodes = [];
while (node) {
if (node.nodeType === Node.TEXT_NODE) {
textNodes.push(node);
}
textNodes.forEach((node, index) => {
assert.equal(
simpleXPathPure($(node), document.body),
test.xpaths[index + 1]
);
});
node = node.nextSibling;
}
textNodes.forEach((node, index) => {
assert.equal(
xpathFromNode(node, document.body),
test.xpaths[index + 1]
);
});
});
});
......
src/annotator/anchoring/xpath-util.js
View file @ f2b25403
import $ from 'jquery';
/**
* Finds the child node associated with the provided index and
* type relative from a parent.
......@@ -25,6 +23,8 @@ export function findChild(parent, type, index) {
/**
* Get the node name for use in generating an xpath expression.
*
* @param {Node} node
*/
function getNodeName(node) {
const nodeName = node.nodeName.toLowerCase();
......@@ -37,9 +37,12 @@ function getNodeName(node) {
/**
* Get the index of the node as it appears in its parent's child list
*
* @param {Node} node
*/
function getNodePosition(node) {
let pos = 0;
/** @type {Node|null} */
let tmp = node;
while (tmp) {
if (tmp.nodeName === node.nodeName) {
......@@ -50,26 +53,6 @@ function getNodePosition(node) {
return pos;
}
/**
* A simple XPath evaluator using jQuery which can evaluate queries of
*
* @deprecated
*/
export function simpleXPathJQuery(nodes, relativeRoot) {
const paths = nodes.map((index, node) => {
let path = '';
let elem = node;
while (elem.nodeType === Node.ELEMENT_NODE && elem !== relativeRoot) {
let tagName = elem.tagName.replace(':', '\\:');
let idx = $(elem.parentNode).children(tagName).index(elem) + 1;
path = '/' + elem.tagName.toLowerCase() + `[${idx}]` + path;
elem = elem.parentNode;
}
return path;
});
return paths.get();
}
function getPathSegment(node) {
const name = getNodeName(node);
const pos = getNodePosition(node);
......@@ -77,31 +60,28 @@ function getPathSegment(node) {
}
/**
* A simple XPath evaluator using only standard DOM methods which can
* evaluate queries of the form /tag[index]/tag[index].
* A simple XPath generator using which can generate XPaths of the form
* /tag[index]/tag[index].
*
* @param {Node} node - The node to generate a path to
* @param {Node} root - Root node to which the returned path is relative
*/
export function simpleXPathPure(nodes, relativeRoot) {
let rootNode = relativeRoot;
export function xpathFromNode(node, root) {
let xpath = '';
function getPathTo(node) {
let xpath = '';
let elem = node;
while (elem !== rootNode) {
if (!elem) {
throw new Error(
'Called getPathTo on a node which was not a descendant of @rootNode. ' +
rootNode
);
}
xpath = getPathSegment(elem) + '/' + xpath;
elem = elem.parentNode;
/** @type {Node|null} */
let elem = node;
while (elem !== root) {
if (!elem) {
throw new Error('Node is not a descendant of root');
}
xpath = '/' + xpath;
xpath = xpath.replace(/\/$/, '');
return xpath;
xpath = getPathSegment(elem) + '/' + xpath;
elem = elem.parentNode;
}
const paths = nodes.map((index, node) => getPathTo(node));
return paths.get();
xpath = '/' + xpath;
xpath = xpath.replace(/\/$/, '');
return xpath;
}
/**
......
src/annotator/anchoring/xpath.js
View file @ f2b25403
import { findChild, simpleXPathJQuery, simpleXPathPure } from './xpath-util';
import { findChild } from './xpath-util';
/**
* Wrapper for simpleXPath. Attempts to call the jquery
* version, and if that excepts, then falls back to pure js
* version.
*/
export function xpathFromNode(el, relativeRoot) {
let result;
try {
result = simpleXPathJQuery(el, relativeRoot);
} catch (e) {
// eslint-disable-next-line no-console
console.log(
'jQuery-based XPath construction failed! Falling back to manual.'
);
result = simpleXPathPure(el, relativeRoot);
}
return result;
}
export { xpathFromNode } from './xpath-util';
/**
* Finds an element node using an XPath relative to the document root.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment