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 2020/06/25 21:41:50 UTC

[incubator-annotator] branch improve-range-stuff created (now 9a765d1)

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

gerben pushed a change to branch improve-range-stuff
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git.


      at 9a765d1  WIP reimplement range iteration

This branch includes the following new commits:

     new 8800720  Support empty ranges in highlighter & demo
     new 462a3ab  Replace replaceChild with replaceWith
     new d09af67  Add failing test for range without Text nodes
     new 9a765d1  WIP reimplement range iteration

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] 02/04: Replace replaceChild with replaceWith

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

gerben pushed a commit to branch improve-range-stuff
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit 462a3abdcbaafb45dc647408acb77ecda1f18026
Author: Gerben <ge...@treora.com>
AuthorDate: Thu Jun 25 22:47:16 2020 +0200

    Replace replaceChild with replaceWith
    
    Or: In the function that invokes replaceChild, replace with replaceWith replaceChild.
---
 packages/dom/src/highlight-range.ts | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/packages/dom/src/highlight-range.ts b/packages/dom/src/highlight-range.ts
index 091c24f..8c52530 100644
--- a/packages/dom/src/highlight-range.ts
+++ b/packages/dom/src/highlight-range.ts
@@ -129,10 +129,7 @@ function removeHighlight(highlightElement: HTMLElement) {
   // If it has somehow been removed already, there is nothing to be done.
   if (!highlightElement.parentNode) return;
   if (highlightElement.childNodes.length === 1) {
-    highlightElement.parentNode.replaceChild(
-      highlightElement.firstChild as ChildNode,
-      highlightElement,
-    );
+    highlightElement.replaceWith(highlightElement.firstChild as Node);
   } else {
     // If the highlight somehow contains multiple nodes now, move them all.
     while (highlightElement.firstChild) {


[incubator-annotator] 01/04: Support empty ranges in highlighter & demo

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

gerben pushed a commit to branch improve-range-stuff
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit 8800720efcc99548341ee76d33d0db6988371e66
Author: Gerben <ge...@treora.com>
AuthorDate: Thu Jun 25 22:10:56 2020 +0200

    Support empty ranges in highlighter & demo
---
 packages/dom/src/highlight-range.ts |  3 ---
 web/demo/index.js                   | 18 ++++--------------
 web/style.css                       |  2 +-
 3 files changed, 5 insertions(+), 18 deletions(-)

diff --git a/packages/dom/src/highlight-range.ts b/packages/dom/src/highlight-range.ts
index e18a6a4..091c24f 100644
--- a/packages/dom/src/highlight-range.ts
+++ b/packages/dom/src/highlight-range.ts
@@ -55,9 +55,6 @@ export function highlightRange(
 
 // Return an array of the text nodes in the range. Split the start and end nodes if required.
 function textNodesInRange(range: Range): Text[] {
-  // If the range is empty, avoid creating and returning an empty text node.
-  if (range.collapsed) return [];
-
   // If the start or end node is a text node and only partly in the range, split it.
   if (
     isTextNode(range.startContainer) &&
diff --git a/web/demo/index.js b/web/demo/index.js
index 88b9a06..a1688b0 100644
--- a/web/demo/index.js
+++ b/web/demo/index.js
@@ -120,22 +120,12 @@ async function anchor(selector) {
   info.innerText = JSON.stringify(selector, null, 2);
 }
 
-async function describeSelection() {
+async function onSelectionChange() {
+  cleanup();
   const selection = document.getSelection();
-  if (selection.type !== 'Range') return;
-
   const range = selection.getRangeAt(0);
-  if (range.collapsed) return;
-
-  return describeTextQuote(range, source);
-}
-
-async function onSelectionChange() {
-  const selector = await describeSelection();
-  if (selector) {
-    cleanup();
-    anchor(selector);
-  }
+  const selector = await describeTextQuote(range, source);
+  anchor(selector);
 }
 
 function onSelectorExampleClick(event) {
diff --git a/web/style.css b/web/style.css
index 29fef4d..4ef867d 100644
--- a/web/style.css
+++ b/web/style.css
@@ -56,7 +56,7 @@ li {
 
 mark {
   background-color: rgba(255, 255, 0, 0.5);
-  outline: 0.1px solid rgba(255, 100, 0, 0.8);
+  outline: 1px solid rgba(255, 100, 0, 0.8);
 }
 
 .columns {


[incubator-annotator] 04/04: WIP reimplement range iteration

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

gerben pushed a commit to branch improve-range-stuff
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit 9a765d13760a7a77bf0284a6b66e2023b7f41a0d
Author: Gerben <ge...@treora.com>
AuthorDate: Thu Jun 25 23:40:54 2020 +0200

    WIP reimplement range iteration
---
 packages/dom/src/text-iterator.ts       | 64 +++++++++++++++++++++++++++++++++
 packages/dom/src/text-quote/describe.ts | 25 ++-----------
 2 files changed, 66 insertions(+), 23 deletions(-)

diff --git a/packages/dom/src/text-iterator.ts b/packages/dom/src/text-iterator.ts
new file mode 100644
index 0000000..53e8732
--- /dev/null
+++ b/packages/dom/src/text-iterator.ts
@@ -0,0 +1,64 @@
+/**
+ * @license
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+interface TextRange extends Range {
+  // We guarantee that to always have Text nodes as start and end containers.
+  readonly startContainer: Text;
+  readonly endContainer: Text;
+  cloneRange(): TextRange;
+
+  // Allow only Text nodes to be passed to these methods.
+  insertNode(node: Text): void;
+  selectNodeContents(node: Text): void;
+  setEnd(node: Text, offset: number): void;
+  setStart(node: Text, offset: number): void;
+
+  // Do not allow these methods to be used at all.
+  selectNode(node: never): void;
+  setEndAfter(node: never): void;
+  setEndBefore(node: never): void;
+  setStartAfter(node: never): void;
+  setStartBefore(node: never): void;
+  surroundContents(newParent: never): void;
+}
+
+function shrinkRangeToTextNodes(range: Range): TextRange {
+  // TODO walk to first & last text nodes inside the range.
+  return range as TextRange;
+}
+
+class TextIterator {
+  constructor(range: Range) {
+    const iter = document.createNodeIterator(
+      range.commonAncestorContainer,
+      NodeFilter.SHOW_TEXT,
+      {
+        acceptNode: node =>
+          range.intersectsNode(node)
+            ? NodeFilter.FILTER_ACCEPT
+            : NodeFilter.FILTER_REJECT
+      },
+    );
+
+    // Move to the start of the first text node (if any).
+    if (iter.nextNode())
+      iter.previousNode();
+  }
+}
diff --git a/packages/dom/src/text-quote/describe.ts b/packages/dom/src/text-quote/describe.ts
index 15749c9..0cc1760 100644
--- a/packages/dom/src/text-quote/describe.ts
+++ b/packages/dom/src/text-quote/describe.ts
@@ -148,29 +148,8 @@ function getRangeTextPosition(range: Range, scope: DomScope): number {
     },
   );
   const scopeOffset = isTextNode(scopeAsRange.startContainer) ? scopeAsRange.startOffset : 0;
-  if (isTextNode(range.startContainer))
-    return seek(iter, range.startContainer) + range.startOffset - scopeOffset;
-  else
-    return seek(iter, firstTextNodeInRange(range)) - scopeOffset;
-}
-
-function firstTextNodeInRange(range: Range): Text {
-  // Find the first text node inside the range.
-  const iter = document.createNodeIterator(
-    range.commonAncestorContainer,
-    NodeFilter.SHOW_TEXT,
-    {
-      acceptNode(node: Text) {
-        // Only reveal nodes within the range; and skip any empty text nodes.
-        return range.intersectsNode(node) && node.length > 0
-          ? NodeFilter.FILTER_ACCEPT
-          : NodeFilter.FILTER_REJECT
-      },
-    },
-  );
-  const node = iter.nextNode() as Text | null;
-  if (node === null) throw new Error('Range contains no text nodes');
-  return node;
+  const rangeOffset = isTextNode(range.startContainer) ? range.startOffset : 0;
+  return seek(iter, range.startContainer) + rangeOffset - scopeOffset;
 }
 
 function isTextNode(node: Node): node is Text {


[incubator-annotator] 03/04: Add failing test for range without Text nodes

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

gerben pushed a commit to branch improve-range-stuff
in repository https://gitbox.apache.org/repos/asf/incubator-annotator.git

commit d09af67eb1bbe22997564f2050a03f308d6a4862
Author: Gerben <ge...@treora.com>
AuthorDate: Thu Jun 25 23:38:11 2020 +0200

    Add failing test for range without Text nodes
---
 packages/dom/test/text-quote/describe.test.ts | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/packages/dom/test/text-quote/describe.test.ts b/packages/dom/test/text-quote/describe.test.ts
index f962157..50e39f0 100644
--- a/packages/dom/test/text-quote/describe.test.ts
+++ b/packages/dom/test/text-quote/describe.test.ts
@@ -72,6 +72,20 @@ describe('describeTextQuote', () => {
     assert.deepEqual(result, expected);
   });
 
+  it('works if range does not contain Text nodes', async () => {
+    const html = `<b>Try quoting this image: <img/> — would that work?</b>`
+    const doc = domParser.parseFromString(html, 'text/html');
+    const range = document.createRange();
+    range.selectNode(evaluateXPath(doc, '//img'));
+    const result = await describeTextQuote(range, doc);
+    assert.deepEqual(result, {
+      type: 'TextQuoteSelector',
+      exact: '',
+      prefix: ': ',
+      suffix: '',
+    });
+  })
+
   describe('inverts test cases of text quote matcher', () => {
     const applicableTestCases = Object.entries(testMatchCases)
       .filter(([_, { expected }]) => expected.length > 0);