From 32b0e00ba7e0635a224041a7e00c28e40c04b4e2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 6 Jan 2021 14:43:05 +0100 Subject: [PATCH 1/2] Don't dispatch "pageclose" events if a "pageopen" wasn't dispatched for the page (PR 12747 follow-up) Given that "pageopen" events are not guaranteed to occur, if the page becomes inactive *before* it finishes rendering, we should probably also avoid dispatching a "pageclose" event in that case to avoid confusing/inconsistent state in any event handlers. --- web/base_viewer.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index 0103797ba..c4a2e8103 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -1517,6 +1517,9 @@ class BaseViewer { const { eventBus } = this; const dispatchPageClose = pageNumber => { + if (this._pageOpenPendingSet?.has(pageNumber)) { + return; // No "pageopen" event was dispatched for the previous page. + } eventBus.dispatch("pageclose", { source: this, pageNumber }); }; const dispatchPageOpen = (pageNumber, force = false) => { @@ -1542,10 +1545,7 @@ class BaseViewer { }); eventBus._on("pagerendered", ({ pageNumber }) => { - if (!this._pageOpenPendingSet) { - return; // No pending "pageopen" events. - } - if (!this._pageOpenPendingSet.has(pageNumber)) { + if (!this._pageOpenPendingSet?.has(pageNumber)) { return; // No pending "pageopen" event for the newly rendered page. } if (pageNumber !== this._currentPageNumber) { From 373230185af7012dfc502c83bd22ddbc42a95dbf Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Wed, 6 Jan 2021 16:09:34 +0100 Subject: [PATCH 2/2] Unconditionally initialize the `this._pageOpenPendingSet` in `BaseViewer._initializeScriptingEvents` (PR 12747 follow-up) With the code dispatching a "pageopen" event on the existing (general) `BaseViewer` event "pagesinit", in practice this means that the `Set` is always being created. Hence we can simplify the method overall, by always initializing the `this._pageOpenPendingSet` property. --- web/base_viewer.js | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/web/base_viewer.js b/web/base_viewer.js index c4a2e8103..60cfbcb2e 100644 --- a/web/base_viewer.js +++ b/web/base_viewer.js @@ -654,7 +654,7 @@ class BaseViewer { this._pagesCapability = createPromiseCapability(); this._scrollMode = ScrollMode.VERTICAL; this._spreadMode = SpreadMode.NONE; - this._pageOpenPendingSet = null; + this._pageOpenPendingSet?.clear(); if (this._onBeforeDraw) { this.eventBus._off("pagerender", this._onBeforeDraw); @@ -1514,10 +1514,11 @@ class BaseViewer { if (!this.enableScripting) { return; } - const { eventBus } = this; + const eventBus = this.eventBus, + pageOpenPendingSet = (this._pageOpenPendingSet ||= new Set()); const dispatchPageClose = pageNumber => { - if (this._pageOpenPendingSet?.has(pageNumber)) { + if (pageOpenPendingSet.has(pageNumber)) { return; // No "pageopen" event was dispatched for the previous page. } eventBus.dispatch("pageclose", { source: this, pageNumber }); @@ -1525,14 +1526,11 @@ class BaseViewer { const dispatchPageOpen = (pageNumber, force = false) => { const pageView = this._pages[pageNumber - 1]; if (force || pageView?.renderingState === RenderingStates.FINISHED) { - this._pageOpenPendingSet?.delete(pageNumber); + pageOpenPendingSet.delete(pageNumber); eventBus.dispatch("pageopen", { source: this, pageNumber }); } else { - if (!this._pageOpenPendingSet) { - this._pageOpenPendingSet = new Set(); - } - this._pageOpenPendingSet.add(pageNumber); + pageOpenPendingSet.add(pageNumber); } }; @@ -1545,7 +1543,7 @@ class BaseViewer { }); eventBus._on("pagerendered", ({ pageNumber }) => { - if (!this._pageOpenPendingSet?.has(pageNumber)) { + if (!pageOpenPendingSet.has(pageNumber)) { return; // No pending "pageopen" event for the newly rendered page. } if (pageNumber !== this._currentPageNumber) {