Handle multi-step chapter load process more reliably in VitalSource integration

dev-server/documents/html/vitalsource-epub.mustache

@@ -27,7 +27,7 @@
 
           let chapterIndex = 0;
 
-          const setChapter = index => {
+          const setChapter = (index, { initialLoad = false } = {}) => {
             if (index < 0 || index >= chapterURLs.length) {
               return;
             }
@@ -38,8 +38,33 @@
             // does. The client should be robust to either approach.
             this.contentFrame?.remove();
             this.contentFrame = document.createElement('iframe');
-            this.contentFrame.src = chapterURLs[chapterIndex];
             this.shadowRoot.append(this.contentFrame);
+
+            const chapterURL = chapterURLs[chapterIndex];
+
+            if (initialLoad) {
+              // Simulate client loading after VS chapter content has already
+              // loaded.
+              this.contentFrame.src = chapterURL;
+            } else {
+              // Simulate chapter navigation after client is injected. These
+              // navigations happen in several stages:
+              //
+              // 1. The previous chapter's iframe is removed
+              // 2. A new iframe is created. The initial HTML is a "blank" page
+              //    containing (invisible) content data for the new chapter as
+              //    text in the page.
+              // 3. The content data is posted to the server via a form
+              //    submission, which returns the decoded HTML.
+              //
+              // The client should only inject into the new frame after step 3.
+              this.contentFrame.src = 'about:blank';
+              setTimeout(() => {
+                // Set the final URL in a way that doesn't update the `src` attribute
+                // of the iframe, to make sure the client isn't relying on that.
+                this.contentFrame.contentWindow.location.href = chapterURL;
+              }, 50);
+            }
           };
 
           const styles = document.createElement('style');
@@ -66,7 +91,7 @@
           this.nextButton.onclick = () => setChapter(chapterIndex + 1);
           controlBar.append(this.nextButton);
 
-          setChapter(0);
+          setChapter(0, { initialLoad: true });
         }
       }
       customElements.define('mosaic-book', MosaicElement);

src/annotator/integrations/test/vitalsource-test.js

@@ -15,20 +15,41 @@ class FakeVitalSourceViewer {
     this.bookElement.shadowRoot.append(this.contentFrame);
 
     document.body.append(this.bookElement);
+
+    this.contentFrame.contentDocument.body.innerHTML = '<p>Initial content</p>';
   }
 
   destroy() {
     this.bookElement.remove();
   }
 
-  /** Simulate navigation to a different chapter of the book. */
-  loadNextChapter() {
+  /**
+   * Simulate navigation to a different chapter of the book.
+   *
+   * This process happens in two steps. This method simulates the first step.
+   * `finishChapterLoad` simulates the second step.
+   */
+  beginChapterLoad() {
     this.contentFrame.remove();
 
     // VS handles navigations by removing the frame and creating a new one,
     // rather than navigating the existing frame.
     this.contentFrame = document.createElement('iframe');
     this.bookElement.shadowRoot.append(this.contentFrame);
+
+    // When the new frame initially loads, it will contain some encoded/encrypted
+    // data for the new chapter. VS will then make a form submission to get the
+    // decoded HTML.
+    //
+    // The integration should not inject the client if the frame contains this
+    // data content.
+    this.contentFrame.contentDocument.body.innerHTML =
+      '<div>Encrypted content</div>';
+  }
+
+  finishChapterLoad() {
+    this.contentFrame.contentDocument.body.innerHTML = '<p>New content</p>';
+    this.contentFrame.dispatchEvent(new Event('load'));
   }
 }
 
@@ -107,12 +128,40 @@ describe('annotator/integrations/vitalsource', () => {
     it('re-injects client when content frame is changed', async () => {
       fakeGuest.injectClient.resetHistory();
 
-      fakeViewer.loadNextChapter();
+      fakeViewer.beginChapterLoad();
       await delay(0);
+      assert.notCalled(fakeGuest.injectClient);
 
+      fakeViewer.finishChapterLoad();
+      await delay(0);
       assert.calledWith(fakeGuest.injectClient, fakeViewer.contentFrame);
     });
 
+    it("doesn't re-inject if content frame is removed", async () => {
+      fakeGuest.injectClient.resetHistory();
+
+      // Remove the content frame. This will trigger a re-injection check, but
+      // do nothing as there is no content frame.
+      fakeViewer.contentFrame.remove();
+      await delay(0);
+
+      assert.notCalled(fakeGuest.injectClient);
+    });
+
+    it("doesn't re-inject if content frame siblings change", async () => {
+      fakeGuest.injectClient.resetHistory();
+
+      // Modify the DOM tree. This will trigger a re-injection check, but do
+      // nothing as we've already handled the current frame.
+      fakeViewer.contentFrame.insertAdjacentElement(
+        'afterend',
+        document.createElement('div')
+      );
+      await delay(0);
+
+      assert.notCalled(fakeGuest.injectClient);
+    });
+
     it('does not allow annotation in the container frame', async () => {
       assert.equal(integration.canAnnotate(), false);
 

src/annotator/integrations/vitalsource.js

@@ -55,20 +55,45 @@ export class VitalSourceContainerIntegration {
       throw new Error('Book container element not found');
     }
 
+    /** @type {WeakSet<HTMLIFrameElement>} */
+    const contentFrames = new WeakSet();
+
+    /** @param {HTMLIFrameElement} frame */
+    const injectIfContentReady = frame => {
+      // Check if this frame contains decoded ebook content, as opposed to
+      // invisible and encrypted book content, which is created initially after a
+      // chapter navigation. These encrypted pages are replaced with the real
+      // content after a form submission.
+      //
+      // The format of the decoded HTML can vary, but as a simple heuristic,
+      // we look for a text paragraph.
+      //
+      // If the document has not yet finished loading, then we rely on this function
+      // being called again once loading completes.
+      const isBookContent = frame.contentDocument?.querySelector('p');
+      if (isBookContent) {
+        annotator.injectClient(frame);
+      }
+    };
+
     const shadowRoot = /** @type {ShadowRoot} */ (bookElement.shadowRoot);
     const injectClientIntoContentFrame = () => {
       const frame = shadowRoot.querySelector('iframe');
-      if (frame) {
-        annotator.injectClient(frame);
+      if (!frame || contentFrames.has(frame)) {
+        // Either there is no content frame or we are already watching it.
+        return;
       }
+      contentFrames.add(frame);
+
+      injectIfContentReady(frame);
+      frame.addEventListener('load', () => {
+        injectIfContentReady(frame);
+      });
     };
 
     injectClientIntoContentFrame();
 
     // Re-inject client into content frame after a chapter navigation.
-    //
-    // We currently don't do any debouncing here and rely on `injectClient` to
-    // be idempotent and cheap.
     this._frameObserver = new MutationObserver(injectClientIntoContentFrame);
     this._frameObserver.observe(shadowRoot, { childList: true, subtree: true });
   }