Commit 0649e964 authored by Robert Knight's avatar Robert Knight

Remove logic to ignore Hypothesis UI when generating range selectors

Remove the logic that was intended to ignore Hypothesis UI when
generating range selectors.

We don't need this any more because all Hypothesis UI elements in the
host page which contain text have `user-select` set to prevent text
selection. This affects the bucket bar, vertical toolbar and adder.

In addition, the logic did not serve its intended purpose as it only applied to
range selector generation. If the user did somehow succeed in selecting text in
the bucket bar for example the client would still generate quote and position
selectors that referred to that content. What the client would need to
do instead is modify the range from which the annotation's selectors
were generated.

As a side effect this fixes a regression introduced in a5540581 which caused
range selector generation to fail in Chrome and Firefox due to use of
`:not(<inner selector>)` selector where `<inner selector>` is a
list of selectors (the `IGNORE_SELECTOR` value from `guest.js`).
parent 484b4f11
......@@ -6,17 +6,14 @@ import {
} from './xpath-util';
/**
* Return ancestors of `element`, optionally filtered by a CSS `selector`.
* Return ancestors of `node`.
*
* @param {Node} node
* @param {string} [selector]
*/
function parents(node, selector) {
function parents(node) {
const parents = [];
while (node.parentElement) {
if (!selector || node.parentElement.matches(selector)) {
parents.push(node.parentElement);
}
parents.push(node.parentElement);
node = node.parentElement;
}
return parents;
......@@ -169,13 +166,11 @@ export class BrowserRange {
* Creates a range suitable for storage.
*
* root - A root Element from which to anchor the serialization.
* ignoreSelector - A selector String of elements to ignore. For example
* elements injected by the annotator.
*
* Returns an instance of SerializedRange.
*/
serialize(root, ignoreSelector) {
return this.normalize().serialize(root, ignoreSelector);
serialize(root) {
return this.normalize().serialize(root);
}
}
......@@ -250,19 +245,12 @@ export class NormalizedRange {
* character offset), which can be easily stored in a database.
*
* root - The root Element relative to which XPaths should be calculated
* ignoreSelector - A selector String of elements to ignore. For example
* elements injected by the annotator.
*
* Returns an instance of SerializedRange.
*/
serialize(root, ignoreSelector) {
serialize(root) {
const serialization = (node, isEnd) => {
let origParent;
if (ignoreSelector) {
origParent = parents(node, `:not(${ignoreSelector})`)[0];
} else {
origParent = node.parentElement;
}
const origParent = node.parentElement;
const xpath = xpathFromNode(origParent, root ?? document);
const textNodes = getTextNodes(origParent);
// Calculate real offset as the combined length of all the
......@@ -433,13 +421,11 @@ export class SerializedRange {
* Creates a range suitable for storage.
*
* root - A root Element from which to anchor the serialization.
* ignoreSelector - A selector String of elements to ignore. For example
* elements injected by the annotator.
*
* Returns an instance of SerializedRange.
*/
serialize(root, ignoreSelector) {
return this.normalize(root).serialize(root, ignoreSelector);
serialize(root) {
return this.normalize(root).serialize(root);
}
// Returns the range as an Object literal.
......
......@@ -271,17 +271,6 @@ describe('annotator/anchoring/range', () => {
endOffset: 1,
});
});
it('converts BrowserRange to SerializedRange instance with `ignoreSelector` condition', () => {
const browserRange = createBrowserRange();
const result = browserRange.serialize(container, 'p');
assert.deepEqual(result, {
start: '/section[1]', // /p[1] selector in xpath ignored
startOffset: 0,
end: '/section[1]/span[1]', // /p[1] selector in xpath ignored
endOffset: 1,
});
});
});
});
......@@ -378,19 +367,6 @@ describe('annotator/anchoring/range', () => {
});
});
it('serialize the range with `ignoreSelector` condition', () => {
const serializedRange = createNormalizedRange().serialize(
container,
'#p-3'
);
assert.deepEqual(serializedRange, {
start: '/section[1]/p[1]',
end: '/section[1]/span[1]',
startOffset: 0,
endOffset: 6,
});
});
it('serialize the range with multiple text nodes', () => {
const serializedRange = createNormalizedRange({
start: container.querySelector('#p-2').firstChild.nextSibling
......@@ -511,15 +487,6 @@ describe('annotator/anchoring/range', () => {
// The copied instance shall be identical to the initial.
assert.deepEqual(result, serializedRange);
});
it('converts a SerializedRange to a new SerializedRange instance with `ignoreSelector` condition', () => {
const serializedRange = createSerializedRange();
const result = serializedRange.serialize(container, '#p-3');
// End xpath shall ignore the provided selector and not
// be identical to the initial end xpath.
assert.notEqual(serializedRange.end, result.end);
assert.equal(result.end, '/section[1]/span[1]');
});
});
});
});
......@@ -75,12 +75,10 @@ export class RangeAnchor {
}
/**
* @param {Object} [options]
* @param {string} [options.ignoreSelector]
* @return {RangeSelector}
*/
toSelector(options = {}) {
const range = this.range.serialize(this.root, options.ignoreSelector);
toSelector() {
const range = this.range.serialize(this.root);
return {
type: 'RangeSelector',
startContainer: range.start,
......
......@@ -63,13 +63,6 @@ function annotationsAt(node) {
return /** @type {AnnotationData[]} */ (items);
}
// A selector which matches elements added to the DOM by Hypothesis (eg. for
// highlights and annotation UI).
//
// We can simplify this once all classes are converted from an "annotator-"
// prefix to a "hypothesis-" prefix.
const IGNORE_SELECTOR = '[class^="annotator-"],[class^="hypothesis-"]';
export default class Guest extends Delegator {
constructor(element, config, anchoring = htmlAnchoring) {
const defaultConfig = {
......@@ -355,11 +348,8 @@ export default class Guest extends Delegator {
}
// Find a target using the anchoring module.
const options = {
ignoreSelector: IGNORE_SELECTOR,
};
return this.anchoring
.anchor(root, target.selector, options)
.anchor(root, target.selector)
.then(range => ({
annotation,
target,
......@@ -477,11 +467,8 @@ export default class Guest extends Delegator {
this.selectedRanges = null;
const getSelectors = range => {
const options = {
ignoreSelector: IGNORE_SELECTOR,
};
// Returns an array of selectors for the passed range.
return this.anchoring.describe(root, range, options);
return this.anchoring.describe(root, range);
};
const setDocumentInfo = info => {
......
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