Prevent accidental miss-clicks outside multi-select checkboxes

hypothesis · Sep 9, 2024 · 4b60a07 · 4b60a07
1 parent 292038c
commit 4b60a07
Show file tree

Hide file tree

Showing 3 changed files with 86 additions and 33 deletions.
diff --git a/src/annotator/integrations/pdf-metadata.ts b/src/annotator/integrations/pdf-metadata.ts
@@ -49,6 +49,23 @@ function pdfViewerInitialized(app: PDFViewerApplication): Promise<void> {
   }
 }
 
+/**
+ * Wait for PDF to be downloaded.
+ *
+ * For PDF.js versions older than v4.5, we rely on
+ * `PDFViewerApplication.downloadComplete`.
+ * For newer PDF.js versions we wait for
+ * `PDFViewerApplication.pdfDocument.getDownloadInfo()` to resolve.
+ */
+async function isPDFDownloaded(app: PDFViewerApplication): Promise<boolean> {
+  if (app.downloadComplete !== undefined) {
+    return app.downloadComplete;
+  }
+
+  await app.pdfDocument.getDownloadInfo();
+  return true;
+}
+
 /**
  * PDFMetadata extracts metadata about a loading/loaded PDF document from a
  * PDF.js PDFViewerApplication object.
@@ -70,9 +87,10 @@ export class PDFMetadata {
    * @param app - The `PDFViewerApplication` global from PDF.js
    */
   constructor(app: PDFViewerApplication) {
-    this._loaded = pdfViewerInitialized(app).then(() => {
+    this._loaded = pdfViewerInitialized(app).then(async () => {
       // Check if document has already loaded.
-      if (app.downloadComplete) {
+      const isDownloadComplete = await isPDFDownloaded(app);
+      if (isDownloadComplete) {
         return app;
       }
 

diff --git a/src/annotator/integrations/test/pdf-metadata-test.js b/src/annotator/integrations/test/pdf-metadata-test.js
@@ -1,6 +1,7 @@
 import { delay } from '@hypothesis/frontend-testing';
 import EventEmitter from 'tiny-emitter';
 
+import { promiseWithResolvers } from '../../../shared/promise-with-resolvers';
 import { PDFMetadata } from '../pdf-metadata';
 
 /**
@@ -29,7 +30,18 @@ class FakeMetadata {
  * Fake implementation of PDF.js `window.PDFViewerApplication.pdfDocument`.
  */
 class FakePDFDocumentProxy {
-  constructor({
+  constructor() {
+    this._contentDispositionFilename = null;
+    this._info = null;
+    this._metadata = null;
+    this.fingerprint = null;
+
+    const { resolve, promise } = promiseWithResolvers();
+    this._downloadInfoResolver = resolve;
+    this._downloadInfoPromise = promise;
+  }
+
+  finishLoading({
     contentDispositionFilename = null,
     fingerprint,
     info,
@@ -43,6 +55,8 @@ class FakePDFDocumentProxy {
     this._info = info;
     this._metadata = metadata;
 
+    this._downloadInfoResolver({ length: 100 });
+
     if (newFingerprintAPI) {
       this.fingerprints = [fingerprint, null];
     } else {
@@ -57,6 +71,10 @@ class FakePDFDocumentProxy {
       metadata: this._metadata,
     };
   }
+
+  async getDownloadInfo() {
+    return this._downloadInfoPromise;
+  }
 }
 
 /**
@@ -77,6 +95,7 @@ class FakePDFViewerApplication {
    *   @prop {boolean} eventBusEvents - Whether the `eventBus` API is enabled
    *   @prop {boolean} initializedPromise - Whether the `initializedPromise` API is enabled
    *   @prop {boolean} newFingerprintAPI - Whether to emulate the new fingerprints API
+   *   @prop {boolean} withDownloadComplete - Whether to explicitly set `downloadComplete`
    */
   constructor(
     url = '',
@@ -85,15 +104,17 @@ class FakePDFViewerApplication {
       eventBusEvents = true,
       initializedPromise = true,
       newFingerprintAPI = true,
+      withDownloadComplete = true,
     } = {},
   ) {
     this.url = url;
     this.documentInfo = undefined;
     this.metadata = undefined;
-    this.pdfDocument = null;
+    this.pdfDocument = new FakePDFDocumentProxy();
     this.dispatchDOMEvents = domEvents;
     this.initialized = false;
     this.newFingerprintAPI = newFingerprintAPI;
+    this.downloadComplete = withDownloadComplete ? false : undefined;
 
     // Use `EventEmitter` as a fake version of PDF.js's `EventBus` class as the
     // API for subscribing to events is the same.
@@ -132,7 +153,7 @@ class FakePDFViewerApplication {
       info.Title = title;
     }
 
-    this.pdfDocument = new FakePDFDocumentProxy({
+    this.pdfDocument.finishLoading({
       contentDispositionFilename,
       fingerprint,
       info,
@@ -188,38 +209,36 @@ describe('PDFMetadata', () => {
       eventBusEvents: true,
       initializedPromise: true,
     },
-  ].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();
+    {
+      // PDF.js >= 4.5: `downloadComplete` prop was removed.
+      withDownloadComplete: false,
+    },
+  ].forEach(({ eventName, ...appOptions }, i) => {
+    it(`waits for PDF to load (${i})`, async () => {
+      const fakeApp = new FakePDFViewerApplication('', appOptions);
+      const pdfMetadata = new PDFMetadata(fakeApp);
 
-        // Request the PDF URL before the document has finished loading.
-        const uriPromise = pdfMetadata.getUri();
+      fakeApp.completeInit();
 
-        // 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);
+      // Request the PDF URL before the document has finished loading.
+      const uriPromise = pdfMetadata.getUri();
 
-        fakeApp.finishLoading({
-          eventName,
-          url: 'http://fake.com',
-          fingerprint: 'fakeFingerprint',
-        });
+      // 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);
 
-        assert.equal(await uriPromise, 'http://fake.com/');
+      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.

diff --git a/src/types/pdfjs.ts b/src/types/pdfjs.ts
@@ -66,6 +66,13 @@ export type PDFDocument = {
    */
   fingerprints?: [string, string | null];
   getMetadata(): Promise<PDFDocumentMetadata>;
+
+  /**
+   * @return A promise that is resolved when the document's data is loaded.
+   *         It is resolved with an {Object} that contains the `length` property
+   *         that indicates size of the PDF data in bytes.
+   */
+  getDownloadInfo(): Promise<{ length: number }>;
 };
 
 export type GetTextContentParameters = {
@@ -159,7 +166,16 @@ export type PDFViewerApplication = {
   eventBus?: EventBus;
   pdfDocument: PDFDocument;
   pdfViewer: PDFViewer;
-  downloadComplete: boolean;
+
+  /**
+   * Indicates the download of the PDF has completed.
+   * This prop is not set in PDF.js >=4.5, in which case you should use
+   * `PDFViewerApplication.pdfDocument.getDownloadInfo()` instead.
+   *
+   * @see {PDFDocument}
+   */
+  downloadComplete?: boolean;
+
   documentInfo: PDFDocumentInfo;
   metadata: Metadata;
   /**