Commit ea7f5930 authored by Robert Knight's avatar Robert Knight

Allow integrations to indicate whether text can be annotated

Add a `canAnnotate` method to the `Integration` interface to indicate
whether a range is part of the annotatable content of the document. This
allows integrations to signal whether the Annotate/Highlight toolbar
should be shown for a given selection.

Implement it for PDFs by returning `true` if the range is part of the
text layer of a single page. Implement it for HTML documents by always
returning true.

Fixes #3790
parent 924a7b46
......@@ -568,17 +568,14 @@ export async function anchor(root, selectors) {
}
/**
* Convert a DOM Range object into a set of selectors.
* Prepare a DOM range for generating selectors and find the containing text layer.
*
* Converts a DOM `Range` object into a `[position, quote]` tuple of selectors
* which can be saved with an annotation and later passed to `anchor` to
* convert the selectors back to a `Range`.
* Throws if the range cannot be annotated.
*
* @param {HTMLElement} root - The root element
* @param {Range} range
* @return {Promise<Selector[]>}
* @return {[Range, Element]}
*/
export async function describe(root, range) {
function getTextLayerForRange(range) {
// "Shrink" the range so that the start and endpoints are at offsets within
// text nodes rather than any containing nodes.
try {
......@@ -598,18 +595,52 @@ export async function describe(root, range) {
throw new Error('Selecting across page breaks is not supported');
}
return [range, startTextLayer];
}
/**
* Return true if selectors can be generated for a range using `describe`.
*
* This function is faster than calling `describe` if the selectors are not
* required.
*
* @param {Range} range
*/
export function canDescribe(range) {
try {
getTextLayerForRange(range);
return true;
} catch {
return false;
}
}
/**
* Convert a DOM Range object into a set of selectors.
*
* Converts a DOM `Range` object into a `[position, quote]` tuple of selectors
* which can be saved with an annotation and later passed to `anchor` to
* convert the selectors back to a `Range`.
*
* @param {HTMLElement} root - The root element
* @param {Range} range
* @return {Promise<Selector[]>}
*/
export async function describe(root, range) {
const [textRange, textLayer] = getTextLayerForRange(range);
const startPos = TextPosition.fromPoint(
range.startContainer,
range.startOffset
).relativeTo(startTextLayer);
textRange.startContainer,
textRange.startOffset
).relativeTo(textLayer);
const endPos = TextPosition.fromPoint(
range.endContainer,
range.endOffset
).relativeTo(endTextLayer);
textRange.endContainer,
textRange.endOffset
).relativeTo(textLayer);
const startPageIndex = getSiblingIndex(
/** @type {Node} */ (startTextLayer.parentNode)
/** @type {Node} */ (textLayer.parentNode)
);
const pageOffset = await getPageOffset(startPageIndex);
......@@ -620,7 +651,7 @@ export async function describe(root, range) {
end: pageOffset + endPos.offset,
};
const quote = TextQuoteAnchor.fromRange(root, range).toSelector();
const quote = TextQuoteAnchor.fromRange(root, textRange).toSelector();
return [position, quote];
}
......
......@@ -90,7 +90,7 @@ describe('annotator/anchoring/pdf', () => {
container.remove();
});
describe('#describe', () => {
describe('describe', () => {
it('returns position and quote selectors', () => {
viewer.pdfViewer.setCurrentPage(2);
const range = findText(container, 'Netherfield Park');
......@@ -234,7 +234,50 @@ describe('annotator/anchoring/pdf', () => {
});
});
describe('#anchor', () => {
describe('canDescribe', () => {
it('returns true if range is in text layer', () => {
viewer.pdfViewer.setCurrentPage(2);
const range = findText(container, 'Netherfield Park');
assert.isTrue(pdfAnchoring.canDescribe(range));
});
// nb. These tests should correspond to the situations where `describe` throws.
it('returns false if range does not contain any text', () => {
viewer.pdfViewer.setCurrentPage(2, 3);
const range = new Range();
const el = document.createElement('div');
range.setStart(el, 0);
range.setEnd(el, 0);
assert.isFalse(pdfAnchoring.canDescribe(range));
});
it('returns false if range spans multiple pages', () => {
viewer.pdfViewer.setCurrentPage(2, 3);
const firstPageRange = findText(container, 'occupied again?');
const secondPageRange = findText(container, 'NODE A');
const range = new Range();
range.setStart(firstPageRange.startContainer, firstPageRange.startOffset);
range.setEnd(secondPageRange.startContainer, secondPageRange.endOffset);
assert.isFalse(pdfAnchoring.canDescribe(range));
});
it('returns false if range is outside text layer', () => {
viewer.pdfViewer.setCurrentPage(2, 3);
const range = new Range();
const el = document.createElement('div');
el.append('foobar');
range.setStart(el.firstChild, 0);
range.setEnd(el.firstChild, 6);
assert.isFalse(pdfAnchoring.canDescribe(range));
});
});
describe('anchor', () => {
it('anchors previously created selectors if the page is rendered', () => {
viewer.pdfViewer.setCurrentPage(2);
const range = findText(container, 'My dear Mr. Bennet');
......
......@@ -545,6 +545,11 @@ export default class Guest {
* @param {Range} range
*/
_onSelection(range) {
if (!this._integration.canAnnotate(range)) {
this._onClearSelection();
return;
}
const selection = /** @type {Selection} */ (document.getSelection());
const isBackwards = rangeUtil.isSelectionBackwards(selection);
const focusRect = rangeUtil.selectionFocusRect(selection);
......
......@@ -26,6 +26,10 @@ export class HTMLIntegration {
this._htmlMeta = new HTMLMetadata();
}
canAnnotate() {
return true;
}
destroy() {
// There is nothing to do here yet.
}
......
......@@ -5,6 +5,7 @@ import { ListenerCollection } from '../../shared/listener-collection';
import {
RenderingStates,
anchor,
canDescribe,
describe,
documentHasText,
} from '../anchoring/pdf';
......@@ -167,6 +168,15 @@ export class PDFIntegration {
return anchor(root, selectors);
}
/**
* Return true if the text in a range lies within the text layer of a PDF.
*
* @param {Range} range
*/
canAnnotate(range) {
return canDescribe(range);
}
/**
* Generate selectors for the text in `range`.
*
......
......@@ -36,6 +36,14 @@ describe('HTMLIntegration', () => {
assert.equal(integration.describe, fakeHTMLAnchoring.describe);
});
describe('#canAnnotate', () => {
it('is always true', () => {
const integration = new HTMLIntegration();
const range = new Range();
assert.isTrue(integration.canAnnotate(range));
});
});
describe('#contentContainer', () => {
it('returns body by default', () => {
const integration = new HTMLIntegration();
......
......@@ -71,6 +71,7 @@ describe('annotator/integrations/pdf', () => {
fakePDFAnchoring = {
RenderingStates,
anchor: sinon.stub(),
canDescribe: sinon.stub().returns(true),
describe: sinon.stub(),
documentHasText: sinon.stub().resolves(true),
};
......@@ -177,6 +178,14 @@ describe('annotator/integrations/pdf', () => {
});
});
describe('#canAnnotate', () => {
it('checks if range is in text layer of PDF', () => {
const range = new Range();
assert.equal(pdfIntegration.canAnnotate(range), true);
assert.calledWith(fakePDFAnchoring.canDescribe, range);
});
});
describe('#describe', () => {
it('generates selectors for passed range', async () => {
pdfIntegration = createPDFIntegration();
......
......@@ -80,6 +80,7 @@ describe('Guest', () => {
fakeIntegration = {
anchor: sinon.stub(),
canAnnotate: sinon.stub().returns(true),
contentContainer: sinon.stub().returns({}),
describe: sinon.stub(),
destroy: sinon.stub(),
......@@ -594,6 +595,17 @@ describe('Guest', () => {
assert.called(FakeAdder.instance.hide);
});
it('hides the adder if the integration indicates that the selection cannot be annotated', () => {
// Simulate integration indicating text is not part of annotatable content
// (eg. text that is part of the PDF.js UI)
fakeIntegration.canAnnotate.returns(false);
createGuest();
simulateSelectionWithText();
assert.notCalled(FakeAdder.instance.show);
});
it('emits `hasSelectionChanged` event with argument `true` if selection is non-empty', () => {
const guest = createGuest();
const callback = sandbox.stub();
......
......@@ -90,6 +90,9 @@
* of supporting a specific document type (web page, PDF, ebook, etc.).
*
* @typedef IntegrationBase
* @prop {(range: Range) => boolean} canAnnotate -
* Return whether the specified DOM range is part of the annotatable content
* of the current document.
* @prop {(root: HTMLElement, selectors: Selector[]) => Promise<Range>} anchor -
* Attempt to resolve a set of serialized selectors to the corresponding content in the
* current document.
......
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