Skip to content

Commit

Permalink
Merge pull request #11655 from Snuffleupagus/rm-getGlobalEventBus
Browse files Browse the repository at this point in the history
[api-minor] Remove the `getGlobalEventBus` viewer functionality, and the `eventBusDispatchToDOM` option/preference (PR 11631 follow-up)
  • Loading branch information
timvandermeij authored Mar 30, 2020
2 parents 35c9f8d + 664b79a commit ce17276
Show file tree
Hide file tree
Showing 13 changed files with 39 additions and 100 deletions.
4 changes: 0 additions & 4 deletions extensions/chromium/preferences_schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,10 +58,6 @@
"type": "boolean",
"default": false
},
"eventBusDispatchToDOM": {
"type": "boolean",
"default": false
},
"pdfBugEnabled": {
"title": "Enable debugging tools",
"description": "Whether to enable debugging tools.",
Expand Down
50 changes: 1 addition & 49 deletions test/unit/ui_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.");
}
Expand All @@ -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);

Expand Down
10 changes: 7 additions & 3 deletions web/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,10 @@ class DefaultExternalServices {
metaKey: true,
});
}

static get isInAutomation() {
return shadow(this, "isInAutomation", false);
}
}

const PDFViewerApplication = {
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 0 additions & 5 deletions web/app_options.js
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
3 changes: 1 addition & 2 deletions web/base_viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
CSS_UNITS,
DEFAULT_SCALE,
DEFAULT_SCALE_VALUE,
getGlobalEventBus,
getVisibleElements,
isPortraitOrientation,
isValidRotation,
Expand Down Expand Up @@ -145,7 +144,7 @@ class BaseViewer {

this.container = options.container;
this.viewer = options.viewer || options.container.firstElementChild;
this.eventBus = options.eventBus || getGlobalEventBus();
this.eventBus = options.eventBus;
this.linkService = options.linkService || new SimpleLinkService();
this.downloadManager = options.downloadManager || null;
this.findController = options.findController || null;
Expand Down
7 changes: 7 additions & 0 deletions web/firefoxcom.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 2 additions & 2 deletions web/pdf_find_bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
* limitations under the License.
*/

import { getGlobalEventBus, NullL10n } from "./ui_utils.js";
import { FindState } from "./pdf_find_controller.js";
import { NullL10n } from "./ui_utils.js";

const MATCHES_COUNT_LIMIT = 1000;

Expand All @@ -38,7 +38,7 @@ class PDFFindBar {
this.findResultsCount = options.findResultsCount || null;
this.findPreviousButton = options.findPreviousButton || null;
this.findNextButton = options.findNextButton || null;
this.eventBus = eventBus || getGlobalEventBus();
this.eventBus = eventBus;
this.l10n = l10n;

// Add event listeners to the DOM elements.
Expand Down
4 changes: 2 additions & 2 deletions web/pdf_find_controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
* limitations under the License.
*/

import { getGlobalEventBus, scrollIntoView } from "./ui_utils.js";
import { createPromiseCapability } from "pdfjs-lib";
import { getCharacterType } from "./pdf_find_utils.js";
import { scrollIntoView } from "./ui_utils.js";

const FindState = {
FOUND: 0,
Expand Down Expand Up @@ -69,7 +69,7 @@ class PDFFindController {
*/
constructor({ linkService, eventBus }) {
this._linkService = linkService;
this._eventBus = eventBus || getGlobalEventBus();
this._eventBus = eventBus;

this._reset();
eventBus._on("findbarclose", this._onFindBarClose.bind(this));
Expand Down
3 changes: 1 addition & 2 deletions web/pdf_history.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
*/

import {
getGlobalEventBus,
isValidRotation,
parseQueryString,
waitOnEventOrTimeout,
Expand Down Expand Up @@ -59,7 +58,7 @@ class PDFHistory {
*/
constructor({ linkService, eventBus }) {
this.linkService = linkService;
this.eventBus = eventBus || getGlobalEventBus();
this.eventBus = eventBus;

this._initialized = false;
this._fingerprint = "";
Expand Down
4 changes: 2 additions & 2 deletions web/pdf_link_service.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
* limitations under the License.
*/

import { getGlobalEventBus, parseQueryString } from "./ui_utils.js";
import { parseQueryString } from "./ui_utils.js";

/**
* @typedef {Object} PDFLinkServiceOptions
Expand Down Expand Up @@ -44,7 +44,7 @@ class PDFLinkService {
externalLinkEnabled = true,
ignoreDestinationZoom = false,
} = {}) {
this.eventBus = eventBus || getGlobalEventBus();
this.eventBus = eventBus;
this.externalLinkTarget = externalLinkTarget;
this.externalLinkRel = externalLinkRel;
this.externalLinkEnabled = externalLinkEnabled;
Expand Down
3 changes: 1 addition & 2 deletions web/pdf_page_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
approximateFraction,
CSS_UNITS,
DEFAULT_SCALE,
getGlobalEventBus,
getOutputScale,
NullL10n,
RendererType,
Expand Down Expand Up @@ -92,7 +91,7 @@ class PDFPageView {
this.useOnlyCssZoom = options.useOnlyCssZoom || false;
this.maxCanvasPixels = options.maxCanvasPixels || MAX_CANVAS_PIXELS;

this.eventBus = options.eventBus || getGlobalEventBus();
this.eventBus = options.eventBus;
this.renderingQueue = options.renderingQueue;
this.textLayerFactory = options.textLayerFactory;
this.annotationLayerFactory = options.annotationLayerFactory;
Expand Down
3 changes: 1 addition & 2 deletions web/text_layer_builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
* limitations under the License.
*/

import { getGlobalEventBus } from "./ui_utils.js";
import { renderTextLayer } from "pdfjs-lib";

const EXPAND_DIVS_TIMEOUT = 300; // ms
Expand Down Expand Up @@ -45,7 +44,7 @@ class TextLayerBuilder {
enhanceTextSelection = false,
}) {
this.textLayerDiv = textLayerDiv;
this.eventBus = eventBus || getGlobalEventBus();
this.eventBus = eventBus;
this.textContent = null;
this.textContentItemsStr = [];
this.textContentStream = null;
Expand Down
39 changes: 14 additions & 25 deletions web/ui_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand All @@ -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;
}
}

Expand All @@ -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);
}
Expand Down Expand Up @@ -848,7 +846,10 @@ class EventBus {
});
externalListeners = null;
}
if (this._dispatchToDOM) {
if (
(typeof PDFJSDev === "undefined" || PDFJSDev.test("MOZCENTRAL")) &&
this._isInAutomation
) {
dispatchDOMEvent(eventName, args);
}
}
Expand Down Expand Up @@ -884,17 +885,6 @@ class EventBus {
}
}

let globalEventBus = null;
function getGlobalEventBus(dispatchToDOM = false) {
console.error(
"getGlobalEventBus is deprecated, use a manually created EventBus instance instead."
);
if (!globalEventBus) {
globalEventBus = new EventBus({ dispatchToDOM });
}
return globalEventBus;
}

function clamp(v, min, max) {
return Math.min(Math.max(v, min), max);
}
Expand Down Expand Up @@ -1013,7 +1003,6 @@ export {
SpreadMode,
NullL10n,
EventBus,
getGlobalEventBus,
clamp,
ProgressBar,
getPDFFileNameFromURL,
Expand Down

0 comments on commit ce17276

Please sign in to comment.