Merge pull request #12853 from Snuffleupagus/BaseViewer-initializeScriptingEvents

Fix the initialization/resetting of scripting-related events in the `BaseViewer`

src/display/api.js

@@ -1140,12 +1140,9 @@ class PDFPageProxy {
    *   {Object} with JS actions.
    */
   getJSActions() {
-    if (!this._jsActionsPromise) {
-      this._jsActionsPromise = this._transport.getPageJSActions(
-        this._pageIndex
-      );
-    }
-    return this._jsActionsPromise;
+    return (this._jsActionsPromise ||= this._transport.getPageJSActions(
+      this._pageIndex
+    ));
   }
 
   /**

web/app.js

@@ -867,7 +867,9 @@ const PDFViewerApplication = {
     if (typeof PDFBug !== "undefined") {
       PDFBug.cleanup();
     }
-    return Promise.all(promises);
+    await Promise.all(promises);
+
+    return undefined;
   },
 
   /**
@@ -1564,31 +1566,22 @@ const PDFViewerApplication = {
     internalEvents.set("updatefromsandbox", updateFromSandbox);
 
     const visitedPages = new Map();
-    const pageOpen = ({ pageNumber }) => {
+    const pageOpen = ({ pageNumber, actionsPromise }) => {
       visitedPages.set(
         pageNumber,
         (async () => {
           // Avoid sending, and thus serializing, the `actions` data
-          // when the same page is open several times.
+          // when the same page is opened several times.
           let actions = null;
           if (!visitedPages.has(pageNumber)) {
-            // visitedPages doesn't contain pageNumber: first visit.
-
-            const pageView = this.pdfViewer.getPageView(
-              /* index = */ pageNumber - 1
-            );
-            if (pageView?.pdfPage) {
-              actions = await pageView.pdfPage.getJSActions();
-            } else {
-              actions = await pdfDocument.getPage(pageNumber).getJSActions();
-            }
+            actions = await actionsPromise;
 
             if (pdfDocument !== this.pdfDocument) {
               return; // The document was closed while the actions resolved.
             }
           }
 
-          this._scriptingInstance?.scripting.dispatchEventInSandbox({
+          await this._scriptingInstance?.scripting.dispatchEventInSandbox({
             id: "page",
             name: "PageOpen",
             pageNumber,
@@ -1599,20 +1592,21 @@ const PDFViewerApplication = {
     };
 
     const pageClose = async ({ pageNumber }) => {
-      const promise = visitedPages.get(pageNumber);
-      if (!promise) {
+      const actionsPromise = visitedPages.get(pageNumber);
+      if (!actionsPromise) {
+        // Ensure that the "pageclose" event was preceded by a "pageopen" event.
         return;
       }
       visitedPages.set(pageNumber, null);
 
-      // Wait for PageOpen has been sent.
-      await promise;
+      // Ensure that the "pageopen" event is handled first.
+      await actionsPromise;
 
       if (pdfDocument !== this.pdfDocument) {
         return; // The document was closed while the actions resolved.
       }
 
-      this._scriptingInstance?.scripting.dispatchEventInSandbox({
+      await this._scriptingInstance?.scripting.dispatchEventInSandbox({
         id: "page",
         name: "PageClose",
         pageNumber,
@@ -1678,8 +1672,7 @@ const PDFViewerApplication = {
       id: "doc",
       name: "Open",
     });
-
-    await pageOpen({ pageNumber: this.pdfViewer.currentPageNumber });
+    await this.pdfViewer.initializeScriptingEvents();
 
     // Used together with the integration-tests, see the `scriptingReady`
     // getter, to enable awaiting full initialization of the scripting/sandbox.

web/base_viewer.js

@@ -215,7 +215,6 @@ class BaseViewer {
     if (this.removePageBorders) {
       this.viewer.classList.add("removePageBorders");
     }
-    this._initializeScriptingEvents();
     // Defer the dispatching of this event, to give other viewer components
     // time to initialize *and* register 'baseviewerinit' event listeners.
     Promise.resolve().then(() => {
@@ -654,7 +653,6 @@ class BaseViewer {
     this._pagesCapability = createPromiseCapability();
     this._scrollMode = ScrollMode.VERTICAL;
     this._spreadMode = SpreadMode.NONE;
-    this._pageOpenPendingSet?.clear();
 
     if (this._onBeforeDraw) {
       this.eventBus._off("pagerender", this._onBeforeDraw);
@@ -664,6 +662,8 @@ class BaseViewer {
       this.eventBus._off("pagerendered", this._onAfterDraw);
       this._onAfterDraw = null;
     }
+    this._resetScriptingEvents();
+
     // Remove the pages from the DOM...
     this.viewer.textContent = "";
     // ... and reset the Scroll mode CSS class(es) afterwards.
@@ -1507,15 +1507,13 @@ class BaseViewer {
     this.update();
   }
 
-  /**
-   * @private
-   */
-  _initializeScriptingEvents() {
-    if (!this.enableScripting) {
+  initializeScriptingEvents() {
+    if (!this.enableScripting || this._pageOpenPendingSet) {
       return;
     }
     const eventBus = this.eventBus,
-      pageOpenPendingSet = (this._pageOpenPendingSet ||= new Set());
+      pageOpenPendingSet = (this._pageOpenPendingSet = new Set()),
+      scriptingEvents = (this._scriptingEvents ||= Object.create(null));
 
     const dispatchPageClose = pageNumber => {
       if (pageOpenPendingSet.has(pageNumber)) {
@@ -1523,42 +1521,71 @@ class BaseViewer {
       }
       eventBus.dispatch("pageclose", { source: this, pageNumber });
     };
-    const dispatchPageOpen = (pageNumber, force = false) => {
+    const dispatchPageOpen = pageNumber => {
       const pageView = this._pages[pageNumber - 1];
-      if (force || pageView?.renderingState === RenderingStates.FINISHED) {
+      if (pageView?.renderingState === RenderingStates.FINISHED) {
         pageOpenPendingSet.delete(pageNumber);
 
-        eventBus.dispatch("pageopen", { source: this, pageNumber });
+        eventBus.dispatch("pageopen", {
+          source: this,
+          pageNumber,
+          actionsPromise: pageView.pdfPage?.getJSActions(),
+        });
       } else {
         pageOpenPendingSet.add(pageNumber);
       }
     };
 
-    eventBus._on("pagechanging", ({ pageNumber, previous }) => {
+    scriptingEvents.onPageChanging = ({ pageNumber, previous }) => {
       if (pageNumber === previous) {
         return; // The active page didn't change.
       }
       dispatchPageClose(previous);
       dispatchPageOpen(pageNumber);
-    });
+    };
+    eventBus._on("pagechanging", scriptingEvents.onPageChanging);
 
-    eventBus._on("pagerendered", ({ pageNumber }) => {
+    scriptingEvents.onPageRendered = ({ pageNumber }) => {
       if (!pageOpenPendingSet.has(pageNumber)) {
         return; // No pending "pageopen" event for the newly rendered page.
       }
       if (pageNumber !== this._currentPageNumber) {
         return; // The newly rendered page is no longer the current one.
       }
-      dispatchPageOpen(pageNumber, /* force = */ true);
-    });
+      dispatchPageOpen(pageNumber);
+    };
+    eventBus._on("pagerendered", scriptingEvents.onPageRendered);
 
-    eventBus._on("pagesinit", () => {
-      dispatchPageOpen(this._currentPageNumber);
-    });
-
-    eventBus._on("pagesdestroy", () => {
+    scriptingEvents.onPagesDestroy = () => {
       dispatchPageClose(this._currentPageNumber);
-    });
+    };
+    eventBus._on("pagesdestroy", scriptingEvents.onPagesDestroy);
+
+    // Ensure that a "pageopen" event is dispatched for the initial page.
+    dispatchPageOpen(this._currentPageNumber);
+  }
+
+  /**
+   * @private
+   */
+  _resetScriptingEvents() {
+    if (!this.enableScripting || !this._pageOpenPendingSet) {
+      return;
+    }
+    const eventBus = this.eventBus,
+      scriptingEvents = this._scriptingEvents;
+
+    // Remove the event listeners.
+    eventBus._off("pagechanging", scriptingEvents.onPageChanging);
+    scriptingEvents.onPageChanging = null;
+
+    eventBus._off("pagerendered", scriptingEvents.onPageRendered);
+    scriptingEvents.onPageRendered = null;
+
+    eventBus._off("pagesdestroy", scriptingEvents.onPagesDestroy);
+    scriptingEvents.onPagesDestroy = null;
+
+    this._pageOpenPendingSet = null;
   }
 }