Fix create of a page not in VitalSource

After #4176, the 'create note' button has stopped working in
VitalSource. That's because we don't create the guest frame in the host
frame. Creating a note relayed on one of the guest frames having a
frameIdentifier` with value `null`. That's not longer the case.

Now, when creating a page note we send the request to the first
connected guest frame. This is analogous to #4177.

In addition, we only send `textUnselected`, `textSelected` and
`createAnnotation` RPC events to only specific guest frames. That
simplifies the logic by avoiding to send a `frameIdentifier`.

src/annotator/guest.js

@@ -302,28 +302,12 @@ export class Guest {
   }
 
   async _connectHost() {
-    this._hostRPC.on(
-      'clearSelectionExceptIn',
-      /** @param {string|null} frameIdentifier */
-      frameIdentifier => {
-        if (this._frameIdentifier === frameIdentifier) {
-          return;
-        }
-
-        this._informHostOnNextSelectionClear = false;
-        removeTextSelection();
-      }
-    );
+    this._hostRPC.on('clearSelection', () => {
+      this._informHostOnNextSelectionClear = false;
+      removeTextSelection();
+    });
 
-    this._hostRPC.on(
-      'createAnnotationIn',
-      /** @param {string|null} frameIdentifier */
-      frameIdentifier => {
-        if (this._frameIdentifier === frameIdentifier) {
-          this.createAnnotation();
-        }
-      }
-    );
+    this._hostRPC.on('createAnnotation', () => this.createAnnotation());
 
     this._hostRPC.on(
       'focusAnnotations',
@@ -692,7 +676,7 @@ export class Guest {
     }
 
     this.selectedRanges = [range];
-    this._hostRPC.call('textSelectedIn', this._frameIdentifier);
+    this._hostRPC.call('textSelected');
 
     this._adder.annotationsForSelection = annotationsForSelection();
     this._isAdderVisible = true;
@@ -704,7 +688,7 @@ export class Guest {
     this._adder.hide();
     this.selectedRanges = [];
     if (this._informHostOnNextSelectionClear) {
-      this._hostRPC.call('textUnselectedIn', this._frameIdentifier);
+      this._hostRPC.call('textUnselected');
     }
     this._informHostOnNextSelectionClear = true;
   }

src/annotator/sidebar.js

@@ -67,10 +67,10 @@ export class Sidebar {
     this._emitter = eventBus.createEmitter();
 
     /**
-     * Tracks which `Guest` has a text selection. `null` indicates the host frame,
-     * the top-level frame. Other `guest` frames use string identifier.
+     * Tracks which `Guest` has a text selection. `null` indicates to default
+     * to the first connected guest frame.
      *
-     * @type {string|null}
+     * @type {PortRPC|null}
      */
     this._guestWithSelection = null;
 
@@ -145,10 +145,14 @@ export class Sidebar {
     // Set up the toolbar on the left edge of the sidebar.
     const toolbarContainer = document.createElement('div');
     this.toolbar = new ToolbarController(toolbarContainer, {
-      createAnnotation: () =>
-        this._guestRPC.forEach(rpc =>
-          rpc.call('createAnnotationIn', this._guestWithSelection)
-        ),
+      createAnnotation: () => {
+        if (this._guestRPC.length === 0) {
+          return;
+        }
+
+        const rpc = this._guestWithSelection ?? this._guestRPC[0];
+        rpc.call('createAnnotation');
+      },
       setSidebarOpen: open => (open ? this.open() : this.close()),
       setHighlightsVisible: show => this.setHighlightsVisible(show),
     });
@@ -261,29 +265,21 @@ export class Sidebar {
     /** @type {PortRPC<GuestToHostEvent, HostToGuestEvent>} */
     const guestRPC = new PortRPC();
 
-    guestRPC.on(
-      'textSelectedIn',
-      /** @param {string|null} frameIdentifier */
-      frameIdentifier => {
-        this._guestWithSelection = frameIdentifier;
-        this.toolbar.newAnnotationType = 'annotation';
-        this._guestRPC.forEach(rpc =>
-          rpc.call('clearSelectionExceptIn', frameIdentifier)
-        );
-      }
-    );
+    guestRPC.on('textSelected', () => {
+      this._guestWithSelection = guestRPC;
+      this.toolbar.newAnnotationType = 'annotation';
+      this._guestRPC
+        .filter(port => port !== guestRPC)
+        .forEach(rpc => rpc.call('clearSelection'));
+    });
 
-    guestRPC.on(
-      'textUnselectedIn',
-      /** @param {string|null}  frameIdentifier */
-      frameIdentifier => {
-        this._guestWithSelection = null; // default to the `host` frame
-        this.toolbar.newAnnotationType = 'note';
-        this._guestRPC.forEach(rpc =>
-          rpc.call('clearSelectionExceptIn', frameIdentifier)
-        );
-      }
-    );
+    guestRPC.on('textUnselected', () => {
+      this._guestWithSelection = null;
+      this.toolbar.newAnnotationType = 'note';
+      this._guestRPC
+        .filter(port => port !== guestRPC)
+        .forEach(rpc => rpc.call('clearSelection'));
+    });
 
     // The listener will do nothing if the sidebar doesn't have a bucket bar
     // (clean theme)
@@ -303,6 +299,9 @@ export class Sidebar {
 
     guestRPC.on('close', () => {
       guestRPC.destroy();
+      if (guestRPC === this._guestWithSelection) {
+        this._guestWithSelection = null;
+      }
       this._guestRPC = this._guestRPC.filter(rpc => rpc !== guestRPC);
     });
 

src/annotator/test/guest-test.js

@@ -439,6 +439,17 @@ describe('Guest', () => {
       });
     });
 
+    describe('on "createAnnotation" event', () => {
+      it('creates an annotation', async () => {
+        createGuest();
+
+        emitHostEvent('createAnnotation');
+        await delay(0);
+
+        assert.calledWith(sidebarRPC().call, 'createAnnotation');
+      });
+    });
+
     describe('on "deleteAnnotation" event', () => {
       it('detaches annotation', () => {
         createGuest();
@@ -645,38 +656,20 @@ describe('Guest', () => {
       assert.notCalled(FakeAdder.instance.show);
     });
 
-    it('calls "textSelectedIn" RPC method with argument "null" if selection is non-empty', () => {
+    it('calls "textSelected" RPC method when selecting text', () => {
       createGuest();
 
       simulateSelectionWithText();
 
-      assert.calledWith(hostRPC().call, 'textSelectedIn', null);
+      assert.calledWith(hostRPC().call, 'textSelected');
     });
 
-    it('calls "textSelectedIn" RPC method with the subFrameIdentifier as argument if selection is non-empty', () => {
-      const subFrameIdentifier = 'subframe identifier';
-      createGuest({ subFrameIdentifier });
-
-      simulateSelectionWithText();
-
-      assert.calledWith(hostRPC().call, 'textSelectedIn', subFrameIdentifier);
-    });
-
-    it('calls "textUnselectedIn" RPC method with argument "null" if selection is empty', () => {
+    it('calls "textUnselected" RPC method when clearing text selection', () => {
       createGuest();
 
       simulateSelectionWithoutText();
 
-      assert.calledWith(hostRPC().call, 'textUnselectedIn', null);
-    });
-
-    it('calls "textUnselectedIn" RPC method with the subFrameIdentifier as argument if selection is empty', () => {
-      const subFrameIdentifier = 'subframe identifier';
-      createGuest({ subFrameIdentifier });
-
-      simulateSelectionWithoutText();
-
-      assert.calledWith(hostRPC().call, 'textUnselectedIn', subFrameIdentifier);
+      assert.calledWith(hostRPC().call, 'textUnselected');
     });
 
     it('unselects text if another iframe has made a selection', () => {
@@ -687,7 +680,7 @@ describe('Guest', () => {
 
       simulateSelectionWithText();
       hostRPC().call.resetHistory();
-      emitHostEvent('clearSelectionExceptIn', 'subframe identifier');
+      emitHostEvent('clearSelection');
 
       assert.calledOnce(removeAllRanges);
       notifySelectionChanged(null); // removing the text selection triggers the selection observer
@@ -698,7 +691,7 @@ describe('Guest', () => {
       // On next selection clear it should be inform the host.
       notifySelectionChanged(null);
       assert.calledOnce(hostRPC().call);
-      assert.calledWithExactly(hostRPC().call, 'textUnselectedIn', null);
+      assert.calledWithExactly(hostRPC().call, 'textUnselected');
     });
 
     it("doesn't unselect text if frame identifier matches", () => {
@@ -809,22 +802,6 @@ describe('Guest', () => {
   });
 
   describe('#createAnnotation', () => {
-    it('creates an annotation if host calls "createAnnotationIn" RPC method', async () => {
-      createGuest();
-      await delay(0);
-      sidebarRPC().call.resetHistory(); // Discard `documentInfoChanged` call
-
-      emitHostEvent('createAnnotationIn', 'dummy');
-      await delay(0);
-
-      assert.notCalled(sidebarRPC().call);
-
-      emitHostEvent('createAnnotationIn', null);
-      await delay(0);
-
-      assert.calledWith(sidebarRPC().call, 'createAnnotation');
-    });
-
     it('adds document metadata to the annotation', async () => {
       const guest = createGuest();
 

src/annotator/test/sidebar-test.js

@@ -43,13 +43,13 @@ describe('Sidebar', () => {
   };
 
   /** Return the PortRPC instance for the first connected guest. */
-  const guestRPC = () => {
-    return fakePortRPCs[1];
+  const guestRPC = (index = 1) => {
+    return fakePortRPCs[index];
   };
 
-  const emitGuestEvent = (event, ...args) => {
+  const emitNthGuestEvent = (index = 1, event, ...args) => {
     const result = [];
-    for (let [evt, fn] of guestRPC().on.args) {
+    for (let [evt, fn] of guestRPC(index).on.args) {
       if (event === evt) {
         result.push(fn(...args));
       }
@@ -57,6 +57,10 @@ describe('Sidebar', () => {
     return result;
   };
 
+  const emitGuestEvent = (event, ...args) => {
+    return emitNthGuestEvent(1, event, ...args);
+  };
+
   const emitSidebarEvent = (event, ...args) => {
     const result = [];
     for (let [evt, fn] of sidebarRPC().on.args) {
@@ -253,55 +257,70 @@ describe('Sidebar', () => {
       assert.calledWith(sidebar.setHighlightsVisible, false);
     });
 
-    it('creates an annotation in the host frame when toolbar button is clicked', () => {
+    it("doesn't throw when creating an annotation by clicking in toolbar button and there are not connected guests", () => {
+      createSidebar();
+
+      FakeToolbarController.args[0][1].createAnnotation();
+    });
+
+    it('creates an annotation in the first connected guest when toolbar button is clicked', () => {
       const sidebar = createSidebar();
       connectGuest(sidebar);
 
       FakeToolbarController.args[0][1].createAnnotation();
 
-      assert.calledWith(guestRPC().call, 'createAnnotationIn', null);
+      assert.calledWith(guestRPC().call, 'createAnnotation');
     });
 
-    it('creates an annotation in frame with selection when toolbar button is clicked', () => {
+    it('creates an annotation in guest with selected text when toolbar button is clicked', () => {
       const sidebar = createSidebar({});
       connectGuest(sidebar);
+      connectGuest(sidebar);
+
+      emitNthGuestEvent(2, 'textSelected');
+      FakeToolbarController.args[0][1].createAnnotation();
+
+      assert.neverCalledWith(guestRPC(1).call, 'createAnnotation');
+      assert.calledWith(guestRPC(2).call, 'createAnnotation');
+    });
 
-      const frameIdentifier = 'subframe identifier';
-      emitGuestEvent('textSelectedIn', frameIdentifier);
+    it('creates an annotation in the first connected guest if guest with selected text has closed', () => {
+      const sidebar = createSidebar({});
+      connectGuest(sidebar);
+      connectGuest(sidebar);
 
+      emitNthGuestEvent(2, 'textSelected');
+      emitNthGuestEvent(2, 'close');
       FakeToolbarController.args[0][1].createAnnotation();
 
-      assert.calledWith(guestRPC().call, 'createAnnotationIn', frameIdentifier);
+      assert.calledWith(guestRPC(1).call, 'createAnnotation');
+      assert.neverCalledWith(guestRPC(2).call, 'createAnnotation');
     });
 
     it('toggles create annotation button to "Annotation" when selection becomes non-empty', () => {
       const sidebar = createSidebar();
       connectGuest(sidebar);
+      connectGuest(sidebar);
 
-      const frameIdentifier = 'subframe identifier';
-      emitGuestEvent('textSelectedIn', frameIdentifier);
+      emitGuestEvent('textSelected');
 
       assert.equal(sidebar.toolbar.newAnnotationType, 'annotation');
-      assert.calledWith(
-        guestRPC().call,
-        'clearSelectionExceptIn',
-        frameIdentifier
-      );
+      assert.neverCalledWith(guestRPC(1).call, 'clearSelection');
+      assert.calledWith(guestRPC(2).call, 'clearSelection');
     });
 
     it('toggles create annotation button to "Page Note" when selection becomes empty', () => {
       const sidebar = createSidebar();
       connectGuest(sidebar);
+      connectGuest(sidebar);
+      emitGuestEvent('textSelected');
+      assert.equal(sidebar.toolbar.newAnnotationType, 'annotation');
 
-      const frameIdentifier = null;
-      emitGuestEvent('textUnselectedIn', frameIdentifier);
+      emitGuestEvent('textUnselected');
 
       assert.equal(sidebar.toolbar.newAnnotationType, 'note');
-      assert.calledWith(
-        guestRPC().call,
-        'clearSelectionExceptIn',
-        frameIdentifier
-      );
+      assert.neverCalledWith(guestRPC(1).call, 'clearSelection');
+      assert.calledWith(guestRPC(2).call, 'clearSelection');
     });
   });
 

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

@@ -7,15 +7,11 @@
  * Events that the guest sends to the host
  */
 export type GuestToHostEvent =
-  /**
-   * The guest informs the host that text has been unselected at a frame with that identifier.
-   */
-  | 'textUnselectedIn'
+  /** The guest informs the host that text has been unselected. */
+  | 'textUnselected'
 
-  /**
-   * The guest informs the host that text has been selected at a frame with that identifier.
-   */
-  | 'textSelectedIn'
+  /** The guest informs the host that text has been selected. */
+  | 'textSelected'
 
   /**
    * The guest informs the host that the anchors have been changed in the main annotatable frame.
@@ -68,15 +64,11 @@ export type GuestToSidebarEvent =
  * Events that the host sends to the guest
  */
 export type HostToGuestEvent =
-  /**
-   * The host requests a guest with a specific frame identifier to create an annotation.
-   */
-  | 'createAnnotationIn'
+  /** The host requests a guest to create an annotation. */
+  | 'createAnnotation'
 
-  /**
-   * The host informs guests that text should be unselected, except in the guest with a given frame identifier
-   */
-  | 'clearSelectionExceptIn'
+  /** The host informs guests that text should be unselected. */
+  | 'clearSelection'
 
   /**
    * The host informs guests to focus on a set of annotations