[incubator-annotator] 01/19: Fix normalizeRange edge case

[ge...@apache.org]

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 <ge...@treora.com>
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;