Skip to content

Commit 74db085

Browse files
committed
Re-factor how "internal" EventBus listeners are handled in the viewer
Currently the viewer uses semi-private `EventBus.prototype.{_on, _off}` methods, to try and ensure that all internal viewer state is updated *before* any "external" listeners are invoked. For all use-cases outside of the viewer, e.g in the integration-tests, the `EventBus.prototype.{on, off}` methods are supposed to be used instead. Unfortunately this isn't currently enforced in any way, except (hopefully) during review, and generally speaking it's not really possible to prevent the semi-private methods being used (e.g. by third-party users). Hence this patch adds a new `INTERNAL_EVT` property which is *not* exposed anywhere (neither in the API nor globally), and whose value is generated at build-time, that the viewer uses to mark its `EventBus` listeners are internal. This allows us to remove the semi-private `EventBus` methods, which helps to simplify that class a little bit.
1 parent 19d95c8 commit 74db085

32 files changed

Lines changed: 633 additions & 410 deletions

gulpfile.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,7 @@ const DEFINES = Object.freeze({
116116
WORKER_THREAD: false,
117117
TESTING: undefined,
118118
COVERAGE: undefined,
119+
INTERNAL_EVT: crypto.randomUUID(),
119120
// The main build targets:
120121
GENERIC: false,
121122
MOZCENTRAL: false,

src/display/editor/tools.js

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import {
3636
stopEvent,
3737
} from "../display_utils.js";
3838
import { FloatingToolbar } from "./toolbar.js";
39+
import { internalOpt } from "../../shared/internal_evt.js";
3940

4041
function bindEvents(obj, element, names) {
4142
for (const name of names) {
@@ -939,17 +940,21 @@ class AnnotationEditorUIManager {
939940
this.#signatureManager = signatureManager;
940941
this.#pdfDocument = pdfDocument;
941942
this._eventBus = eventBus;
942-
eventBus._on("editingaction", this.onEditingAction.bind(this), { signal });
943-
eventBus._on("pagechanging", this.onPageChanging.bind(this), { signal });
944-
eventBus._on("scalechanging", this.onScaleChanging.bind(this), { signal });
945-
eventBus._on("rotationchanging", this.onRotationChanging.bind(this), {
946-
signal,
947-
});
948-
eventBus._on("setpreference", this.onSetPreference.bind(this), { signal });
949-
eventBus._on(
943+
944+
const evtOpts = { signal, ...internalOpt };
945+
eventBus.on("editingaction", this.onEditingAction.bind(this), evtOpts);
946+
eventBus.on("pagechanging", this.onPageChanging.bind(this), evtOpts);
947+
eventBus.on("scalechanging", this.onScaleChanging.bind(this), evtOpts);
948+
eventBus.on(
949+
"rotationchanging",
950+
this.onRotationChanging.bind(this),
951+
evtOpts
952+
);
953+
eventBus.on("setpreference", this.onSetPreference.bind(this), evtOpts);
954+
eventBus.on(
950955
"switchannotationeditorparams",
951956
evt => this.updateParams(evt.type, evt.value),
952-
{ signal }
957+
evtOpts
953958
);
954959
window.addEventListener(
955960
"pointerdown",
@@ -1222,7 +1227,7 @@ class AnnotationEditorUIManager {
12221227
const { resolve, promise } = Promise.withResolvers();
12231228
const onEditorsRendered = evt => {
12241229
if (evt.pageNumber === pageNumber) {
1225-
this._eventBus._off("editorsrendered", onEditorsRendered);
1230+
this._eventBus.off("editorsrendered", onEditorsRendered);
12261231
resolve();
12271232
}
12281233
};

src/shared/internal_evt.js

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/* Copyright 2026 Mozilla Foundation
2+
*
3+
* Licensed under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License.
5+
* You may obtain a copy of the License at
6+
*
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
*
9+
* Unless required by applicable law or agreed to in writing, software
10+
* distributed under the License is distributed on an "AS IS" BASIS,
11+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
* See the License for the specific language governing permissions and
13+
* limitations under the License.
14+
*/
15+
16+
/**
17+
* Keep this file in sync with `web/internal_evt.js`.
18+
*/
19+
20+
const INTERNAL_EVT =
21+
typeof PDFJSDev !== "undefined" && !PDFJSDev.test("TESTING")
22+
? PDFJSDev.eval("INTERNAL_EVT")
23+
: "internalEvent";
24+
25+
const internalOpt = Object.freeze({ internal: INTERNAL_EVT });
26+
27+
export { INTERNAL_EVT, internalOpt };

test/integration/reorganize_pages_spec.mjs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3556,7 +3556,7 @@ describe("Reorganize Pages View", () => {
35563556
await waitAndClick(page, getThumbnailSelector(2));
35573557

35583558
const handleMerged = await createPromise(page, resolve => {
3559-
window.PDFViewerApplication.eventBus._on(
3559+
window.PDFViewerApplication.eventBus.on(
35603560
"thumbnailsloaded",
35613561
resolve,
35623562
{ once: true }
@@ -3628,7 +3628,7 @@ describe("Reorganize Pages View", () => {
36283628
await waitForThumbnailVisible(page, 1);
36293629

36303630
const handleMerged = await createPromise(page, resolve => {
3631-
window.PDFViewerApplication.eventBus._on(
3631+
window.PDFViewerApplication.eventBus.on(
36323632
"thumbnailsloaded",
36333633
resolve,
36343634
{ once: true }

web/alt_text_manager.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
import { DOMSVGFactory } from "pdfjs-lib";
17+
import { internalOpt } from "./internal_evt.js";
1718

1819
class AltTextManager {
1920
#clickAC = null;
@@ -170,8 +171,9 @@ class AltTextManager {
170171
this.#uiManager.removeEditListeners();
171172

172173
this.#resizeAC = new AbortController();
173-
this.#eventBus._on("resize", this.#setPosition.bind(this), {
174+
this.#eventBus.on("resize", this.#setPosition.bind(this), {
174175
signal: this.#resizeAC.signal,
176+
...internalOpt,
175177
});
176178

177179
try {

web/annotation_editor_params.js

Lines changed: 40 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import {
2121
getRGBA,
2222
Util,
2323
} from "pdfjs-lib";
24+
import { internalOpt } from "./internal_evt.js";
2425

2526
/**
2627
* @typedef {Object} AnnotationEditorParamsOptions
@@ -155,42 +156,46 @@ class AnnotationEditorParams {
155156
dispatchEvent("CREATE");
156157
});
157158

158-
eventBus._on("annotationeditorparamschanged", evt => {
159-
for (const [type, value] of evt.details) {
160-
switch (type) {
161-
case AnnotationEditorParamsType.FREETEXT_SIZE:
162-
editorFreeTextFontSize.value = value;
163-
break;
164-
case AnnotationEditorParamsType.FREETEXT_COLOR:
165-
editorFreeTextColor.value = value;
166-
break;
167-
case AnnotationEditorParamsType.INK_COLOR:
168-
updateInkColor(value);
169-
break;
170-
case AnnotationEditorParamsType.INK_THICKNESS:
171-
editorInkThickness.value = value;
172-
break;
173-
case AnnotationEditorParamsType.INK_OPACITY:
174-
updateInkOpacity(value);
175-
break;
176-
case AnnotationEditorParamsType.HIGHLIGHT_COLOR:
177-
eventBus.dispatch("mainhighlightcolorpickerupdatecolor", {
178-
source: this,
179-
value,
180-
});
181-
break;
182-
case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
183-
editorFreeHighlightThickness.value = value;
184-
break;
185-
case AnnotationEditorParamsType.HIGHLIGHT_FREE:
186-
editorFreeHighlightThickness.disabled = !value;
187-
break;
188-
case AnnotationEditorParamsType.HIGHLIGHT_SHOW_ALL:
189-
editorHighlightShowAll.setAttribute("aria-pressed", value);
190-
break;
159+
eventBus.on(
160+
"annotationeditorparamschanged",
161+
evt => {
162+
for (const [type, value] of evt.details) {
163+
switch (type) {
164+
case AnnotationEditorParamsType.FREETEXT_SIZE:
165+
editorFreeTextFontSize.value = value;
166+
break;
167+
case AnnotationEditorParamsType.FREETEXT_COLOR:
168+
editorFreeTextColor.value = value;
169+
break;
170+
case AnnotationEditorParamsType.INK_COLOR:
171+
updateInkColor(value);
172+
break;
173+
case AnnotationEditorParamsType.INK_THICKNESS:
174+
editorInkThickness.value = value;
175+
break;
176+
case AnnotationEditorParamsType.INK_OPACITY:
177+
updateInkOpacity(value);
178+
break;
179+
case AnnotationEditorParamsType.HIGHLIGHT_COLOR:
180+
eventBus.dispatch("mainhighlightcolorpickerupdatecolor", {
181+
source: this,
182+
value,
183+
});
184+
break;
185+
case AnnotationEditorParamsType.HIGHLIGHT_THICKNESS:
186+
editorFreeHighlightThickness.value = value;
187+
break;
188+
case AnnotationEditorParamsType.HIGHLIGHT_FREE:
189+
editorFreeHighlightThickness.disabled = !value;
190+
break;
191+
case AnnotationEditorParamsType.HIGHLIGHT_SHOW_ALL:
192+
editorHighlightShowAll.setAttribute("aria-pressed", value);
193+
break;
194+
}
191195
}
192-
}
193-
});
196+
},
197+
internalOpt
198+
);
194199
}
195200
}
196201

web/annotation_layer_builder.js

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import {
3535
setLayerDimensions,
3636
Util,
3737
} from "pdfjs-lib";
38+
import { internalOpt } from "./internal_evt.js";
3839
import { PresentationModeState } from "./ui_utils.js";
3940

4041
/**
@@ -75,7 +76,7 @@ class AnnotationLayerBuilder {
7576

7677
#onAppend = null;
7778

78-
#eventAbortController = null;
79+
#eventAC = null;
7980

8081
#linksInjected = false;
8182

@@ -190,15 +191,15 @@ class AnnotationLayerBuilder {
190191
if (this.linkService.isInPresentationMode) {
191192
this.#updatePresentationModeState(PresentationModeState.FULLSCREEN);
192193
}
193-
if (!this.#eventAbortController) {
194-
this.#eventAbortController = new AbortController();
194+
if (!this.#eventAC) {
195+
this.#eventAC = new AbortController();
195196

196-
this._eventBus?._on(
197+
this._eventBus?.on(
197198
"presentationmodechanged",
198199
evt => {
199200
this.#updatePresentationModeState(evt.state);
200201
},
201-
{ signal: this.#eventAbortController.signal }
202+
{ signal: this.#eventAC.signal, ...internalOpt }
202203
);
203204
}
204205
}
@@ -221,8 +222,8 @@ class AnnotationLayerBuilder {
221222
cancel() {
222223
this._cancelled = true;
223224

224-
this.#eventAbortController?.abort();
225-
this.#eventAbortController = null;
225+
this.#eventAC?.abort();
226+
this.#eventAC = null;
226227
}
227228

228229
hide(internal = false) {

0 commit comments

Comments
 (0)