From 486c84321556d9652015fe5477fd8e017fddecb1 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 28 Aug 2018 09:56:26 +0200 Subject: [PATCH 1/3] Add `source` parameters to all remaining `EventBus.dispatch` calls that are currently missing those This is necessary for subsequent patches, and will help avoid unnecessary event re-dispatching in cases where the event source is `window`. --- web/app.js | 9 ++++++--- web/pdf_presentation_mode.js | 8 ++++---- web/toolbar.js | 16 ++++++++-------- 3 files changed, 18 insertions(+), 15 deletions(-) diff --git a/web/app.js b/web/app.js index f3d063500..34a4a6adf 100644 --- a/web/app.js +++ b/web/app.js @@ -160,7 +160,7 @@ let PDFViewerApplication = { this.l10n.translate(appContainer).then(() => { // Dispatch the 'localized' event on the `eventBus` once the viewer // has been fully initialized and translated. - this.eventBus.dispatch('localized'); + this.eventBus.dispatch('localized', { source: this, }); }); this.initialized = true; @@ -1371,14 +1371,15 @@ let PDFViewerApplication = { }; _boundEvents.windowHashChange = () => { eventBus.dispatch('hashchange', { + source: window, hash: document.location.hash.substring(1), }); }; _boundEvents.windowBeforePrint = () => { - eventBus.dispatch('beforeprint'); + eventBus.dispatch('beforeprint', { source: window, }); }; _boundEvents.windowAfterPrint = () => { - eventBus.dispatch('afterprint'); + eventBus.dispatch('afterprint', { source: window, }); }; window.addEventListener('wheel', webViewerWheel); @@ -1560,6 +1561,7 @@ function webViewerInitialized() { return; } PDFViewerApplication.eventBus.dispatch('fileinputchange', { + source: this, fileInput: evt.target, }); }); @@ -1578,6 +1580,7 @@ function webViewerInitialized() { return; } PDFViewerApplication.eventBus.dispatch('fileinputchange', { + source: this, fileInput: evt.dataTransfer, }); }); diff --git a/web/pdf_presentation_mode.js b/web/pdf_presentation_mode.js index c1794b5b4..93eedb2bf 100644 --- a/web/pdf_presentation_mode.js +++ b/web/pdf_presentation_mode.js @@ -60,19 +60,19 @@ class PDFPresentationMode { if (contextMenuItems) { contextMenuItems.contextFirstPage.addEventListener('click', () => { this.contextMenuOpen = false; - this.eventBus.dispatch('firstpage'); + this.eventBus.dispatch('firstpage', { source: this, }); }); contextMenuItems.contextLastPage.addEventListener('click', () => { this.contextMenuOpen = false; - this.eventBus.dispatch('lastpage'); + this.eventBus.dispatch('lastpage', { source: this, }); }); contextMenuItems.contextPageRotateCw.addEventListener('click', () => { this.contextMenuOpen = false; - this.eventBus.dispatch('rotatecw'); + this.eventBus.dispatch('rotatecw', { source: this, }); }); contextMenuItems.contextPageRotateCcw.addEventListener('click', () => { this.contextMenuOpen = false; - this.eventBus.dispatch('rotateccw'); + this.eventBus.dispatch('rotateccw', { source: this, }); }); } } diff --git a/web/toolbar.js b/web/toolbar.js index 19b43f35e..986f638f4 100644 --- a/web/toolbar.js +++ b/web/toolbar.js @@ -100,19 +100,19 @@ class Toolbar { let self = this; items.previous.addEventListener('click', function() { - eventBus.dispatch('previouspage'); + eventBus.dispatch('previouspage', { source: self, }); }); items.next.addEventListener('click', function() { - eventBus.dispatch('nextpage'); + eventBus.dispatch('nextpage', { source: self, }); }); items.zoomIn.addEventListener('click', function() { - eventBus.dispatch('zoomin'); + eventBus.dispatch('zoomin', { source: self, }); }); items.zoomOut.addEventListener('click', function() { - eventBus.dispatch('zoomout'); + eventBus.dispatch('zoomout', { source: self, }); }); items.pageNumber.addEventListener('click', function() { @@ -137,19 +137,19 @@ class Toolbar { }); items.presentationModeButton.addEventListener('click', function() { - eventBus.dispatch('presentationmode'); + eventBus.dispatch('presentationmode', { source: self, }); }); items.openFile.addEventListener('click', function() { - eventBus.dispatch('openfile'); + eventBus.dispatch('openfile', { source: self, }); }); items.print.addEventListener('click', function() { - eventBus.dispatch('print'); + eventBus.dispatch('print', { source: self, }); }); items.download.addEventListener('click', function() { - eventBus.dispatch('download'); + eventBus.dispatch('download', { source: self, }); }); // Suppress context menus for some controls. From 7bc4bfcc8b7f52b14107f0a551becdf01643c5c2 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 28 Aug 2018 10:06:19 +0200 Subject: [PATCH 2/3] Add 'documentinit'/'documentloaded' events to `PDFViewerApplication.load` The new events follow the same naming pattern as the 'pagesinit'/'pagesloaded' events dispatched on `BaseViewer` instances, and the intention is to allow the eventual removal of 'documentload'. --- web/app.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/web/app.js b/web/app.js index 34a4a6adf..c5bd4ff3a 100644 --- a/web/app.js +++ b/web/app.js @@ -897,6 +897,9 @@ let PDFViewerApplication = { this.loadingBar.hide(); firstPagePromise.then(() => { + this.eventBus.dispatch('documentloaded', { source: this, }); + // TODO: Remove the following event, i.e. 'documentload', + // once the mozilla-central tests have been updated. this.eventBus.dispatch('documentload', { source: this, }); }); }); @@ -1000,6 +1003,7 @@ let PDFViewerApplication = { this.setInitialView(hash, { rotation, sidebarView, scrollMode, spreadMode, }); + this.eventBus.dispatch('documentinit', { source: this, }); // Make all navigation keys work on document load, // unless the viewer is embedded in a web page. From 0b1f41c5b3885a0b916a3fa29ecf7f5fb835a55a Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Tue, 28 Aug 2018 10:26:18 +0200 Subject: [PATCH 3/3] Add general support for re-dispatching events, on `EventBus` instances, to the DOM This patch is the first step to be able to eventually get rid of the `attachDOMEventsToEventBus` function, by allowing `EventBus` instances to simply re-dispatch most[1] events to the DOM. Note that the re-dispatching is purposely implemented to occur *after* all registered `EventBus` listeners have been serviced, to prevent the ordering issues that necessitated the duplicated page/scale-change events. The DOM events are currently necessary for the `mozilla-central` tests, see https://hg.mozilla.org/mozilla-central/file/tip/browser/extensions/pdfjs/test, and perhaps also for custom deployments of the PDF.js default viewer. Once this have landed, and been successfully uplifted to `mozilla-central`, I intent to submit a patch to update the test-code to utilize the new preference. This will thus, eventually, make it possible to remove the `attachDOMEventsToEventBus` functionality. *Please note:* I've successfully ran all `mozilla-central` tests locally, with these patches applied. --- [1] The exception being events that originated on the `window` or `document`, since those are already globally available anyway. --- extensions/chromium/preferences_schema.json | 4 +++ test/unit/ui_utils_spec.js | 39 +++++++++++++++++++++ web/app.js | 3 +- web/app_options.js | 5 +++ web/default_preferences.json | 1 + web/dom_events.js | 11 +++--- web/ui_utils.js | 35 +++++++++++++++++- 7 files changed, 91 insertions(+), 7 deletions(-) diff --git a/extensions/chromium/preferences_schema.json b/extensions/chromium/preferences_schema.json index 1833f61a6..696090120 100644 --- a/extensions/chromium/preferences_schema.json +++ b/extensions/chromium/preferences_schema.json @@ -47,6 +47,10 @@ "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 0e93bd8ac..4ba2a22e0 100644 --- a/test/unit/ui_utils_spec.js +++ b/test/unit/ui_utils_spec.js @@ -262,6 +262,45 @@ describe('ui_utils', function() { eventBus.dispatch('test'); expect(count).toEqual(2); }); + + it('should not, by default, re-dispatch to DOM', function(done) { + if (isNodeJS()) { + pending('Document in not supported in Node.js.'); + } + const eventBus = new EventBus(); + let count = 0; + eventBus.on('test', function() { + count++; + }); + document.addEventListener('test', function() { + count++; + }); + eventBus.dispatch('test'); + + Promise.resolve().then(() => { + expect(count).toEqual(1); + 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() { + count++; + }); + document.addEventListener('test', function() { + count++; + }); + eventBus.dispatch('test'); + + Promise.resolve().then(() => { + expect(count).toEqual(2); + done(); + }); + }); }); describe('isValidRotation', function() { diff --git a/web/app.js b/web/app.js index c5bd4ff3a..4304c4cb9 100644 --- a/web/app.js +++ b/web/app.js @@ -288,7 +288,8 @@ let PDFViewerApplication = { return new Promise((resolve, reject) => { this.overlayManager = new OverlayManager(); - let eventBus = appConfig.eventBus || getGlobalEventBus(); + const dispatchToDOM = AppOptions.get('eventBusDispatchToDOM'); + let eventBus = appConfig.eventBus || getGlobalEventBus(dispatchToDOM); this.eventBus = eventBus; let pdfRenderingQueue = new PDFRenderingQueue(); diff --git a/web/app_options.js b/web/app_options.js index 83b7d3d47..08fb28848 100644 --- a/web/app_options.js +++ b/web/app_options.js @@ -68,6 +68,11 @@ const defaultOptions = { value: false, kind: OptionKind.VIEWER, }, + eventBusDispatchToDOM: { + /** @type {boolean} */ + value: false, + kind: OptionKind.VIEWER, + }, externalLinkRel: { /** @type {string} */ value: 'noopener noreferrer nofollow', diff --git a/web/default_preferences.json b/web/default_preferences.json index 78a998495..3389a5cf8 100644 --- a/web/default_preferences.json +++ b/web/default_preferences.json @@ -4,6 +4,7 @@ "sidebarViewOnLoad": 0, "cursorToolOnLoad": 0, "enableWebGL": false, + "eventBusDispatchToDOM": false, "pdfBugEnabled": false, "disableRange": false, "disableStream": false, diff --git a/web/dom_events.js b/web/dom_events.js index 1d7250f00..51a6999d5 100644 --- a/web/dom_events.js +++ b/web/dom_events.js @@ -129,12 +129,13 @@ function attachDOMEventsToEventBus(eventBus) { } let globalEventBus = null; -function getGlobalEventBus() { - if (globalEventBus) { - return globalEventBus; +function getGlobalEventBus(dispatchToDOM = false) { + if (!globalEventBus) { + globalEventBus = new EventBus({ dispatchToDOM, }); + if (!dispatchToDOM) { + attachDOMEventsToEventBus(globalEventBus); + } } - globalEventBus = new EventBus(); - attachDOMEventsToEventBus(globalEventBus); return globalEventBus; } diff --git a/web/ui_utils.js b/web/ui_utils.js index 7c615f2f8..d87b78c15 100644 --- a/web/ui_utils.js +++ b/web/ui_utils.js @@ -683,8 +683,9 @@ let animationStarted = new Promise(function (resolve) { * used. */ class EventBus { - constructor() { + constructor({ dispatchToDOM = false, } = {}) { this._listeners = Object.create(null); + this._dispatchToDOM = dispatchToDOM === true; } on(eventName, listener) { @@ -708,6 +709,9 @@ class EventBus { dispatch(eventName) { let eventListeners = this._listeners[eventName]; if (!eventListeners || eventListeners.length === 0) { + if (this._dispatchToDOM) { + this._dispatchDOMEvent(eventName); + } return; } // Passing all arguments after the eventName to the listeners. @@ -717,6 +721,35 @@ class EventBus { eventListeners.slice(0).forEach(function (listener) { listener.apply(null, args); }); + if (this._dispatchToDOM) { + this._dispatchDOMEvent(eventName, args); + } + } + + /** + * @private + */ + _dispatchDOMEvent(eventName, args = null) { + if (!this._dispatchToDOM) { + return; + } + const details = Object.create(null); + if (args && args.length > 0) { + const obj = args[0]; + for (let key in obj) { + const value = obj[key]; + if (key === 'source') { + if (value === window || value === document) { + return; // No need to re-dispatch (already) global events. + } + continue; // Ignore the `source` property. + } + details[key] = value; + } + } + const event = document.createEvent('CustomEvent'); + event.initCustomEvent(eventName, true, true, details); + document.dispatchEvent(event); } }