You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@annotator.apache.org by ge...@apache.org on 2021/06/04 18:36:19 UTC

[incubator-annotator] branch allow-node-as-scope created (now b943fa1)

This is an automated email from the ASF dual-hosted git repository.

gerben pushed a change to branch allow-node-as-scope
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git.


      at b943fa1  Make all functions also accept a Node as scope

This branch includes the following new commits:

     new b5d35c7  Make default scope of describeCss the whole document
     new 6a86dad  Make css matcher return an Element, not Range
     new ed9d1b9  Make css matcher also accept a Node as scope
     new b943fa1  Make all functions also accept a Node as scope

The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[incubator-annotator] 01/04: Make default scope of describeCss the whole document

Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gerben pushed a commit to branch allow-node-as-scope
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit b5d35c7c3f2fff825c6a787706dcb2e7f746e76e
Author: Gerben <ge...@treora.com>
AuthorDate: Fri Jun 4 19:02:47 2021 +0200

    Make default scope of describeCss the whole document
    
    Instead of just the body.
    
    Accordingly, allow any Node as scope, not only Elements.
---
 packages/dom/src/css.ts | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/dom/src/css.ts b/packages/dom/src/css.ts
index 1a62141..746486a 100644
--- a/packages/dom/src/css.ts
+++ b/packages/dom/src/css.ts
@@ -77,9 +77,9 @@ export function createCssSelectorMatcher(
 
 export async function describeCss(
   element: HTMLElement,
-  scope?: HTMLElement,
+  scope: Node = element.ownerDocument,
 ): Promise<CssSelector> {
-  const selector = optimalSelect(element, { root: scope ?? element.ownerDocument.body });
+  const selector = optimalSelect(element, { root: scope });
   return {
     type: 'CssSelector',
     value: selector,

[incubator-annotator] 04/04: Make all functions also accept a Node as scope

Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gerben pushed a commit to branch allow-node-as-scope
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit b943fa1be70ecb392b0fdf7d113800c024b69b74
Author: Gerben <ge...@treora.com>
AuthorDate: Fri Jun 4 20:05:31 2021 +0200

    Make all functions also accept a Node as scope
---
 packages/dom/src/range/match.ts            | 35 ++++++++++++++++--------------
 packages/dom/src/text-node-chunker.ts      | 11 ++++++----
 packages/dom/src/text-position/describe.ts | 25 ++++++++-------------
 packages/dom/src/text-position/match.ts    | 17 +++++----------
 packages/dom/src/text-quote/describe.ts    | 24 +++++++-------------
 packages/dom/src/text-quote/match.ts       | 15 +++++--------
 6 files changed, 53 insertions(+), 74 deletions(-)

diff --git a/packages/dom/src/range/match.ts b/packages/dom/src/range/match.ts
index 061eb75..98cf250 100644
--- a/packages/dom/src/range/match.ts
+++ b/packages/dom/src/range/match.ts
@@ -24,11 +24,11 @@ import type {
   Selector,
 } from '@apache-annotator/selector';
 import { ownerDocument } from '../owner-document';
+import { toRange } from '../range-node-conversion';
 import { cartesian } from './cartesian';
 
 /**
- * Find the range(s) corresponding to the given {@link
- * RangeSelector}.
+ * Find the range(s) corresponding to the given {@link RangeSelector}.
  *
  * As a RangeSelector itself nests two further selectors, one needs to pass a
  * `createMatcher` function that will be used to process those nested selectors.
@@ -36,13 +36,12 @@ import { cartesian } from './cartesian';
  * The function is curried, taking first the `createMatcher` function, then the
  * selector, and then the scope.
  *
- * As there may be multiple matches for a given selector, the matcher will
- * return an (async) generator that produces each match in the order they are
- * found in the text. If both its nested selectors produce multiple matches, the
- * RangeSelector matches each possible pair among those in which the order of
- * start and end are respected. *(Note this behaviour is a rather free
- * interpretation — the Web Annotation Data Model spec is silent about multiple
- * matches for RangeSelectors)*
+ * As there may be multiple matches for the start & end selectors, the resulting
+ * matcher will return an (async) iterable, that produces a match for each
+ * possible pair of matches of the nested selectors (except those where its end
+ * would precede its start). *(Note that this behaviour is a rather free
+ * interpretation of the Web Annotation Data Model spec, which is silent about
+ * the possibility of multiple matches for RangeSelectors)*
  *
  * @example
  * By using a matcher for {@link TextQuoteSelector}s, one
@@ -88,15 +87,15 @@ import { cartesian } from './cartesian';
  * ```
  *
  * @param createMatcher - The function used to process nested selectors.
- * @returns A function that, given a RangeSelector, creates a {@link
- * Matcher} function that applies it to a given {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range}
+ * @returns A function that, given a RangeSelector `selector`, creates a {@link
+ * Matcher} function that can apply it to a given `scope`.
  *
  * @public
  */
 export function makeCreateRangeSelectorMatcher(
-  createMatcher: <T extends Selector>(selector: T) => Matcher<Range, Range>,
-): (selector: RangeSelector) => Matcher<Range, Range> {
+  createMatcher: <T extends Selector, TMatch extends Node | Range>(selector: T)
+    => Matcher<Node | Range, TMatch>,
+): (selector: RangeSelector) => Matcher<Node | Range, Range> {
   return function createRangeSelectorMatcher(selector) {
     const startMatcher = createMatcher(selector.startSelector);
     const endMatcher = createMatcher(selector.endSelector);
@@ -107,10 +106,14 @@ export function makeCreateRangeSelectorMatcher(
 
       const pairs = cartesian(startMatches, endMatches);
 
-      for await (const [start, end] of pairs) {
-        const result = ownerDocument(scope).createRange();
+      for await (let [start, end] of pairs) {
+        start = toRange(start);
+        end = toRange(end);
 
+        const result = ownerDocument(scope).createRange();
         result.setStart(start.startContainer, start.startOffset);
+        // Note that a RangeSelector’s match *excludes* the endSelector’s match,
+        // hence we take the end’s startContainer & startOffset.
         result.setEnd(end.startContainer, end.startOffset);
 
         if (!result.collapsed) yield result;
diff --git a/packages/dom/src/text-node-chunker.ts b/packages/dom/src/text-node-chunker.ts
index dc916a6..6981d1c 100644
--- a/packages/dom/src/text-node-chunker.ts
+++ b/packages/dom/src/text-node-chunker.ts
@@ -21,6 +21,7 @@
 import type { Chunk, Chunker, ChunkRange } from '@apache-annotator/selector';
 import { normalizeRange } from './normalize-range';
 import { ownerDocument } from './owner-document';
+import { toRange } from './range-node-conversion';
 
 export interface PartialTextNode extends Chunk<string> {
   readonly node: Text;
@@ -44,6 +45,7 @@ export class OutOfScopeError extends TypeError {
 }
 
 export class TextNodeChunker implements Chunker<PartialTextNode> {
+  private scope: Range;
   private iter: NodeIterator;
 
   get currentChunk(): PartialTextNode {
@@ -116,13 +118,14 @@ export class TextNodeChunker implements Chunker<PartialTextNode> {
   /**
    * @param scope A Range that overlaps with at least one text node.
    */
-  constructor(private scope: Range) {
+  constructor(scope: Node | Range) {
+    this.scope = toRange(scope);
     this.iter = ownerDocument(scope).createNodeIterator(
-      scope.commonAncestorContainer,
+      this.scope.commonAncestorContainer,
       NodeFilter.SHOW_TEXT,
       {
-        acceptNode(node: Text) {
-          return scope.intersectsNode(node)
+        acceptNode: (node: Text) => {
+          return this.scope.intersectsNode(node)
             ? NodeFilter.FILTER_ACCEPT
             : NodeFilter.FILTER_REJECT;
         },
diff --git a/packages/dom/src/text-position/describe.ts b/packages/dom/src/text-position/describe.ts
index fd96680..74a0cb1 100644
--- a/packages/dom/src/text-position/describe.ts
+++ b/packages/dom/src/text-position/describe.ts
@@ -21,6 +21,7 @@
 import type { TextPositionSelector } from '@apache-annotator/selector';
 import { describeTextPosition as abstractDescribeTextPosition } from '@apache-annotator/selector';
 import { ownerDocument } from '../owner-document';
+import { toRange } from '../range-node-conversion';
 import { TextNodeChunker } from '../text-node-chunker';
 
 /**
@@ -45,32 +46,24 @@ import { TextNodeChunker } from '../text-node-chunker';
  * // }
  * ```
  *
- * @param range - The range of characters that the selector should describe
- * @param maybeScope - A {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range} that serves as the ‘document’ for purposes of finding occurrences
- * and determining prefix and suffix. Defaults to span the full Document
- * containing the range.
+ * @param range - The {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
+ * | Range} whose text content will be described.
+ * @param scope - A Node or Range that serves as the ‘document’ for purposes of
+ * finding occurrences and determining prefix and suffix. Defaults to span the
+ * full Document that contains the range.
  * @returns The selector describing the `range` relative to `scope`
  *
  * @public
  */
 export async function describeTextPosition(
   range: Range,
-  maybeScope?: Range,
+  scope?: Node | Range,
 ): Promise<TextPositionSelector> {
-  // Default to search in the whole document.
-  let scope: Range;
-  if (maybeScope !== undefined) {
-    scope = maybeScope;
-  } else {
-    const document = ownerDocument(range);
-    scope = document.createRange();
-    scope.selectNodeContents(document);
-  }
+  scope = toRange(scope ?? ownerDocument(range))
 
   const textChunks = new TextNodeChunker(scope);
   if (textChunks.currentChunk === null)
-    throw new RangeError('Range does not contain any Text nodes.');
+    throw new RangeError('Scope does not contain any Text nodes.');
 
   return await abstractDescribeTextPosition(
     textChunks.rangeToChunkRange(range),
diff --git a/packages/dom/src/text-position/match.ts b/packages/dom/src/text-position/match.ts
index 089def8..ac26caf 100644
--- a/packages/dom/src/text-position/match.ts
+++ b/packages/dom/src/text-position/match.ts
@@ -39,29 +39,22 @@ import { TextNodeChunker } from '../text-node-chunker';
  * @example
  * ```
  * const selector = { type: 'TextPositionSelector', start: 702, end: 736 };
- *
- * // Search in the whole document.
- * const scope = document.createRange();
- * scope.selectNodeContents(document);
- *
+ * const scope = document.body;
  * const matches = textQuoteSelectorMatcher(selector)(scope);
  * const match = (await matches.next()).value;
- *
  * // ⇒ Range { startContainer: #text, startOffset: 64, endContainer: #text,
  * //   endOffset: 98, … }
  * ```
  *
- * @param selector - The {@link TextPositionSelector}
- * to be anchored
- * @returns A {@link Matcher} function that applies
- * `selector` to a given {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range}
+ * @param selector - The {@link TextPositionSelector} to be anchored.
+ * @returns A {@link Matcher} function that applies `selector` within a given
+ * `scope`.
  *
  * @public
  */
 export function createTextPositionSelectorMatcher(
   selector: TextPositionSelector,
-): Matcher<Range, Range> {
+): Matcher<Node | Range, Range> {
   const abstractMatcher = abstractTextPositionSelectorMatcher(selector);
 
   return async function* matchAll(scope) {
diff --git a/packages/dom/src/text-quote/describe.ts b/packages/dom/src/text-quote/describe.ts
index 642070d..90fc022 100644
--- a/packages/dom/src/text-quote/describe.ts
+++ b/packages/dom/src/text-quote/describe.ts
@@ -24,6 +24,7 @@ import type {
 } from '@apache-annotator/selector';
 import { describeTextQuote as abstractDescribeTextQuote } from '@apache-annotator/selector';
 import { ownerDocument } from '../owner-document';
+import { toRange } from '../range-node-conversion';
 import { TextNodeChunker } from '../text-node-chunker';
 
 /**
@@ -52,10 +53,9 @@ import { TextNodeChunker } from '../text-node-chunker';
  *
  * @param range - The {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
  * | Range} whose text content will be described
- * @param maybeScope - A {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range} that serves as the ‘document’ for purposes of finding occurrences
- * and determining prefix and suffix. Defaults to span the full Document
- * containing the range.
+ * @param scope - A Node or Range that serves as the ‘document’ for purposes of
+ * finding occurrences and determining prefix and suffix. Defaults to span the
+ * full Document that contains the range.
  * @param options - Options to fine-tune the function’s behaviour.
  * @returns The selector unambiguously describing the `range` in `scope`.
  *
@@ -63,24 +63,16 @@ import { TextNodeChunker } from '../text-node-chunker';
  */
 export async function describeTextQuote(
   range: Range,
-  maybeScope?: Range,
+  scope?: Node | Range,
   options: DescribeTextQuoteOptions = {},
 ): Promise<TextQuoteSelector> {
-  // Default to search in the whole document.
-  let scope: Range;
-  if (maybeScope !== undefined) {
-    scope = maybeScope;
-  } else {
-    const document = ownerDocument(range);
-    scope = document.createRange();
-    scope.selectNodeContents(document);
-  }
+  const scopeAsRange = toRange(scope ?? ownerDocument(range));
 
-  const chunker = new TextNodeChunker(scope);
+  const chunker = new TextNodeChunker(scopeAsRange);
 
   return await abstractDescribeTextQuote(
     chunker.rangeToChunkRange(range),
-    () => new TextNodeChunker(scope),
+    () => new TextNodeChunker(scopeAsRange),
     options,
   );
 }
diff --git a/packages/dom/src/text-quote/match.ts b/packages/dom/src/text-quote/match.ts
index a923586..2b29df6 100644
--- a/packages/dom/src/text-quote/match.ts
+++ b/packages/dom/src/text-quote/match.ts
@@ -44,10 +44,7 @@ import { TextNodeChunker, EmptyScopeError } from '../text-node-chunker';
  * ```
  * // Find the word ‘banana’.
  * const selector = { type: 'TextQuoteSelector', exact: 'banana' };
- *
- * // Search in the document body.
- * const scope = document.createRange();
- * scope.selectNodeContents(document.body);
+ * const scope = document.body;
  *
  * // Read all matches.
  * const matches = textQuoteSelectorMatcher(selector)(scope);
@@ -58,17 +55,15 @@ import { TextNodeChunker, EmptyScopeError } from '../text-node-chunker';
  * //   endOffset: 637, … }
  * ```
  *
- * @param selector - The {@link TextQuoteSelector}
- * to be anchored
- * @returns a {@link Matcher} function that applies
- * `selector` to a given {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range}
+ * @param selector - The {@link TextQuoteSelector} to be anchored.
+ * @returns A {@link Matcher} function that applies `selector` within a given
+ * `scope`.
  *
  * @public
  */
 export function createTextQuoteSelectorMatcher(
   selector: TextQuoteSelector,
-): Matcher<Range, Range> {
+): Matcher<Node | Range, Range> {
   const abstractMatcher = abstractTextQuoteSelectorMatcher(selector);
 
   return async function* matchAll(scope) {

[incubator-annotator] 02/04: Make css matcher return an Element, not Range

Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gerben pushed a commit to branch allow-node-as-scope
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit 6a86dad175a8ef27f90a86eda0d0dce1cde02808
Author: Gerben <ge...@treora.com>
AuthorDate: Fri Jun 4 19:23:23 2021 +0200

    Make css matcher return an Element, not Range
---
 packages/dom/src/css.ts | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/packages/dom/src/css.ts b/packages/dom/src/css.ts
index 746486a..038698e 100644
--- a/packages/dom/src/css.ts
+++ b/packages/dom/src/css.ts
@@ -45,11 +45,6 @@ import { ownerDocument } from './owner-document';
  * > “If […] the user agent discovers multiple matching text sequences, then the
  * > selection SHOULD be treated as matching all of the matches.”
  *
- * Each matching element is returned as a {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range} surrounding that element. This in order to make its output reusable
- * as the scope for any subsequents selectors that {@link
- * Selector.refinedBy | refine} this CssSelector.
- *
  * @param selector - The {@link CssSelector} to be anchored
  * @returns A {@link Matcher} function that applies `selector` to a given {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
  * | Range}
@@ -58,7 +53,7 @@ import { ownerDocument } from './owner-document';
  */
 export function createCssSelectorMatcher(
   selector: CssSelector,
-): Matcher<Range, Range> {
+): Matcher<Range, Element> {
   return async function* matchAll(scope) {
     const document = ownerDocument(scope);
     for (const element of document.querySelectorAll(selector.value)) {
@@ -69,7 +64,7 @@ export function createCssSelectorMatcher(
         scope.isPointInRange(range.startContainer, range.startOffset) &&
         scope.isPointInRange(range.endContainer, range.endOffset)
       ) {
-        yield range;
+        yield element;
       }
     }
   };

[incubator-annotator] 03/04: Make css matcher also accept a Node as scope

Posted by ge...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gerben pushed a commit to branch allow-node-as-scope
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit ed9d1b9f33b9f232e8704fd00c48ecd7cb046ce7
Author: Gerben <ge...@treora.com>
AuthorDate: Fri Jun 4 19:38:39 2021 +0200

    Make css matcher also accept a Node as scope
---
 packages/dom/src/css.ts                            | 10 ++++----
 packages/dom/src/owner-document.ts                 | 17 ++++++++++---
 ...{owner-document.ts => range-node-conversion.ts} | 28 ++++++++++++++++++----
 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/packages/dom/src/css.ts b/packages/dom/src/css.ts
index 038698e..143da9e 100644
--- a/packages/dom/src/css.ts
+++ b/packages/dom/src/css.ts
@@ -21,6 +21,7 @@
 import optimalSelect from 'optimal-select';
 import type { CssSelector, Matcher } from '@apache-annotator/selector';
 import { ownerDocument } from './owner-document';
+import { toRange } from './range-node-conversion';
 
 /**
  * Find the elements corresponding to the given {@link
@@ -45,16 +46,17 @@ import { ownerDocument } from './owner-document';
  * > “If […] the user agent discovers multiple matching text sequences, then the
  * > selection SHOULD be treated as matching all of the matches.”
  *
- * @param selector - The {@link CssSelector} to be anchored
- * @returns A {@link Matcher} function that applies `selector` to a given {@link https://developer.mozilla.org/en-US/docs/Web/API/Range
- * | Range}
+ * @param selector - The {@link CssSelector} to be anchored.
+ * @returns A {@link Matcher} function that applies `selector` to a given
+ * `scope`.
  *
  * @public
  */
 export function createCssSelectorMatcher(
   selector: CssSelector,
-): Matcher<Range, Element> {
+): Matcher<Node | Range, Element> {
   return async function* matchAll(scope) {
+    scope = toRange(scope);
     const document = ownerDocument(scope);
     for (const element of document.querySelectorAll(selector.value)) {
       const range = document.createRange();
diff --git a/packages/dom/src/owner-document.ts b/packages/dom/src/owner-document.ts
index 1c0621e..fe7818a 100644
--- a/packages/dom/src/owner-document.ts
+++ b/packages/dom/src/owner-document.ts
@@ -18,8 +18,19 @@
  * under the License.
  */
 
-export function ownerDocument(range: Range): Document {
-  const { startContainer } = range;
+/**
+ * Get the ownerDocument for either a range or a node.
+ *
+ * @param nodeOrRange the node or range for which to get the owner document.
+ */
+export function ownerDocument(nodeOrRange: Node | Range): Document {
+  const node = isRange(nodeOrRange)
+    ? nodeOrRange.startContainer
+    : nodeOrRange;
   // node.ownerDocument is null iff node is itself a Document.
-  return startContainer.ownerDocument ?? (startContainer as Document);
+  return node.ownerDocument ?? (node as Document);
+}
+
+function isRange(nodeOrRange: Node | Range): nodeOrRange is Range {
+  return ('startContainer' in nodeOrRange);
 }
diff --git a/packages/dom/src/owner-document.ts b/packages/dom/src/range-node-conversion.ts
similarity index 53%
copy from packages/dom/src/owner-document.ts
copy to packages/dom/src/range-node-conversion.ts
index 1c0621e..fd62543 100644
--- a/packages/dom/src/owner-document.ts
+++ b/packages/dom/src/range-node-conversion.ts
@@ -18,8 +18,28 @@
  * under the License.
  */
 
-export function ownerDocument(range: Range): Document {
-  const { startContainer } = range;
-  // node.ownerDocument is null iff node is itself a Document.
-  return startContainer.ownerDocument ?? (startContainer as Document);
+import { ownerDocument } from "./owner-document";
+
+/**
+ * Returns a range that exactly selects the contents of the given node.
+ *
+ * This function is idempotent: If the given argument is already a range, it
+ * simply returns that range.
+ *
+ * @param nodeOrRange The node/range to convert to a range if it is not already
+ * a range.
+ */
+export function toRange(nodeOrRange: Node | Range): Range {
+  if (isRange(nodeOrRange)) {
+    return nodeOrRange;
+  } else {
+    const node = nodeOrRange;
+    const range = ownerDocument(node).createRange();
+    range.selectNodeContents(node);
+    return range;
+  }
+}
+
+function isRange(nodeOrRange: Node | Range): nodeOrRange is Range {
+  return ('startContainer' in nodeOrRange);
 }