Explicitly specify guest/sidebar frames to ping in startDiscovery

docs/publishers/embedding.rst

@@ -23,7 +23,8 @@ above script tag to the document displayed in the iframe. This will display the
 sidebar in the iframe itself.
 
 Additionally Hypothesis has limited support for enabling annotation of iframed
-content while showing the sidebar in the top-level document. To use this:
+content while showing the sidebar in the top-level document where Hypothesis
+was initially loaded. To use this:
 
 1. Add the above script tag to the top-level document
 
@@ -38,6 +39,8 @@ content while showing the sidebar in the top-level document. To use this:
    ...
    </iframe>
 
-This method *only* works for iframes which are same-origin with the top-level
-document. The client will watch for new iframes being added to the document and
-will automatically enable annotation for them.
+This method *only* works for iframes which are direct children of the top-level
+document and have the same origin.
+
+The client will watch for new iframes being added to the document and will
+automatically enable annotation for them.

src/annotator/cross-frame.js

@@ -63,12 +63,28 @@ export class CrossFrame {
       frameIdentifiers.delete(frame);
     };
 
-    // Initiate connection to the sidebar.
-    discovery.startDiscovery((source, origin, token) =>
-      bridge.createChannel({ source, origin, token })
-    );
     frameObserver.observe(injectIntoFrame, iframeUnloaded);
 
+    /**
+     * Attempt to connect to the sidebar frame.
+     *
+     * Returns a promise that resolves once the connection has been established.
+     *
+     * @param {Window} frame - The window containing the sidebar application
+     * @return {Promise<void>}
+     */
+    this.connectToSidebar = frame => {
+      return new Promise(resolve => {
+        discovery.startDiscovery(
+          (source, origin, token) => {
+            bridge.createChannel({ source, origin, token });
+            resolve();
+          },
+          [frame]
+        );
+      });
+    };
+
     /**
      * Remove the connection between the sidebar and annotator.
      */

src/annotator/index.js

@@ -33,33 +33,66 @@ const appLinkEl = /** @type {Element} */ (
   )
 );
 
+/**
+ * Entry point for the part of the Hypothesis client that runs in the page being
+ * annotated.
+ *
+ * Depending on the client configuration in the current frame, this can
+ * initialize different functionality. In "host" frames the sidebar controls and
+ * iframe containing the sidebar application are created. In "guest" frames the
+ * functionality to support anchoring and creating annotations is loaded. An
+ * instance of Hypothesis will have one host frame, one sidebar frame and one or
+ * more guest frames. The most common case is that the host frame, where the
+ * client is initially loaded, is also the only guest frame.
+ */
 function init() {
+  // Create an internal global used to share data between same-origin guest and
+  // host frames.
+  window_.__hypothesis = {};
+
   const annotatorConfig = getConfig('annotator');
   const isPDF = typeof window_.PDFViewerApplication !== 'undefined';
 
-  if (annotatorConfig.subFrameIdentifier) {
-    // Other modules use this to detect if this
-    // frame context belongs to hypothesis.
-    // Needs to be a global property that's set.
-    window_.__hypothesis_frame = true;
-  }
-
   const eventBus = new EventBus();
   const guest = new Guest(document.body, eventBus, {
     ...annotatorConfig,
     // Load the PDF anchoring/metadata integration.
     // nb. documentType is an internal config property only
     documentType: isPDF ? 'pdf' : 'html',
   });
-  const sidebar = !annotatorConfig.subFrameIdentifier
-    ? new Sidebar(document.body, eventBus, guest, getConfig('sidebar'))
-    : null;
+
+  // Create the sidebar if this is the host frame. The `subFrameIdentifier`
+  // config option indicates a non-host/guest-only frame.
+  let sidebar;
+  if (!annotatorConfig.subFrameIdentifier) {
+    sidebar = new Sidebar(document.body, eventBus, guest, getConfig('sidebar'));
+
+    // Expose sidebar window reference for use by same-origin guest frames.
+    window_.__hypothesis.sidebarWindow = sidebar.iframe.contentWindow;
+  }
+
   // Clear `annotations` value from the notebook's config to prevent direct-linked
   // annotations from filtering the threads.
   const notebook = new Notebook(document.body, eventBus, getConfig('notebook'));
 
+  // Set up communication between this host/guest frame and the sidebar frame.
+  const sidebarWindow = sidebar
+    ? sidebar.iframe.contentWindow
+    : /** @type {HypothesisWindow} */ (window.parent).__hypothesis
+        ?.sidebarWindow;
+  if (sidebarWindow) {
+    guest.crossframe.connectToSidebar(sidebarWindow);
+  } else {
+    // eslint-disable-next-line no-console
+    console.warn(
+      `Hypothesis guest frame in ${location.origin} could not find a sidebar to connect to`
+    );
+  }
+
   appLinkEl.addEventListener('destroy', () => {
+    delete window_.__hypothesis;
     sidebar?.destroy();
+
     notebook.destroy();
     guest.destroy();
 

src/annotator/sidebar.js

@@ -67,7 +67,12 @@ export default class Sidebar {
    */
   constructor(element, eventBus, guest, config = {}) {
     this._emitter = eventBus.createEmitter();
+
+    /**
+     * The `<iframe>` element containing the sidebar application.
+     */
     this.iframe = createSidebarIframe(config);
+
     this.options = config;
 
     /** @type {BucketBar|null} */

src/annotator/test/cross-frame-test.js

@@ -78,18 +78,24 @@ describe('CrossFrame', () => {
         emit: sinon.match.func,
       });
     });
+  });
 
-    it('starts the discovery of new channels', () => {
-      createCrossFrame();
-      assert.called(fakeDiscovery.startDiscovery);
-    });
+  describe('#connectToSidebar', () => {
+    it('starts the discovery of new channels', async () => {
+      const cf = createCrossFrame();
+      const sidebarFrame = {};
+      fakeDiscovery.startDiscovery.callsFake((callback, frames) => {
+        setTimeout(() => callback(frames[0], 'ORIGIN', 'TOKEN'), 0);
+      });
 
-    it('creates a channel when a new frame is discovered', () => {
-      createCrossFrame();
-      fakeDiscovery.startDiscovery.yield('SOURCE', 'ORIGIN', 'TOKEN');
+      await cf.connectToSidebar(sidebarFrame);
+
+      assert.calledWith(fakeDiscovery.startDiscovery, sinon.match.func, [
+        sidebarFrame,
+      ]);
       assert.called(fakeBridge.createChannel);
       assert.calledWith(fakeBridge.createChannel, {
-        source: 'SOURCE',
+        source: sidebarFrame,
         origin: 'ORIGIN',
         token: 'TOKEN',
       });

src/annotator/test/integration/multi-frame-test.js

@@ -137,7 +137,7 @@ describe('CrossFrame multi-frame scenario', () => {
     const frame = document.createElement('iframe');
     frame.setAttribute('enable-annotation', '');
     container.appendChild(frame);
-    frame.contentWindow.eval('window.__hypothesis_frame = true');
+    frame.contentWindow.eval('window.__hypothesis = {}');
 
     crossFrame = createCrossFrame();
 

src/annotator/test/sidebar-test.js

@@ -145,6 +145,16 @@ describe('Sidebar', () => {
     });
   });
 
+  describe('#iframe', () => {
+    it('returns a reference to the `<iframe>` containing the sidebar', () => {
+      const sidebar = createSidebar();
+      const iframe = containers[0]
+        .querySelector('hypothesis-sidebar')
+        .shadowRoot.querySelector('iframe');
+      assert.equal(sidebar.iframe, iframe);
+    });
+  });
+
   function getConfigString(sidebar) {
     return sidebar.iframe.src;
   }

src/annotator/util/frame-util.js

@@ -17,7 +17,7 @@ export function findFrames(container) {
 
 // Check if the iframe has already been injected
 export function hasHypothesis(iframe) {
-  return iframe.contentWindow.__hypothesis_frame === true;
+  return '__hypothesis' in iframe.contentWindow;
 }
 
 // Inject embed.js into the iframe

src/shared/discovery.js

@@ -72,8 +72,9 @@ export default class Discovery {
    *
    * @param {DiscoveryCallback} onDiscovery - Callback to invoke with a token when
    *   another frame is discovered.
+   * @param {Window[]} frames - A list of frames to attempt to connect to.
    */
-  startDiscovery(onDiscovery) {
+  startDiscovery(onDiscovery, frames) {
     if (this.onDiscovery) {
       throw new Error(
         'Discovery is already in progress. Call stopDiscovery() first'
@@ -84,7 +85,14 @@ export default class Discovery {
 
     // Listen for messages from other frames.
     this._listeners.add(this.target, 'message', this._onMessage);
-    this._beacon();
+
+    // Ping specified other frames to tell them about the existence of this frame.
+    const beaconMessage = this.server
+      ? '__cross_frame_dhcp_offer'
+      : '__cross_frame_dhcp_discovery';
+    for (let frame of frames) {
+      frame.postMessage(beaconMessage, this.origin);
+    }
   }
 
   /**
@@ -95,33 +103,6 @@ export default class Discovery {
     this._listeners.removeAll();
   }
 
-  /**
-   * Send a message to other frames in the current window to inform them about
-   * the existence of this frame and tell them whether this frame is a client
-   * or server.
-   */
-  _beacon() {
-    let beaconMessage;
-    if (this.server) {
-      beaconMessage = '__cross_frame_dhcp_offer';
-    } else {
-      beaconMessage = '__cross_frame_dhcp_discovery';
-    }
-
-    // Perform a top-down, breadth-first traversal of frames in the current
-    // window and send messages to them.
-    const queue = [this.target.top];
-    while (queue.length > 0) {
-      const parent = /** @type {Window} */ (queue.shift());
-      if (parent !== this.target) {
-        parent.postMessage(beaconMessage, this.origin);
-      }
-      for (let i = 0; i < parent.frames.length; i++) {
-        queue.push(parent.frames[i]);
-      }
-    }
-  }
-
   /**
    * Handle a `MessageEvent` from another frame which _may_ be from a
    * `Discovery` instance.