Merge pull request #19950 from timvandermeij/intermittent-all-await

Fix missing `await` for asynchronous method calls in the integration tests
Fix missing await for asynchronous method calls in the integration tests
2026-08-02 12:27:21 +02:00 · 2025-05-18 19:12:48 +02:00 · 2025-05-18 18:19:46 +02:00 · 2025-05-18 15:17:41 +02:00 · 2025-05-18 14:36:19 +02:00 · 2025-05-18 12:05:13 +02:00
5 changed files with 95 additions and 114 deletions
--- a/src/display/api.js
+++ b/src/display/api.js
@ -462,32 +462,14 @@ function getDocument(src = {}) {
        if (!url) {
          throw new Error("getDocument - no `url` parameter provided.");
        }
-        let NetworkStream;
-
-        if (
-          typeof PDFJSDev !== "undefined" &&
-          PDFJSDev.test("GENERIC") &&
-          isNodeJS
-        ) {
-          if (isValidFetchUrl(url)) {
-            if (
-              typeof fetch === "undefined" ||
-              typeof Response === "undefined" ||
-              !("body" in Response.prototype)
-            ) {
-              throw new Error(
-                "getDocument - the Fetch API was disabled in Node.js, see `--no-experimental-fetch`."
-              );
-            }
-            NetworkStream = PDFFetchStream;
-          } else {
-            NetworkStream = PDFNodeStream;
-          }
-        } else {
-          NetworkStream = isValidFetchUrl(url)
-            ? PDFFetchStream
+        // eslint-disable-next-line no-nested-ternary
+        const NetworkStream = isValidFetchUrl(url)
+          ? PDFFetchStream
+          : typeof PDFJSDev !== "undefined" &&
+              PDFJSDev.test("GENERIC") &&
+              isNodeJS
+            ? PDFNodeStream
            : PDFNetworkStream;
-        }

        networkStream = new NetworkStream({
          url,
@ -722,6 +704,16 @@ class PDFDocumentLoadingTask {
 * main-thread memory usage, however it will take ownership of the TypedArrays.
 */
 class PDFDataRangeTransport {
+  #capability = Promise.withResolvers();
+
+  #progressiveDoneListeners = [];
+
+  #progressiveReadListeners = [];
+
+  #progressListeners = [];
+
+  #rangeListeners = [];
+
  /**
   * @param {number} length
   * @param {Uint8Array|null} initialData
@ -738,40 +730,34 @@ class PDFDataRangeTransport {
    this.initialData = initialData;
    this.progressiveDone = progressiveDone;
    this.contentDispositionFilename = contentDispositionFilename;
-
-    this._rangeListeners = [];
-    this._progressListeners = [];
-    this._progressiveReadListeners = [];
-    this._progressiveDoneListeners = [];
-    this._readyCapability = Promise.withResolvers();
  }

  /**
   * @param {function} listener
   */
  addRangeListener(listener) {
-    this._rangeListeners.push(listener);
+    this.#rangeListeners.push(listener);
  }

  /**
   * @param {function} listener
   */
  addProgressListener(listener) {
-    this._progressListeners.push(listener);
+    this.#progressListeners.push(listener);
  }

  /**
   * @param {function} listener
   */
  addProgressiveReadListener(listener) {
-    this._progressiveReadListeners.push(listener);
+    this.#progressiveReadListeners.push(listener);
  }

  /**
   * @param {function} listener
   */
  addProgressiveDoneListener(listener) {
-    this._progressiveDoneListeners.push(listener);
+    this.#progressiveDoneListeners.push(listener);
  }

  /**
@ -779,7 +765,7 @@ class PDFDataRangeTransport {
   * @param {Uint8Array|null} chunk
   */
  onDataRange(begin, chunk) {
-    for (const listener of this._rangeListeners) {
+    for (const listener of this.#rangeListeners) {
      listener(begin, chunk);
    }
  }
@ -789,8 +775,8 @@ class PDFDataRangeTransport {
   * @param {number|undefined} total
   */
  onDataProgress(loaded, total) {
-    this._readyCapability.promise.then(() => {
-      for (const listener of this._progressListeners) {
+    this.#capability.promise.then(() => {
+      for (const listener of this.#progressListeners) {
        listener(loaded, total);
      }
    });
@ -800,23 +786,23 @@ class PDFDataRangeTransport {
   * @param {Uint8Array|null} chunk
   */
  onDataProgressiveRead(chunk) {
-    this._readyCapability.promise.then(() => {
-      for (const listener of this._progressiveReadListeners) {
+    this.#capability.promise.then(() => {
+      for (const listener of this.#progressiveReadListeners) {
        listener(chunk);
      }
    });
  }

  onDataProgressiveDone() {
-    this._readyCapability.promise.then(() => {
-      for (const listener of this._progressiveDoneListeners) {
+    this.#capability.promise.then(() => {
+      for (const listener of this.#progressiveDoneListeners) {
        listener();
      }
    });
  }

  transportReady() {
-    this._readyCapability.resolve();
+    this.#capability.resolve();
  }

  /**
--- a/test/integration/freetext_editor_spec.mjs
+++ b/test/integration/freetext_editor_spec.mjs
@ -669,7 +669,7 @@ describe("FreeText Editor", () => {
            }
          );

-          page.evaluate(() => {
+          await page.evaluate(() => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
@ -1290,7 +1290,7 @@ describe("FreeText Editor", () => {
            ".selectedEditor .internal"
          );

-          page.evaluate(() => {
+          await page.evaluate(() => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
--- a/test/integration/highlight_editor_spec.mjs
+++ b/test/integration/highlight_editor_spec.mjs
@ -626,7 +626,7 @@ describe("Highlight Editor", () => {
          const { width: prevWidth } = await getRect(page, editorSelector);

          value = 24;
-          page.evaluate(val => {
+          await page.evaluate(val => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
@ -763,7 +763,7 @@ describe("Highlight Editor", () => {

          const { width: prevWidth } = await getRect(page, editorSelector);

-          page.evaluate(val => {
+          await page.evaluate(val => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
--- a/test/integration/ink_editor_spec.mjs
+++ b/test/integration/ink_editor_spec.mjs
@ -302,7 +302,7 @@ describe("Ink Editor", () => {
          await page.mouse.up();
          await awaitPromise(clickHandle);

-          page.mouse.click(rect.x - 10, rect.y + 10);
+          await page.mouse.click(rect.x - 10, rect.y + 10);
          await page.waitForSelector(`${getEditorSelector(0)}.disabled`);
        })
      );
@ -583,7 +583,7 @@ describe("Ink Editor", () => {
          }

          const red = "#ff0000";
-          page.evaluate(value => {
+          await page.evaluate(value => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
@ -763,7 +763,7 @@ describe("Ink Editor", () => {
          await selectEditor(page, pdfjsA);

          const red = "#ff0000";
-          page.evaluate(value => {
+          await page.evaluate(value => {
            window.PDFViewerApplication.eventBus.dispatch(
              "switchannotationeditorparams",
              {
--- a/test/integration/stamp_editor_spec.mjs
+++ b/test/integration/stamp_editor_spec.mjs
@ -631,6 +631,7 @@ describe("Stamp Editor", () => {
    });

    it("must check that the alt-text button is here when pasting in the second tab", async () => {
+      // Run sequentially to avoid clipboard issues.
      for (let i = 0; i < pages1.length; i++) {
        const [, page1] = pages1[i];
        await page1.bringToFront();
@ -1592,86 +1593,80 @@ describe("Stamp Editor", () => {
    });

    it("must check that deleting an image can be undone using the undo button", async () => {
-      await Promise.all(
-        pages.map(async ([browserName, page]) => {
-          await switchToStamp(page);
+      // Run sequentially to avoid clipboard issues.
+      for (const [, page] of pages) {
+        await switchToStamp(page);

-          const editorSelector = getEditorSelector(0);
-          await copyImage(page, "../images/firefox_logo.png", editorSelector);
-          await page.waitForSelector(editorSelector);
-          await waitForSerialized(page, 1);
+        const editorSelector = getEditorSelector(0);
+        await copyImage(page, "../images/firefox_logo.png", editorSelector);
+        await page.waitForSelector(editorSelector);
+        await waitForSerialized(page, 1);

-          await page.waitForSelector(`${editorSelector} button.delete`);
-          await page.click(`${editorSelector} button.delete`);
-          await waitForSerialized(page, 0);
-          await page.waitForSelector("#editorUndoBar", { visible: true });
+        await page.waitForSelector(`${editorSelector} button.delete`);
+        await page.click(`${editorSelector} button.delete`);
+        await waitForSerialized(page, 0);
+        await page.waitForSelector("#editorUndoBar", { visible: true });

-          await page.waitForSelector("#editorUndoBarUndoButton", {
-            visible: true,
-          });
-          await page.click("#editorUndoBarUndoButton");
-          await waitForSerialized(page, 1);
-          await page.waitForSelector(editorSelector);
-          await page.waitForSelector(`${editorSelector} canvas`);
-        })
-      );
+        await page.waitForSelector("#editorUndoBarUndoButton", {
+          visible: true,
+        });
+        await page.click("#editorUndoBarUndoButton");
+        await waitForSerialized(page, 1);
+        await page.waitForSelector(editorSelector);
+        await page.waitForSelector(`${editorSelector} canvas`);
+      }
    });

    it("must check that the undo deletion popup displays the correct message", async () => {
-      await Promise.all(
-        pages.map(async ([browserName, page]) => {
-          await switchToStamp(page);
+      // Run sequentially to avoid clipboard issues.
+      for (const [, page] of pages) {
+        await switchToStamp(page);

-          const editorSelector = getEditorSelector(0);
-          await copyImage(page, "../images/firefox_logo.png", editorSelector);
-          await page.waitForSelector(editorSelector);
-          await waitForSerialized(page, 1);
+        const editorSelector = getEditorSelector(0);
+        await copyImage(page, "../images/firefox_logo.png", editorSelector);
+        await page.waitForSelector(editorSelector);
+        await waitForSerialized(page, 1);

-          await page.waitForSelector(`${editorSelector} button.delete`);
-          await page.click(`${editorSelector} button.delete`);
-          await waitForSerialized(page, 0);
+        await page.waitForSelector(`${editorSelector} button.delete`);
+        await page.click(`${editorSelector} button.delete`);
+        await waitForSerialized(page, 0);

-          await page.waitForFunction(() => {
-            const messageElement = document.querySelector(
-              "#editorUndoBarMessage"
-            );
-            return messageElement && messageElement.textContent.trim() !== "";
-          });
-          const message = await page.waitForSelector("#editorUndoBarMessage");
-          const messageText = await page.evaluate(
-            el => el.textContent,
-            message
+        await page.waitForFunction(() => {
+          const messageElement = document.querySelector(
+            "#editorUndoBarMessage"
          );
-          expect(messageText).toContain("Image removed");
-        })
-      );
+          return messageElement && messageElement.textContent.trim() !== "";
+        });
+        const message = await page.waitForSelector("#editorUndoBarMessage");
+        const messageText = await page.evaluate(el => el.textContent, message);
+        expect(messageText).toContain("Image removed");
+      }
    });

    it("must check that the popup disappears when a new image is inserted", async () => {
-      await Promise.all(
-        pages.map(async ([browserName, page]) => {
-          await switchToStamp(page);
+      // Run sequentially to avoid clipboard issues.
+      for (const [, page] of pages) {
+        await switchToStamp(page);

-          const editorSelector = getEditorSelector(0);
-          await copyImage(page, "../images/firefox_logo.png", editorSelector);
-          await page.waitForSelector(editorSelector);
-          await waitForSerialized(page, 1);
+        const editorSelector = getEditorSelector(0);
+        await copyImage(page, "../images/firefox_logo.png", editorSelector);
+        await page.waitForSelector(editorSelector);
+        await waitForSerialized(page, 1);

-          await page.waitForSelector(`${editorSelector} button.delete`);
-          await page.click(`${editorSelector} button.delete`);
-          await waitForSerialized(page, 0);
+        await page.waitForSelector(`${editorSelector} button.delete`);
+        await page.click(`${editorSelector} button.delete`);
+        await waitForSerialized(page, 0);

-          await page.waitForSelector("#editorUndoBar", { visible: true });
-          await page.click("#editorStampAddImage");
-          const newInput = await page.$("#stampEditorFileInput");
-          await newInput.uploadFile(
-            `${path.join(__dirname, "../images/firefox_logo.png")}`
-          );
-          await waitForImage(page, getEditorSelector(1));
-          await waitForSerialized(page, 1);
-          await page.waitForSelector("#editorUndoBar", { hidden: true });
-        })
-      );
+        await page.waitForSelector("#editorUndoBar", { visible: true });
+        await page.click("#editorStampAddImage");
+        const newInput = await page.$("#stampEditorFileInput");
+        await newInput.uploadFile(
+          `${path.join(__dirname, "../images/firefox_logo.png")}`
+        );
+        await waitForImage(page, getEditorSelector(1));
+        await waitForSerialized(page, 1);
+        await page.waitForSelector("#editorUndoBar", { hidden: true });
+      }
    });
  });
Author	SHA1	Message	Date
Tim van der Meij	60574fb7e3	Merge pull request #19950 from timvandermeij/intermittent-all-await Fix missing `await` for asynchronous method calls in the integration tests	2025-05-18 19:12:48 +02:00
Tim van der Meij	393e799e49	Fix missing `await` for asynchronous method calls in the integration tests The `Page.evaluate()` and `Mouse.click()` APIs in Puppeteer both return a promise; see https://pptr.dev/api/puppeteer.page.evaluate and https://pptr.dev/api/puppeteer.mouse.click, and should therefore be awaited before proceeding in tests to make sure that the test's behavior is deterministic and avoid intermittent failures. The following command was used to find potential places to fix: `grep -nEr "[^await\|return] page\." test/integration/*`	2025-05-18 18:19:46 +02:00
Tim van der Meij	819671d42f	Merge pull request #19949 from timvandermeij/intermittent-stamp-undo Fix intermittent failure in the stamp editor's undo-related integration tests	2025-05-18 15:17:41 +02:00
Tim van der Meij	6355dd7ded	Fix intermittent failure in the stamp editor's undo-related integration tests The clipboard, used via the `copyImage` helper function, is a shared resource, so access to it cannot happen concurrently because it could result in tests overwriting each other's contents. Most tests using the clipboard are therefore run sequentially, but only the stamp editor's undo-related tests weren't, so this commit fixes the issue by running those tests sequentially too.	2025-05-18 14:36:19 +02:00
Tim van der Meij	f6e4b1cf4a	Merge pull request #19945 from Snuffleupagus/NetworkStream-rm-Node-Error Remove Node.js-specific checks when using the Fetch API	2025-05-18 12:05:13 +02:00
Tim van der Meij	30bba5f165	Merge pull request #19944 from Snuffleupagus/PDFDataRangeTransport-private-fields Use private fields in the `PDFDataRangeTransport` class	2025-05-18 12:01:03 +02:00
Jonas Jenwald	a882195e9b	Remove Node.js-specific checks when using the Fetch API Given that Node.js has full support for the Fetch API since version 21, see the "History" data at https://nodejs.org/api/globals.html#fetch, it seems unnecessary for us to manually check for various globals before using it. Since our primary development target is browsers in general, and Firefox in particular, being able to remove Node.js-specific compatibility code is always helpful. Note that we still, for now, support Node.js version 20 and if the relevant globals are not available then Errors will instead be thrown from within the `PDFFetchStream` class.	2025-05-18 10:49:02 +02:00
Jonas Jenwald	99b23ea1f6	Use private fields in the `PDFDataRangeTransport` class	2025-05-18 10:16:46 +02:00