From 739d7c6d774b6c1f1aa8ee2ab133ebb3c33d2453 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 29 Dec 2020 16:36:58 +0100 Subject: [PATCH] Support the `once` option, when registering `EventBus` listeners This follows the same principle as the `once` option that exists in the native `addEventListener` method, and will thus automatically remove an `EventBus` listener when it's invoked; see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#Parameters Finally, this patch also tweaks some the existing `EventBus`-code to use modern features such as optional chaining and logical assignment operators. --- test/unit/ui_utils_spec.js | 24 ++++++++++++++++++++++++ web/app.js | 12 +++++++----- web/pdf_history.js | 12 +++++++----- web/ui_utils.js | 38 ++++++++++++++++++++++---------------- 4 files changed, 60 insertions(+), 26 deletions(-) diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index ad3820717..927535dbd 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -312,6 +312,30 @@ describe("ui_utils", function () { expect(count).toEqual(2); }); + it("dispatch event to handlers with/without 'once' option", function () { + const eventBus = new EventBus(); + let multipleCount = 0, + onceCount = 0; + + eventBus.on("test", function () { + multipleCount++; + }); + eventBus.on( + "test", + function () { + onceCount++; + }, + { once: true } + ); + + eventBus.dispatch("test"); + eventBus.dispatch("test"); + eventBus.dispatch("test"); + + expect(multipleCount).toEqual(3); + expect(onceCount).toEqual(1); + }); + it("should not re-dispatch to DOM", function (done) { if (isNodeJS) { pending("Document in not supported in Node.js."); diff --git a/web/app.js b/web/app.js index e0019c289..098488e83 100644 --- a/web/app.js +++ b/web/app.js @@ -1497,11 +1497,13 @@ const PDFViewerApplication = { // It should be *extremely* rare for metadata to not have been resolved // when this code runs, but ensure that we handle that case here. await new Promise(resolve => { - const metadataLoaded = () => { - this.eventBus._off("metadataloaded", metadataLoaded); - resolve(); - }; - this.eventBus._on("metadataloaded", metadataLoaded); + this.eventBus._on( + "metadataloaded", + evt => { + resolve(); + }, + { once: true } + ); }); if (pdfDocument !== this.pdfDocument) { return; // The document was closed while the metadata resolved. diff --git a/web/pdf_history.js b/web/pdf_history.js index c35b16025..c45693e14 100644 --- a/web/pdf_history.js +++ b/web/pdf_history.js @@ -76,11 +76,13 @@ class PDFHistory { this.eventBus._on("pagesinit", () => { this._isPagesLoaded = false; - const onPagesLoaded = evt => { - this.eventBus._off("pagesloaded", onPagesLoaded); - this._isPagesLoaded = !!evt.pagesCount; - }; - this.eventBus._on("pagesloaded", onPagesLoaded); + this.eventBus._on( + "pagesloaded", + evt => { + this._isPagesLoaded = !!evt.pagesCount; + }, + { once: true } + ); }); } diff --git a/web/ui_utils.js b/web/ui_utils.js index 70f54f5e6..b44b55205 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -804,24 +804,32 @@ class EventBus { this._listeners = Object.create(null); if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) { - this._isInAutomation = (options && options.isInAutomation) === true; + this._isInAutomation = options?.isInAutomation === true; } } /** * @param {string} eventName * @param {function} listener + * @param {Object} [options] */ - on(eventName, listener) { - this._on(eventName, listener, { external: true }); + on(eventName, listener, options = null) { + this._on(eventName, listener, { + external: true, + once: options?.once, + }); } /** * @param {string} eventName * @param {function} listener + * @param {Object} [options] */ - off(eventName, listener) { - this._off(eventName, listener, { external: true }); + off(eventName, listener, options = null) { + this._off(eventName, listener, { + external: true, + once: options?.once, + }); } dispatch(eventName) { @@ -841,12 +849,12 @@ class EventBus { let externalListeners; // Making copy of the listeners array in case if it will be modified // during dispatch. - eventListeners.slice(0).forEach(function ({ listener, external }) { + eventListeners.slice(0).forEach(({ listener, external, once }) => { + if (once) { + this._off(eventName, listener); + } if (external) { - if (!externalListeners) { - externalListeners = []; - } - externalListeners.push(listener); + (externalListeners ||= []).push(listener); return; } listener.apply(null, args); @@ -854,7 +862,7 @@ class EventBus { // Dispatch any "external" listeners *after* the internal ones, to give the // viewer components time to handle events and update their state first. if (externalListeners) { - externalListeners.forEach(function (listener) { + externalListeners.forEach(listener => { listener.apply(null, args); }); externalListeners = null; @@ -871,13 +879,11 @@ class EventBus { * @ignore */ _on(eventName, listener, options = null) { - let eventListeners = this._listeners[eventName]; - if (!eventListeners) { - this._listeners[eventName] = eventListeners = []; - } + const eventListeners = (this._listeners[eventName] ||= []); eventListeners.push({ listener, - external: (options && options.external) === true, + external: options?.external === true, + once: options?.once === true, }); }