Use an RPC call instead of direct call to `Guest#selectAnnotations`

We want the `BucketBar` to be able to communicate with `Guest`s that are
in other iframes, like in the Ebook scenario. Currently, the `BucketBar`
has an instance of the `Guest`, associated with `host` frame, and
assumes to be the main `Guest`, for communication purposes.

I have replaced the reliance of the direct invocation of
`Guest#selectAnnotations` by a new `selectAnnotations` RPC event in the
`guest-host` communication channel. This has the advantage of making no
assumptions about which iframe contains the annotations.

I have made some minor modification in the types and functionality of
`range-util`.

src/annotator/adder.js

@@ -62,9 +62,9 @@ function nearestPositionedAncestor(el) {
 
 /**
  * @typedef AdderOptions
- * @prop {() => any} onAnnotate - Callback invoked when "Annotate" button is clicked
- * @prop {() => any} onHighlight - Callback invoked when "Highlight" button is clicked
- * @prop {(annotations: object[]) => any} onShowAnnotations -
+ * @prop {() => void} onAnnotate - Callback invoked when "Annotate" button is clicked
+ * @prop {() => void} onHighlight - Callback invoked when "Highlight" button is clicked
+ * @prop {(tags: string[]) => void} onShowAnnotations -
  *   Callback invoked when  "Show" button is clicked
  *
  * @typedef {import('../types/annotator').Destroyable} Destroyable
@@ -126,11 +126,11 @@ export class Adder {
     this._onShowAnnotations = options.onShowAnnotations;
 
     /**
-     * Annotation objects associated with the current selection. If non-empty,
+     * Annotation tags associated with the current selection. If non-empty,
      * a "Show" button appears in the toolbar. Clicking the button calls the
      * `onShowAnnotations` callback with the current value of `annotationsForSelection`.
      *
-     * @type {object[]}
+     * @type {string[]}
      */
     this.annotationsForSelection = [];
 

src/annotator/bucket-bar.js

@@ -16,13 +16,16 @@ import { anchorBuckets } from './util/buckets';
 export default class BucketBar {
   /**
    * @param {HTMLElement} container
-   * @param {Pick<import('./guest').default, 'anchors'|'scrollToAnchor'|'selectAnnotations'>} guest
+   * @param {Pick<import('./guest').default, 'anchors'|'scrollToAnchor'>} guest
+   * @param {object} options
+   *   @param {(tags: string[], toggle: boolean) => void} options.onSelectAnnotations
    */
-  constructor(container, guest) {
+  constructor(container, guest, { onSelectAnnotations }) {
     this._bucketsContainer = document.createElement('div');
     container.appendChild(this._bucketsContainer);
 
     this._guest = guest;
+    this._onSelectAnnotations = onSelectAnnotations;
 
     // Immediately render the buckets for the current anchors.
     this.update();
@@ -40,8 +43,8 @@ export default class BucketBar {
         above={buckets.above}
         below={buckets.below}
         buckets={buckets.buckets}
-        onSelectAnnotations={(annotations, toggle) =>
-          this._guest.selectAnnotations(annotations, toggle)
+        onSelectAnnotations={(tags, toogle) =>
+          this._onSelectAnnotations(tags, toogle)
         }
         scrollToAnchor={anchor => this._guest.scrollToAnchor(anchor)}
       />,

src/annotator/components/Buckets.js

@@ -15,14 +15,14 @@ import { findClosestOffscreenAnchor } from '../util/buckets';
  *
  * @param {object} props
  *  @param {Bucket} props.bucket
- *  @param {(annotations: AnnotationData[], toggle: boolean) => any} props.onSelectAnnotations
+ *  @param {(tags: string[], toggle: boolean) => void} props.onSelectAnnotations
  */
 function BucketButton({ bucket, onSelectAnnotations }) {
-  const annotations = bucket.anchors.map(anchor => anchor.annotation);
   const buttonTitle = `Select nearby annotations (${bucket.anchors.length})`;
 
   function selectAnnotations(event) {
-    onSelectAnnotations(annotations, event.metaKey || event.ctrlKey);
+    const tags = bucket.anchors.map(anchor => anchor.annotation.$tag);
+    onSelectAnnotations(tags, event.metaKey || event.ctrlKey);
   }
 
   function setFocus(focusState) {
@@ -86,7 +86,7 @@ function NavigationBucketButton({ bucket, direction, scrollToAnchor }) {
  *   @param {Bucket} props.above
  *   @param {Bucket} props.below
  *   @param {Bucket[]} props.buckets
- *   @param {(annotations: AnnotationData[], toggle: boolean) => any} props.onSelectAnnotations
+ *   @param {(tags: string[], toggle: boolean) => void} props.onSelectAnnotations
  *   @param {(a: Anchor) => void} props.scrollToAnchor - Callback invoked to
  *     scroll the document to a given anchor
  */

src/annotator/components/test/Buckets-test.js

@@ -198,8 +198,8 @@ describe('Buckets', () => {
       assert.calledOnce(fakeOnSelectAnnotations);
       const call = fakeOnSelectAnnotations.getCall(0);
       assert.deepEqual(call.args[0], [
-        fakeBuckets[0].anchors[0].annotation,
-        fakeBuckets[0].anchors[1].annotation,
+        fakeBuckets[0].anchors[0].annotation.$tag,
+        fakeBuckets[0].anchors[1].annotation.$tag,
       ]);
       assert.equal(call.args[1], false);
     });

src/annotator/guest.js

@@ -40,34 +40,33 @@ import { normalizeURI } from './util/url';
  */
 
 /**
- * Return all the annotations associated with the selected text.
+ * Return all the annotations tags associated with the selected text.
  *
- * @return {AnnotationData[]}
+ * @return {string[]}
  */
 function annotationsForSelection() {
   const selection = /** @type {Selection} */ (window.getSelection());
   const range = selection.getRangeAt(0);
-  const items = rangeUtil.itemsForRange(
+  const tags = rangeUtil.itemsForRange(
     range,
-
-    // nb. Only non-nullish items are returned by `itemsForRange`.
-    node => /** @type {AnnotationHighlight} */ (node)._annotation
+    node => /** @type {AnnotationHighlight} */ (node)._annotation?.$tag
   );
-  return /** @type {AnnotationData[]} */ (items);
+  return tags;
 }
 
 /**
- * Return the annotations associated with any highlights that contain a given
+ * Return the annotation tags associated with any highlights that contain a given
  * DOM node.
  *
  * @param {Node} node
- * @return {AnnotationData[]}
+ * @return {string[]}
  */
 function annotationsAt(node) {
   const items = getHighlightsContainingNode(node)
     .map(h => /** @type {AnnotationHighlight} */ (h)._annotation)
-    .filter(ann => ann !== undefined);
-  return /** @type {AnnotationData[]} */ (items);
+    .filter(ann => ann !== undefined)
+    .map(ann => ann?.$tag);
+  return /** @type {string[]} */ (items);
 }
 
 /**
@@ -137,15 +136,9 @@ export default class Guest {
     this.selectedRanges = [];
 
     this._adder = new Adder(this.element, {
-      onAnnotate: async () => {
-        await this.createAnnotation();
-      },
-      onHighlight: async () => {
-        await this.createAnnotation({ highlight: true });
-      },
-      onShowAnnotations: anns => {
-        this.selectAnnotations(anns);
-      },
+      onAnnotate: () => this.createAnnotation(),
+      onHighlight: () => this.createAnnotation({ highlight: true }),
+      onShowAnnotations: tags => this.selectAnnotations(tags),
     });
 
     this._selectionObserver = new SelectionObserver(range => {
@@ -256,10 +249,10 @@ export default class Guest {
 
     this._listeners.add(this.element, 'mouseup', event => {
       const { target, metaKey, ctrlKey } = /** @type {MouseEvent} */ (event);
-      const annotations = annotationsAt(/** @type {Element} */ (target));
-      if (annotations.length && this._highlightsVisible) {
+      const tags = annotationsAt(/** @type {Element} */ (target));
+      if (tags.length && this._highlightsVisible) {
         const toggle = metaKey || ctrlKey;
-        this.selectAnnotations(annotations, toggle);
+        this.selectAnnotations(tags, toggle);
       }
     });
 
@@ -274,9 +267,9 @@ export default class Guest {
     });
 
     this._listeners.add(this.element, 'mouseover', ({ target }) => {
-      const annotations = annotationsAt(/** @type {Element} */ (target));
-      if (annotations.length && this._highlightsVisible) {
-        this._focusAnnotations(annotations);
+      const tags = annotationsAt(/** @type {Element} */ (target));
+      if (tags.length && this._highlightsVisible) {
+        this._focusAnnotations(tags);
       }
     });
 
@@ -351,6 +344,15 @@ export default class Guest {
       }
     );
 
+    this._hostRPC.on(
+      'selectAnnotations',
+      /**
+       * @param {string[]} tags
+       * @param {boolean} toggle
+       */
+      (tags, toggle) => this.selectAnnotations(tags, toggle)
+    );
+
     this._hostRPC.on(
       'sidebarLayoutChanged',
       /** @param {SidebarLayout} sidebarLayout */
@@ -666,10 +668,9 @@ export default class Guest {
    * Indicate in the sidebar that certain annotations are focused (ie. the
    * associated document region(s) is hovered).
    *
-   * @param {AnnotationData[]} annotations
+   * @param {string[]} tags
    */
-  _focusAnnotations(annotations) {
-    const tags = annotations.map(a => a.$tag);
+  _focusAnnotations(tags) {
     this._sidebarRPC.call('focusAnnotations', tags);
   }
 
@@ -717,12 +718,11 @@ export default class Guest {
    * This sets up a filter in the sidebar to show only the selected annotations
    * and opens the sidebar.
    *
-   * @param {AnnotationData[]} annotations
+   * @param {string[]} tags
    * @param {boolean} [toggle] - Toggle whether the annotations are selected
    *   instead of showing them regardless of whether they are currently selected.
    */
-  selectAnnotations(annotations, toggle = false) {
-    const tags = annotations.map(a => a.$tag);
+  selectAnnotations(tags, toggle = false) {
     if (toggle) {
       this._sidebarRPC.call('toggleAnnotationSelection', tags);
     } else {

src/annotator/range-util.js

@@ -134,10 +134,12 @@ export function selectionFocusRect(selection) {
  * @template T
  * @param {Range} range
  * @param {(n: Node) => T} itemForNode - Callback returning the item for a given node
- * @return {T[]} items
+ * @return {NonNullable<T>[]} items
  */
 export function itemsForRange(range, itemForNode) {
+  /** @type {Set<Node>} */
   const checkedNodes = new Set();
+  /** @type {Set<NonNullable<T>>} */
   const items = new Set();
 
   forEachNodeInRange(range, node => {
@@ -149,8 +151,10 @@ export function itemsForRange(range, itemForNode) {
       }
       checkedNodes.add(current);
 
-      const item = itemForNode(current);
-      if (item) {
+      const item = /** @type {NonNullable<T>|null|undefined} */ (
+        itemForNode(current)
+      );
+      if (item !== null && item !== undefined) {
         items.add(item);
       }
 

src/annotator/sidebar.js

@@ -114,7 +114,12 @@ export default class Sidebar {
       if (config.theme === 'clean') {
         this.iframeContainer.classList.add('annotator-frame--theme-clean');
       } else {
-        this.bucketBar = new BucketBar(this.iframeContainer, guest);
+        this.bucketBar = new BucketBar(this.iframeContainer, guest, {
+          onSelectAnnotations: (tags, toggle) =>
+            this._guestRPC.forEach(rpc =>
+              rpc.call('selectAnnotations', tags, toggle)
+            ),
+        });
       }
 
       this.iframeContainer.appendChild(this.iframe);

src/annotator/test/bucket-bar-test.js

@@ -6,6 +6,7 @@ describe('BucketBar', () => {
   let container;
   let fakeBucketUtil;
   let fakeGuest;
+  let fakeOnSelectAnnotations;
 
   beforeEach(() => {
     bucketBars = [];
@@ -22,6 +23,8 @@ describe('BucketBar', () => {
       selectAnnotations: sinon.stub(),
     };
 
+    fakeOnSelectAnnotations = sinon.stub();
+
     const FakeBuckets = props => {
       bucketProps = props;
       return <div className="FakeBuckets" />;
@@ -40,7 +43,9 @@ describe('BucketBar', () => {
   });
 
   const createBucketBar = () => {
-    const bucketBar = new BucketBar(container, fakeGuest);
+    const bucketBar = new BucketBar(container, fakeGuest, {
+      onSelectAnnotations: fakeOnSelectAnnotations,
+    });
     bucketBars.push(bucketBar);
     return bucketBar;
   };
@@ -51,13 +56,13 @@ describe('BucketBar', () => {
     assert.ok(bucketBar._bucketsContainer.querySelector('.FakeBuckets'));
   });
 
-  it('should select annotations when Buckets component invokes callback', () => {
+  it('passes "onSelectAnnotations" to the Bucket component', () => {
     createBucketBar();
-    const fakeAnnotations = ['hi', 'there'];
+    const tags = ['t1', 't2'];
 
-    bucketProps.onSelectAnnotations(fakeAnnotations, true);
+    bucketProps.onSelectAnnotations(tags, true);
 
-    assert.calledWith(fakeGuest.selectAnnotations, fakeAnnotations, true);
+    assert.calledWith(fakeOnSelectAnnotations, tags, true);
   });
 
   it('should scroll to anchor when Buckets component invokes callback', () => {

src/annotator/test/guest-test.js

@@ -202,6 +202,21 @@ describe('Guest', () => {
         assert.notCalled(fakeIntegration.fitSideBySide);
       });
     });
+
+    describe('on "selectAnnotations" event', () => {
+      it('calls "Guest#selectAnnotations"', () => {
+        const guest = createGuest();
+        sandbox.stub(guest, 'selectAnnotations').callThrough();
+        const tags = ['t1', 't2'];
+        const toggle = true;
+        sidebarRPC().call.resetHistory();
+
+        emitHostEvent('selectAnnotations', tags, toggle);
+
+        assert.calledWith(guest.selectAnnotations, tags, toggle);
+        assert.calledWith(sidebarRPC().call, 'openSidebar');
+      });
+    });
   });
 
   describe('events from sidebar frame', () => {
@@ -590,14 +605,14 @@ describe('Guest', () => {
 
     it('sets the annotations associated with the selection', () => {
       createGuest();
-      const ann = {};
+      const ann = { $tag: 't1' };
       container._annotation = ann;
       rangeUtil.itemsForRange.callsFake((range, callback) => [
         callback(range.startContainer),
       ]);
       simulateSelectionWithText();
 
-      assert.deepEqual(FakeAdder.instance.annotationsForSelection, [ann]);
+      assert.deepEqual(FakeAdder.instance.annotationsForSelection, ['t1']);
     });
 
     it('hides the adder if the selection does not contain text', () => {
@@ -718,34 +733,32 @@ describe('Guest', () => {
 
     it('shows annotations if "Show" is clicked', () => {
       createGuest();
+      const tags = ['t1', 't2'];
 
-      FakeAdder.instance.options.onShowAnnotations([{ $tag: 'ann1' }]);
+      FakeAdder.instance.options.onShowAnnotations(tags);
 
       assert.calledWith(sidebarRPC().call, 'openSidebar');
-      assert.calledWith(sidebarRPC().call, 'showAnnotations', ['ann1']);
+      assert.calledWith(sidebarRPC().call, 'showAnnotations', tags);
     });
   });
 
   describe('#selectAnnotations', () => {
     it('selects the specified annotations in the sidebar', () => {
       const guest = createGuest();
-      const annotations = [{ $tag: 'ann1' }, { $tag: 'ann2' }];
+      const tags = ['t1', 't2'];
 
-      guest.selectAnnotations(annotations);
+      guest.selectAnnotations(tags);
 
-      assert.calledWith(sidebarRPC().call, 'showAnnotations', ['ann1', 'ann2']);
+      assert.calledWith(sidebarRPC().call, 'showAnnotations', tags);
     });
 
     it('toggles the annotations if `toggle` is true', () => {
       const guest = createGuest();
-      const annotations = [{ $tag: 'ann1' }, { $tag: 'ann2' }];
+      const tags = ['t1', 't2'];
 
-      guest.selectAnnotations(annotations, true /* toggle */);
+      guest.selectAnnotations(tags, true /* toggle */);
 
-      assert.calledWith(sidebarRPC().call, 'toggleAnnotationSelection', [
-        'ann1',
-        'ann2',
-      ]);
+      assert.calledWith(sidebarRPC().call, 'toggleAnnotationSelection', tags);
     });
 
     it('opens the sidebar', () => {

src/annotator/test/sidebar-test.js

@@ -944,5 +944,17 @@ describe('Sidebar', () => {
       });
       assert.isNull(sidebar.bucketBar);
     });
+
+    it('calls the "selectAnnotations" RPC method', () => {
+      const sidebar = createSidebar();
+      connectGuest(sidebar);
+      const { onSelectAnnotations } = FakeBucketBar.getCall(0).args[2];
+      const tags = ['t1', 't2'];
+      const toggle = true;
+
+      onSelectAnnotations(tags, toggle);
+
+      assert.calledWith(guestRPC().call, 'selectAnnotations', tags, true);
+    });
   });
 });

src/types/port-rpc-events.d.ts

@@ -75,6 +75,11 @@ export type HostToGuestEvent =
    */
   | 'clearSelectionExceptIn'
 
+  /**
+   * The host informs guests to select/toggle on a set of annotations
+   */
+  | 'selectAnnotations'
+
   /**
    * The host informs guests that the sidebar layout has been changed.
    */