Make logic for waiting for PDFs to load work in PDF.js >= 2.5.207

f96af43d added support for PDF.js >= v2.5.207 by changing an event
listener to use PDF.js's internal event bus rather than DOM events.
However there was another DOM event listener for the `pagesloaded` event
in `src/annotator/anchoring/pdf.js` that should also have been updated but was
overlooked. This didn't cause a problem in testing with the dev server because
the test documents load quickly enough that they are already loaded by the time
the client's anchoring logic ran.

This commit updates the way that the client listens for events from
PDF.js to use the event bus where available and only fall back to the
DOM in versions of PDF.js that don't support it.

 - Use PDF.js's event bus to listen for `documentload`/`documentloaded`
   and `pagesloaded` events

 - Add a fallback method to wait for event bus to become available in
   versions of PDF.js which support the eventBus but don't have the
   `initializedPromise` API

 - Improve the documentation around which versions of PDF.js support
   different event types and event dispatch methods

 - Add tests to cover the behavior from different releases of PDF.js

For an overview of the different versions of PDF.js that the client
needs to support, see https://github.com/hypothesis/client/issues/2643.

src/annotator/anchoring/pdf.js

@@ -75,10 +75,21 @@ async function getPageView(pageIndex) {
     // a "pdfPage" property.
     pageView = await new Promise(resolve => {
       const onPagesLoaded = () => {
-        document.removeEventListener('pagesloaded', onPagesLoaded);
+        if (pdfViewer.eventBus) {
+          pdfViewer.eventBus.off('pagesloaded', onPagesLoaded);
+        } else {
+          document.removeEventListener('pagesloaded', onPagesLoaded);
+        }
+
         resolve(pdfViewer.getPageView(pageIndex));
       };
-      document.addEventListener('pagesloaded', onPagesLoaded);
+
+      if (pdfViewer.eventBus) {
+        pdfViewer.eventBus.on('pagesloaded', onPagesLoaded);
+      } else {
+        // Old PDF.js versions (< 1.6.210) use DOM events.
+        document.addEventListener('pagesloaded', onPagesLoaded);
+      }
     });
   }
 

src/annotator/anchoring/test/fake-pdf-viewer-application.js

@@ -6,9 +6,12 @@
  * easier to debug than the full PDF.js viewer application.
  *
  * The general structure is to have `Fake{OriginalClassName}` classes for
- * each of the relevant classes in PDF.js.
+ * each of the relevant classes in PDF.js. The APIs of the fakes should conform
+ * to the corresponding interfaces defined in `src/types/pdfjs.js`.
  */
 
+import { TinyEmitter as EventEmitter } from 'tiny-emitter';
+
 import RenderingStates from '../../pdfjs-rendering-states';
 
 /**
@@ -124,6 +127,8 @@ class FakePDFViewer {
     this._pages = [];
 
     this.viewer = this._container;
+
+    this.eventBus = new EventEmitter();
   }
 
   get pagesCount() {
@@ -170,11 +175,17 @@ class FakePDFViewer {
   }
 
   /**
-   * Dispatch a DOM event to notify observers that some event has occurred
+   * Dispatch an event to notify observers that some event has occurred
    * in the PDF viewer.
    */
-  notify(eventName) {
-    this._container.dispatchEvent(new Event(eventName, { bubbles: true }));
+  notify(eventName, { eventDispatch = 'eventBus' } = {}) {
+    if (eventDispatch === 'eventBus') {
+      this.eventBus.emit(eventName);
+    } else if (eventDispatch === 'dom') {
+      this._container.dispatchEvent(
+        new CustomEvent(eventName, { bubbles: true })
+      );
+    }
   }
 
   _checkBounds(index) {

src/annotator/anchoring/test/pdf-test.js

@@ -295,15 +295,28 @@ describe('annotator/anchoring/pdf', function () {
         // `PDFViewer.getPageView` returns a nullish value.
         description: 'page view not loaded',
         pageView: undefined,
+        eventDispatch: 'eventBus',
       },
       {
         // `PDFPageViewer.getPageView` returns a `PDFPageView`, but the associated
         // page is not ready yet and so the `pdfPage` property is missing.
         description: 'page view PDF page not ready',
         pageView: {},
+        eventDispatch: 'eventBus',
       },
-    ].forEach(({ description, pageView }) => {
+      {
+        // Older version of PDF.js (< 1.6.210) using DOM events.
+        description: 'old PDF.js version',
+        pageView: undefined,
+        eventDispatch: 'dom',
+      },
+    ].forEach(({ description, pageView, eventDispatch }) => {
       it(`waits until page views are ready (${description})`, async () => {
+        if (eventDispatch === 'dom') {
+          // To simulate versions of PDF.js < 1.6.210, remove the `eventBus` API.
+          delete viewer.pdfViewer.eventBus;
+        }
+
         viewer.pdfViewer.setCurrentPage(1);
 
         // Simulate PDF viewer not having fully loaded yet.
@@ -323,7 +336,7 @@ describe('annotator/anchoring/pdf', function () {
         // view, but block because it is not ready yet.
         await delay(10);
         getPageView.restore();
-        viewer.pdfViewer.notify('pagesloaded');
+        viewer.pdfViewer.notify('pagesloaded', { eventDispatch });
 
         // Check that anchoring completes successfully when the document has
         // loaded.

src/annotator/plugin/pdf-metadata.js

@@ -18,6 +18,36 @@ import { normalizeURI } from '../util/url';
  *   part of the content if the PDF file does not have a File Identifier.
  */
 
+/**
+ * Wait for a PDFViewerApplication to be initialized.
+ *
+ * @param {PDFViewerApplication} app
+ * @return {Promise<void>}
+ */
+function pdfViewerInitialized(app) {
+  // `initializedPromise` was added in PDF.js v2.4.456.
+  // See https://github.com/mozilla/pdf.js/pull/11607. In earlier versions the
+  // `initialized` property can be queried.
+  if (app.initializedPromise) {
+    return app.initializedPromise;
+  } else if (app.initialized) {
+    return Promise.resolve();
+  } else {
+    // PDF.js < v2.4.456. The recommended approach is to listen for a `localized`
+    // DOM event, but this assumes that PDF.js has been configured to publish
+    // events to the DOM. Here we simply poll `app.initialized` because it is
+    // easier.
+    return new Promise(resolve => {
+      const timeout = setInterval(() => {
+        if (app.initialized) {
+          clearTimeout(timeout);
+          resolve();
+        }
+      }, 5);
+    });
+  }
+}
+
 /**
  * PDFMetadata extracts metadata about a loading/loaded PDF document from a
  * PDF.js PDFViewerApplication object.
@@ -38,37 +68,40 @@ export default class PDFMetadata {
    */
   constructor(app) {
     /** @type {Promise<PDFViewerApplication>} */
-    this._loaded = new Promise(resolve => {
-      const finish = () => {
-        window.removeEventListener('documentload', finish);
-        window.removeEventListener('documentloaded', finish);
-        app.eventBus?.off('documentloaded', finish);
+    this._loaded = pdfViewerInitialized(app).then(() => {
+      // Check if document has already loaded.
+      if (app.downloadComplete) {
+        return app;
+      }
 
-        resolve(app);
-      };
+      return new Promise(resolve => {
+        const finish = () => {
+          if (app.eventBus) {
+            app.eventBus.off('documentload', finish);
+            app.eventBus.off('documentloaded', finish);
+          } else {
+            window.removeEventListener('documentload', finish);
+          }
+          resolve(app);
+        };
 
-      if (app.downloadComplete) {
-        resolve(app);
-      } else {
         // Listen for "documentloaded" event which signals that the document
         // has been downloaded and the first page has been rendered.
-
-        // Newer versions of PDF.js (>= v2.5.207) report events only via
-        // the PDFViewerApplication's own event bus.
-        app.initializedPromise?.then(() => {
-          app.eventBus?.on('documentloaded', finish);
-        });
-
-        // Older versions of PDF.js (< v2.5.207) dispatch events to the DOM
-        // instead or as well.
-
-        // PDF.js >= v2.0.943.
-        // See https://github.com/mozilla/pdf.js/commit/7bc4bfcc8b7f52b14107f0a551becdf01643c5c2
-        window.addEventListener('documentloaded', finish);
-
-        // PDF.js < v2.0.943.
-        window.addEventListener('documentload', finish);
-      }
+        if (app.eventBus) {
+          // PDF.js >= v1.6.210 dispatch events via an internal event bus.
+          // PDF.js < v2.5.207 also dispatches events to the DOM.
+
+          // `documentloaded` is the preferred event in PDF.js >= v2.0.943.
+          // See https://github.com/mozilla/pdf.js/commit/7bc4bfcc8b7f52b14107f0a551becdf01643c5c2
+          app.eventBus.on('documentloaded', finish);
+
+          // `documentload` is dispatched by PDF.js < v2.1.266.
+          app.eventBus.on('documentload', finish);
+        } else {
+          // PDF.js < v1.6.210 dispatches events only to the DOM.
+          window.addEventListener('documentload', finish);
+        }
+      });
     });
   }
 

src/annotator/plugin/test/pdf-metadata-test.js

@@ -42,13 +42,25 @@ class FakePDFViewerApplication {
   /**
    * Initialize the "PDF viewer" as it would be when loading a document or
    * when a document fails to load.
+   *
+   * @param {string} url - Fake PDF URL
+   * @param {Object} options -
+   *   Options to simulate APIs of different versions of PDF.js.
+   *
+   *   @prop {boolean} domEvents - Whether events are emitted on the DOM
+   *   @prop {boolean} eventBusEvents - Whether the `eventBus` API is enabled
+   *   @prop {boolean} initializedPromise - Whether the `initializedPromise` API is enabled
    */
-  constructor(url = '', { domEvents = false, eventBusEvents = true } = {}) {
+  constructor(
+    url = '',
+    { domEvents = false, eventBusEvents = true, initializedPromise = true } = {}
+  ) {
     this.url = url;
     this.documentInfo = undefined;
     this.metadata = undefined;
     this.pdfDocument = null;
     this.dispatchDOMEvents = domEvents;
+    this.initialized = false;
 
     // Use `EventEmitter` as a fake version of PDF.js's `EventBus` class as the
     // API for subscribing to events is the same.
@@ -56,9 +68,16 @@ class FakePDFViewerApplication {
       this.eventBus = new EventEmitter();
     }
 
-    this.initializedPromise = new Promise(resolve => {
-      this._resolveInitializedPromise = resolve;
+    const initPromise = new Promise(resolve => {
+      this._resolveInitializedPromise = () => {
+        this.initialized = true;
+        resolve();
+      };
     });
+
+    if (initializedPromise) {
+      this.initializedPromise = initPromise;
+    }
   }
 
   /**
@@ -110,61 +129,89 @@ function delay(ms) {
 describe('annotator/plugin/pdf-metadata', function () {
   [
     {
-      // Oldest PDF.js versions (pre-2.x)
+      // PDF.js < 1.6.210: `documentload` event dispatched via DOM.
       eventName: 'documentload',
       domEvents: true,
       eventBusEvents: false,
+      initializedPromise: false,
+    },
+    {
+      // PDF.js >= 1.6.210: Event dispatch moved to internal event bus.
+      eventName: 'documentload',
+      domEvents: false,
+      eventBusEvents: true,
+      initializedPromise: false,
     },
     {
-      // Newer PDF.js versions (~ < 2.5.x)
+      // PDF.js >= 2.1.266: Deprecated `documentload` event was removed.
       eventName: 'documentloaded',
-      domEvents: true,
-      eventBusEvents: false,
+      domEvents: false,
+      eventBusEvents: true,
+      initializedPromise: false,
     },
     {
-      // Current PDF.js versions (>= 2.5.x)
+      // PDF.js >= 2.4.456: `initializedPromise` API was introduced.
       eventName: 'documentloaded',
       domEvents: false,
       eventBusEvents: true,
+      initializedPromise: true,
     },
-  ].forEach(({ eventName, domEvents = false, eventBusEvents = false }, i) => {
-    it(`waits for PDF to load (${i})`, async () => {
-      const fakeApp = new FakePDFViewerApplication('', {
-        domEvents,
-        eventBusEvents,
-      });
-      const pdfMetadata = new PDFMetadata(fakeApp);
+  ].forEach(
+    ({ eventName, domEvents, eventBusEvents, initializedPromise }, i) => {
+      it(`waits for PDF to load (${i})`, async () => {
+        const fakeApp = new FakePDFViewerApplication('', {
+          domEvents,
+          eventBusEvents,
+          initializedPromise,
+        });
+        const pdfMetadata = new PDFMetadata(fakeApp);
+
+        fakeApp.completeInit();
 
-      fakeApp.completeInit();
+        // Request the PDF URL before the document has finished loading.
+        const uriPromise = pdfMetadata.getUri();
 
-      // Give `PDFMetadata` a chance to register the "documentloaded" event listener.
-      await delay(0);
+        // Simulate a short delay in completing PDF.js initialization and
+        // loading the PDF.
+        //
+        // Note that this delay is longer than the `app.initialized` polling
+        // interval in `pdfViewerInitialized`.
+        await delay(10);
 
-      fakeApp.finishLoading({
-        eventName,
+        fakeApp.finishLoading({
+          eventName,
+          url: 'http://fake.com',
+          fingerprint: 'fakeFingerprint',
+        });
+
+        assert.equal(await uriPromise, 'http://fake.com/');
+      });
+    }
+  );
+
+  // The `initializedPromise` param simulates different versions of PDF.js with
+  // and without the `PDFViewerApplication.initializedPromise` API.
+  [true, false].forEach(initializedPromise => {
+    it('does not wait for the PDF to load if it has already loaded', function () {
+      const fakePDFViewerApplication = new FakePDFViewerApplication('', {
+        initializedPromise,
+      });
+      fakePDFViewerApplication.completeInit();
+      fakePDFViewerApplication.finishLoading({
         url: 'http://fake.com',
         fingerprint: 'fakeFingerprint',
       });
-
-      assert.equal(await pdfMetadata.getUri(), 'http://fake.com/');
-    });
-  });
-
-  it('does not wait for the PDF to load if it has already loaded', function () {
-    const fakePDFViewerApplication = new FakePDFViewerApplication();
-    fakePDFViewerApplication.finishLoading({
-      url: 'http://fake.com',
-      fingerprint: 'fakeFingerprint',
-    });
-    const pdfMetadata = new PDFMetadata(fakePDFViewerApplication);
-    return pdfMetadata.getUri().then(function (uri) {
-      assert.equal(uri, 'http://fake.com/');
+      const pdfMetadata = new PDFMetadata(fakePDFViewerApplication);
+      return pdfMetadata.getUri().then(function (uri) {
+        assert.equal(uri, 'http://fake.com/');
+      });
     });
   });
 
   describe('metadata sources', function () {
     let pdfMetadata;
     const fakePDFViewerApplication = new FakePDFViewerApplication();
+    fakePDFViewerApplication.completeInit();
     fakePDFViewerApplication.finishLoading({
       fingerprint: 'fakeFingerprint',
       title: 'fakeTitle',
@@ -187,6 +234,7 @@ describe('annotator/plugin/pdf-metadata', function () {
 
       it('returns the fingerprint as a URN when the PDF URL is a local file', function () {
         const fakePDFViewerApplication = new FakePDFViewerApplication();
+        fakePDFViewerApplication.completeInit();
         fakePDFViewerApplication.finishLoading({
           url: 'file:///test.pdf',
           fingerprint: 'fakeFingerprint',
@@ -200,6 +248,7 @@ describe('annotator/plugin/pdf-metadata', function () {
 
       it('resolves relative URLs', () => {
         const fakePDFViewerApplication = new FakePDFViewerApplication();
+        fakePDFViewerApplication.completeInit();
         fakePDFViewerApplication.finishLoading({
           url: 'index.php?action=download&file_id=wibble',
           fingerprint: 'fakeFingerprint',
@@ -258,6 +307,7 @@ describe('annotator/plugin/pdf-metadata', function () {
       it('does not save file:// URLs in document metadata', function () {
         let pdfMetadata;
         const fakePDFViewerApplication = new FakePDFViewerApplication();
+        fakePDFViewerApplication.completeInit();
         fakePDFViewerApplication.finishLoading({
           fingerprint: 'fakeFingerprint',
           url: 'file://fake.pdf',

src/types/pdfjs.js

@@ -62,6 +62,8 @@
  * Defined in `web/pdf_viewer.js` in the PDF.js source.
  *
  * @prop {number} pagesCount
+ * @prop {EventBus} eventBus -
+ *   Reference to the global event bus. Added in PDF.js v1.6.210.
  * @prop {(page: number) => PDFPageView|null} getPageView
  */
 
@@ -80,12 +82,14 @@
  *
  * @typedef PDFViewerApplication
  * @prop {EventBus} [eventBus] -
- *   Global event bus. Since v1.6.210.
+ *   Global event bus. Since v1.6.210. This is not available until the PDF viewer
+ *   has been initialized. See `initialized` and `initializedPromise` properties.
  * @prop {PDFDocument} pdfDocument
  * @prop {PDFViewer} pdfViewer
  * @prop {boolean} downloadComplete
  * @prop {PDFDocumentInfo} documentInfo
  * @prop {Metadata} metadata
+ * @prop {boolean} initialized - Indicates that the PDF viewer is initialized.
  * @prop {Promise<void>} [initializedPromise] -
  *   Promise that resolves when PDF.js is initialized. Since v2.4.456.
  *   See https://github.com/mozilla/pdf.js/wiki/Third-party-viewer-usage#initialization-promise.