This is an automated email from the ASF dual-hosted git repository. gerben pushed a commit to branch import-dom-seek in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git
commit b972ddc5afee7bd021c066c79e7b17b6f215625f Author: Gerben <[email protected]> AuthorDate: Mon Nov 16 20:38:01 2020 +0100 Fix normalizeRange edge case A collapsed range add the start of the scope would end up, after being normalised, at the end of the text node just before the scope; which would not be shown by the chunker. The only solution I see is to tell the normalisation function about the scope. --- packages/dom/src/chunker.ts | 4 +--- packages/dom/src/normalize-range.ts | 22 +++++++++++++++++----- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/packages/dom/src/chunker.ts b/packages/dom/src/chunker.ts index 574b627..e636509 100644 --- a/packages/dom/src/chunker.ts +++ b/packages/dom/src/chunker.ts @@ -106,9 +106,7 @@ export class TextNodeChunker implements Chunker<PartialTextNode> { } rangeToChunkRange(range: Range): ChunkRange<PartialTextNode> { - const textRange = normalizeRange(range); - // FIXME: normalizeRange can mess up: a collapsed range at the very end of - // the chunker’s scope might move to the next text node outside the scope. + const textRange = normalizeRange(range, this.scope); const startChunk = this.nodeToChunk(textRange.startContainer); const startIndex = textRange.startOffset - startChunk.startOffset; diff --git a/packages/dom/src/normalize-range.ts b/packages/dom/src/normalize-range.ts index 0fece41..a4a758e 100644 --- a/packages/dom/src/normalize-range.ts +++ b/packages/dom/src/normalize-range.ts @@ -50,14 +50,24 @@ export interface TextRange extends Range { // // If there is no text between the start and end, they thus collapse onto one a // single position; and if there are multiple equivalent positions, it takes the -// first one. +// first one; or, if scope is passed, the first equivalent falling within scope. // // Note that if the given range does not contain non-empty text nodes, it will -// end up pointing at a text node outside of it (after it if possible, else -// before). If the document does not contain any text nodes, an error is thrown. -export function normalizeRange(range: Range): TextRange { +// end up pointing at a text node outside of it (before it if possible, else +// after). If the document does not contain any text nodes, an error is thrown. +export function normalizeRange(range: Range, scope?: Range): TextRange { const document = ownerDocument(range); - const walker = document.createTreeWalker(document, NodeFilter.SHOW_TEXT); + const walker = document.createTreeWalker( + document, + NodeFilter.SHOW_TEXT, + { + acceptNode(node: Text) { + return (!scope || scope.intersectsNode(node)) + ? NodeFilter.FILTER_ACCEPT + : NodeFilter.FILTER_REJECT; + }, + }, + ); let [ startContainer, startOffset ] = snapBoundaryPointToTextNode(range.startContainer, range.startOffset); @@ -69,6 +79,7 @@ export function normalizeRange(range: Range): TextRange { startOffset = 0; } + // Set the range’s start; note this might move its end too. range.setStart(startContainer, startOffset); let [ endContainer, endOffset ] = snapBoundaryPointToTextNode(range.endContainer, range.endOffset); @@ -81,6 +92,7 @@ export function normalizeRange(range: Range): TextRange { endOffset = endContainer.length; } + // Set the range’s end; note this might move its start too. range.setEnd(endContainer, endOffset); return range as TextRange;
