From 664b79abe0ca4ea6437eda510989276d49a35961 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 27 Feb 2020 15:25:33 +0100 Subject: [PATCH] [api-minor] Remove the `eventBusDispatchToDOM` option/preference, and thus the general ability to dispatch "viewer components" events to the DOM This functionality was only added to the default viewer for backwards compatibility and to support the various PDF viewer tests in mozilla-central, with the intention to eventually remove it completely. While the different mozilla-central tests cannot be *easily* converted from DOM events, it's however possible to limit that functionality to only MOZCENTRAL builds *and* when tests are running. Rather than depending of the re-dispatching of internal events to the DOM, the default viewer can instead be used in e.g. the following way: ```javascript document.addEventListener("webviewerloaded", function() { PDFViewerApplication.initializedPromise.then(function() { // The viewer has now been initialized, and its properties can be accessed. PDFViewerApplication.eventBus.on("pagerendered", function(event) { console.log("Has rendered page number: " + event.pageNumber); }); }); }); ``` --- extensions/chromium/preferences_schema.json | 4 -- test/unit/ui_utils_spec.js | 50 +-------------------- web/app.js | 10 +++-- web/app_options.js | 5 --- web/firefoxcom.js | 7 +++ web/ui_utils.js | 27 +++++------ 6 files changed, 29 insertions(+), 74 deletions(-) diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index 5b428bc6e..add31dae3 100644 --- a/extensions/chromium/preferences_schema.json +++ b/extensions/chromium/preferences_schema.json @@ -58,10 +58,6 @@ "type": "boolean", "default": false }, - "eventBusDispatchToDOM": { - "type": "boolean", - "default": false - }, "pdfBugEnabled": { "title": "Enable debugging tools", "description": "Whether to enable debugging tools.", diff --git a/test/unit/ui_utils_spec.js b/test/unit/ui_utils_spec.js index b60c783d3..132aa3b72 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -312,7 +312,7 @@ describe("ui_utils", function() { expect(count).toEqual(2); }); - it("should not, by default, re-dispatch to DOM", function(done) { + it("should not re-dispatch to DOM", function(done) { if (isNodeJS) { pending("Document in not supported in Node.js."); } @@ -329,54 +329,6 @@ describe("ui_utils", function() { eventBus.dispatch("test"); - Promise.resolve().then(() => { - expect(count).toEqual(1); - - document.removeEventListener("test", domEventListener); - done(); - }); - }); - it("should re-dispatch to DOM", function(done) { - if (isNodeJS) { - pending("Document in not supported in Node.js."); - } - const eventBus = new EventBus({ dispatchToDOM: true }); - let count = 0; - eventBus.on("test", function(evt) { - expect(evt).toEqual(undefined); - count++; - }); - function domEventListener(evt) { - expect(evt.detail).toEqual({}); - count++; - } - document.addEventListener("test", domEventListener); - - eventBus.dispatch("test"); - - Promise.resolve().then(() => { - expect(count).toEqual(2); - - document.removeEventListener("test", domEventListener); - done(); - }); - }); - it("should re-dispatch to DOM, with arguments (without internal listeners)", function(done) { - if (isNodeJS) { - pending("Document in not supported in Node.js."); - } - const eventBus = new EventBus({ dispatchToDOM: true }); - let count = 0; - function domEventListener(evt) { - expect(evt.detail).toEqual({ abc: 123 }); - count++; - } - document.addEventListener("test", domEventListener); - - eventBus.dispatch("test", { - abc: 123, - }); - Promise.resolve().then(() => { expect(count).toEqual(1); diff --git a/web/app.js b/web/app.js index b1ebe23e1..155462fa3 100644 --- a/web/app.js +++ b/web/app.js @@ -125,6 +125,10 @@ class DefaultExternalServices { metaKey: true, }); } + + static get isInAutomation() { + return shadow(this, "isInAutomation", false); + } } const PDFViewerApplication = { @@ -339,13 +343,13 @@ const PDFViewerApplication = { async _initializeViewerComponents() { const appConfig = this.appConfig; - this.overlayManager = new OverlayManager(); - const eventBus = appConfig.eventBus || - new EventBus({ dispatchToDOM: AppOptions.get("eventBusDispatchToDOM") }); + new EventBus({ isInAutomation: this.externalServices.isInAutomation }); this.eventBus = eventBus; + this.overlayManager = new OverlayManager(); + const pdfRenderingQueue = new PDFRenderingQueue(); pdfRenderingQueue.onIdle = this.cleanup.bind(this); this.pdfRenderingQueue = pdfRenderingQueue; diff --git a/web/app_options.js b/web/app_options.js index 3e4736299..3fa1187a1 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -66,11 +66,6 @@ const defaultOptions = { value: false, kind: OptionKind.VIEWER + OptionKind.PREFERENCE, }, - eventBusDispatchToDOM: { - /** @type {boolean} */ - value: false, - kind: OptionKind.VIEWER + OptionKind.PREFERENCE, - }, externalLinkRel: { /** @type {string} */ value: "noopener noreferrer nofollow", diff --git a/web/firefoxcom.js b/web/firefoxcom.js index 75e55a2db..d0167f888 100644 --- a/web/firefoxcom.js +++ b/web/firefoxcom.js @@ -345,6 +345,13 @@ class FirefoxExternalServices extends DefaultExternalServices { ); return shadow(this, "supportedMouseWheelZoomModifierKeys", support); } + + static get isInAutomation() { + // Returns the value of `Cu.isInAutomation`, which is only `true` when e.g. + // various test-suites are running in mozilla-central. + const isInAutomation = FirefoxCom.requestSync("isInAutomation"); + return shadow(this, "isInAutomation", isInAutomation); + } } PDFViewerApplication.externalServices = FirefoxExternalServices; diff --git a/web/ui_utils.js b/web/ui_utils.js index 1841860dd..5ea91834a 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -760,6 +760,9 @@ const animationStarted = new Promise(function(resolve) { * NOTE: Only used to support various PDF viewer tests in `mozilla-central`. */ function dispatchDOMEvent(eventName, args = null) { + if (typeof PDFJSDev !== "undefined" && !PDFJSDev.test("MOZCENTRAL")) { + throw new Error("Not implemented: dispatchDOMEvent"); + } const details = Object.create(null); if (args && args.length > 0) { const obj = args[0]; @@ -784,19 +787,11 @@ function dispatchDOMEvent(eventName, args = null) { * and `off` methods. To raise an event, the `dispatch` method shall be used. */ class EventBus { - constructor({ dispatchToDOM = false } = {}) { + constructor(options) { this._listeners = Object.create(null); - this._dispatchToDOM = dispatchToDOM === true; - if ( - typeof PDFJSDev !== "undefined" && - !PDFJSDev.test("MOZCENTRAL || TESTING") && - dispatchToDOM - ) { - console.error( - "The `eventBusDispatchToDOM` option/preference is deprecated, " + - "add event listeners to the EventBus instance rather than the DOM." - ); + if (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) { + this._isInAutomation = (options && options.isInAutomation) === true; } } @@ -819,7 +814,10 @@ class EventBus { dispatch(eventName) { const eventListeners = this._listeners[eventName]; if (!eventListeners || eventListeners.length === 0) { - if (this._dispatchToDOM) { + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) && + this._isInAutomation + ) { const args = Array.prototype.slice.call(arguments, 1); dispatchDOMEvent(eventName, args); } @@ -848,7 +846,10 @@ class EventBus { }); externalListeners = null; } - if (this._dispatchToDOM) { + if ( + (typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) && + this._isInAutomation + ) { dispatchDOMEvent(eventName, args); } }