1
0
Fork 0
mirror of https://github.com/mozilla/pdf.js.git synced 2025-04-22 16:18:08 +02:00

[Editor] Remove event listeners with AbortSignal.any()

There's a fair number of event listeners in the editor-code that we're currently removing "manually", by keeping references to their event handler functions.
This was necessary since we have a "global" `AbortController` that applies to all event listeners used in the editor-code, however it's now possible to combine multiple `AbortSignal`s; please see https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static

Since this functionality is [fairly new](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal/any_static#browser_compatibility) the viewer will check that `AbortSignal.any()` is available before enabling the editing-functionality.
(It should hopefully be fairly straightforward, famous last words, for users to implement a polyfill to allow editing in older browsers.)

Finally, this patch also adds checks and test-only asserts to ensure that we don't add duplicate event listeners in various editor-code.
This commit is contained in:
Jonas Jenwald 2024-08-08 13:09:16 +02:00
parent 4569e88778
commit c0bf3d3c94
9 changed files with 202 additions and 193 deletions

View file

@ -61,11 +61,7 @@ class AnnotationEditorLayer {
#annotationLayer = null;
#boundPointerup = null;
#boundPointerdown = null;
#boundTextLayerPointerDown = null;
#clickAC = null;
#editorFocusTimeoutId = null;
@ -79,6 +75,8 @@ class AnnotationEditorLayer {
#textLayer = null;
#textSelectionAC = null;
#uiManager;
static _initialized = false;
@ -365,12 +363,14 @@ class AnnotationEditorLayer {
enableTextSelection() {
this.div.tabIndex = -1;
if (this.#textLayer?.div && !this.#boundTextLayerPointerDown) {
this.#boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this);
if (this.#textLayer?.div && !this.#textSelectionAC) {
this.#textSelectionAC = new AbortController();
const signal = this.#uiManager.combinedSignal(this.#textSelectionAC);
this.#textLayer.div.addEventListener(
"pointerdown",
this.#boundTextLayerPointerDown,
{ signal: this.#uiManager._signal }
this.#textLayerPointerDown.bind(this),
{ signal }
);
this.#textLayer.div.classList.add("highlighting");
}
@ -378,12 +378,10 @@ class AnnotationEditorLayer {
disableTextSelection() {
this.div.tabIndex = 0;
if (this.#textLayer?.div && this.#boundTextLayerPointerDown) {
this.#textLayer.div.removeEventListener(
"pointerdown",
this.#boundTextLayerPointerDown
);
this.#boundTextLayerPointerDown = null;
if (this.#textLayer?.div && this.#textSelectionAC) {
this.#textSelectionAC.abort();
this.#textSelectionAC = null;
this.#textLayer.div.classList.remove("highlighting");
}
}
@ -428,26 +426,23 @@ class AnnotationEditorLayer {
}
enableClick() {
if (this.#boundPointerdown) {
if (this.#clickAC) {
return;
}
const signal = this.#uiManager._signal;
this.#boundPointerdown = this.pointerdown.bind(this);
this.#boundPointerup = this.pointerup.bind(this);
this.div.addEventListener("pointerdown", this.#boundPointerdown, {
this.#clickAC = new AbortController();
const signal = this.#uiManager.combinedSignal(this.#clickAC);
this.div.addEventListener("pointerdown", this.pointerdown.bind(this), {
signal,
});
this.div.addEventListener("pointerup", this.pointerup.bind(this), {
signal,
});
this.div.addEventListener("pointerup", this.#boundPointerup, { signal });
}
disableClick() {
if (!this.#boundPointerdown) {
return;
}
this.div.removeEventListener("pointerdown", this.#boundPointerdown);
this.div.removeEventListener("pointerup", this.#boundPointerup);
this.#boundPointerdown = null;
this.#boundPointerup = null;
this.#clickAC?.abort();
this.#clickAC = null;
}
attach(editor) {
@ -611,8 +606,8 @@ class AnnotationEditorLayer {
return AnnotationEditorLayer.#editorTypes.get(this.#uiManager.getMode());
}
get _signal() {
return this.#uiManager._signal;
combinedSignal(ac) {
return this.#uiManager.combinedSignal(ac);
}
/**

View file

@ -54,9 +54,7 @@ class AnnotationEditor {
#savedDimensions = null;
#boundFocusin = this.focusin.bind(this);
#boundFocusout = this.focusout.bind(this);
#focusAC = null;
#focusedResizerName = "";
@ -758,16 +756,17 @@ class AnnotationEditor {
this.#altText?.toggle(false);
const boundResizerPointermove = this.#resizerPointermove.bind(this, name);
const savedDraggable = this._isDraggable;
this._isDraggable = false;
const signal = this._uiManager._signal;
const pointerMoveOptions = { passive: true, capture: true, signal };
const ac = new AbortController();
const signal = this._uiManager.combinedSignal(ac);
this.parent.togglePointerEvents(false);
window.addEventListener(
"pointermove",
boundResizerPointermove,
pointerMoveOptions
this.#resizerPointermove.bind(this, name),
{ passive: true, capture: true, signal }
);
window.addEventListener("contextmenu", noContextMenu, { signal });
const savedX = this.x;
@ -780,17 +779,10 @@ class AnnotationEditor {
window.getComputedStyle(event.target).cursor;
const pointerUpCallback = () => {
ac.abort();
this.parent.togglePointerEvents(true);
this.#altText?.toggle(true);
this._isDraggable = savedDraggable;
window.removeEventListener("pointerup", pointerUpCallback);
window.removeEventListener("blur", pointerUpCallback);
window.removeEventListener(
"pointermove",
boundResizerPointermove,
pointerMoveOptions
);
window.removeEventListener("contextmenu", noContextMenu);
this.parent.div.style.cursor = savedParentCursor;
this.div.style.cursor = savedCursor;
@ -1069,10 +1061,7 @@ class AnnotationEditor {
}
this.setInForeground();
const signal = this._uiManager._signal;
this.div.addEventListener("focusin", this.#boundFocusin, { signal });
this.div.addEventListener("focusout", this.#boundFocusout, { signal });
this.#addFocusListeners();
const [parentWidth, parentHeight] = this.parentDimensions;
if (this.parentRotation % 180 !== 0) {
@ -1132,14 +1121,14 @@ class AnnotationEditor {
const isSelected = this._uiManager.isSelected(this);
this._uiManager.setUpDragSession();
let pointerMoveOptions, pointerMoveCallback;
const signal = this._uiManager._signal;
const ac = new AbortController();
const signal = this._uiManager.combinedSignal(ac);
if (isSelected) {
this.div.classList.add("moving");
pointerMoveOptions = { passive: true, capture: true, signal };
this.#prevDragX = event.clientX;
this.#prevDragY = event.clientY;
pointerMoveCallback = e => {
const pointerMoveCallback = e => {
const { clientX: x, clientY: y } = e;
const [tx, ty] = this.screenToPageTranslation(
x - this.#prevDragX,
@ -1149,23 +1138,17 @@ class AnnotationEditor {
this.#prevDragY = y;
this._uiManager.dragSelectedEditors(tx, ty);
};
window.addEventListener(
"pointermove",
pointerMoveCallback,
pointerMoveOptions
);
window.addEventListener("pointermove", pointerMoveCallback, {
passive: true,
capture: true,
signal,
});
}
const pointerUpCallback = () => {
window.removeEventListener("pointerup", pointerUpCallback);
window.removeEventListener("blur", pointerUpCallback);
ac.abort();
if (isSelected) {
this.div.classList.remove("moving");
window.removeEventListener(
"pointermove",
pointerMoveCallback,
pointerMoveOptions
);
}
this.#hasBeenClicked = false;
@ -1323,15 +1306,24 @@ class AnnotationEditor {
return this.div && !this.isAttachedToDOM;
}
#addFocusListeners() {
if (this.#focusAC || !this.div) {
return;
}
this.#focusAC = new AbortController();
const signal = this._uiManager.combinedSignal(this.#focusAC);
this.div.addEventListener("focusin", this.focusin.bind(this), { signal });
this.div.addEventListener("focusout", this.focusout.bind(this), { signal });
}
/**
* Rebuild the editor in case it has been removed on undo.
*
* To implement in subclasses.
*/
rebuild() {
const signal = this._uiManager._signal;
this.div?.addEventListener("focusin", this.#boundFocusin, { signal });
this.div?.addEventListener("focusout", this.#boundFocusout, { signal });
this.#addFocusListeners();
}
/**
@ -1401,8 +1393,8 @@ class AnnotationEditor {
* It's used on ctrl+backspace action.
*/
remove() {
this.div.removeEventListener("focusin", this.#boundFocusin);
this.div.removeEventListener("focusout", this.#boundFocusout);
this.#focusAC?.abort();
this.#focusAC = null;
if (!this.isEmpty()) {
// The editor is removed but it can be back at some point thanks to

View file

@ -38,22 +38,14 @@ const EOL_PATTERN = /\r\n?|\n/g;
* Basic text editor in order to create a FreeTex annotation.
*/
class FreeTextEditor extends AnnotationEditor {
#boundEditorDivBlur = this.editorDivBlur.bind(this);
#boundEditorDivFocus = this.editorDivFocus.bind(this);
#boundEditorDivInput = this.editorDivInput.bind(this);
#boundEditorDivKeydown = this.editorDivKeydown.bind(this);
#boundEditorDivPaste = this.editorDivPaste.bind(this);
#color;
#content = "";
#editorDivId = `${this.id}-editor`;
#editModeAC = null;
#fontSize;
#initialData = null;
@ -307,20 +299,31 @@ class FreeTextEditor extends AnnotationEditor {
this.editorDiv.contentEditable = true;
this._isDraggable = false;
this.div.removeAttribute("aria-activedescendant");
const signal = this._uiManager._signal;
this.editorDiv.addEventListener("keydown", this.#boundEditorDivKeydown, {
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
assert(
!this.#editModeAC,
"No `this.#editModeAC` AbortController should exist."
);
}
this.#editModeAC = new AbortController();
const signal = this._uiManager.combinedSignal(this.#editModeAC);
this.editorDiv.addEventListener(
"keydown",
this.editorDivKeydown.bind(this),
{ signal }
);
this.editorDiv.addEventListener("focus", this.editorDivFocus.bind(this), {
signal,
});
this.editorDiv.addEventListener("focus", this.#boundEditorDivFocus, {
this.editorDiv.addEventListener("blur", this.editorDivBlur.bind(this), {
signal,
});
this.editorDiv.addEventListener("blur", this.#boundEditorDivBlur, {
this.editorDiv.addEventListener("input", this.editorDivInput.bind(this), {
signal,
});
this.editorDiv.addEventListener("input", this.#boundEditorDivInput, {
signal,
});
this.editorDiv.addEventListener("paste", this.#boundEditorDivPaste, {
this.editorDiv.addEventListener("paste", this.editorDivPaste.bind(this), {
signal,
});
}
@ -337,11 +340,9 @@ class FreeTextEditor extends AnnotationEditor {
this.editorDiv.contentEditable = false;
this.div.setAttribute("aria-activedescendant", this.#editorDivId);
this._isDraggable = true;
this.editorDiv.removeEventListener("keydown", this.#boundEditorDivKeydown);
this.editorDiv.removeEventListener("focus", this.#boundEditorDivFocus);
this.editorDiv.removeEventListener("blur", this.#boundEditorDivBlur);
this.editorDiv.removeEventListener("input", this.#boundEditorDivInput);
this.editorDiv.removeEventListener("paste", this.#boundEditorDivPaste);
this.#editModeAC?.abort();
this.#editModeAC = null;
// On Chrome, the focus is given to <body> when contentEditable is set to
// false, hence we focus the div.

View file

@ -700,34 +700,33 @@ class HighlightEditor extends AnnotationEditor {
width: parentWidth,
height: parentHeight,
} = textLayer.getBoundingClientRect();
const pointerMove = e => {
this.#highlightMove(parent, e);
};
const signal = parent._signal;
const pointerDownOptions = { capture: true, passive: false, signal };
const ac = new AbortController();
const signal = parent.combinedSignal(ac);
const pointerDown = e => {
// Avoid to have undesired clicks during the drawing.
e.preventDefault();
e.stopPropagation();
};
const pointerUpCallback = e => {
textLayer.removeEventListener("pointermove", pointerMove);
window.removeEventListener("blur", pointerUpCallback);
window.removeEventListener("pointerup", pointerUpCallback);
window.removeEventListener(
"pointerdown",
pointerDown,
pointerDownOptions
);
window.removeEventListener("contextmenu", noContextMenu);
ac.abort();
this.#endHighlight(parent, e);
};
window.addEventListener("blur", pointerUpCallback, { signal });
window.addEventListener("pointerup", pointerUpCallback, { signal });
window.addEventListener("pointerdown", pointerDown, pointerDownOptions);
window.addEventListener("pointerdown", pointerDown, {
capture: true,
passive: false,
signal,
});
window.addEventListener("contextmenu", noContextMenu, { signal });
textLayer.addEventListener("pointermove", pointerMove, { signal });
textLayer.addEventListener(
"pointermove",
this.#highlightMove.bind(this, parent),
{ signal }
);
this._freeHighlight = new FreeOutliner(
{ x, y },
[layerX, layerY, parentWidth, parentHeight],

View file

@ -16,6 +16,7 @@
import {
AnnotationEditorParamsType,
AnnotationEditorType,
assert,
Util,
} from "../../shared/util.js";
import { AnnotationEditor } from "./editor.js";
@ -31,26 +32,22 @@ class InkEditor extends AnnotationEditor {
#baseWidth = 0;
#boundCanvasPointermove = this.canvasPointermove.bind(this);
#boundCanvasPointerleave = this.canvasPointerleave.bind(this);
#boundCanvasPointerup = this.canvasPointerup.bind(this);
#boundCanvasPointerdown = this.canvasPointerdown.bind(this);
#canvasContextMenuTimeoutId = null;
#currentPath2D = new Path2D();
#disableEditing = false;
#drawingAC = null;
#hasSomethingToDraw = false;
#isCanvasInitialized = false;
#observer = null;
#pointerdownAC = null;
#realWidth = 0;
#realHeight = 0;
@ -296,9 +293,7 @@ class InkEditor extends AnnotationEditor {
super.enableEditMode();
this._isDraggable = false;
this.canvas.addEventListener("pointerdown", this.#boundCanvasPointerdown, {
signal: this._uiManager._signal,
});
this.#addPointerdownListener();
}
/** @inheritdoc */
@ -310,11 +305,7 @@ class InkEditor extends AnnotationEditor {
super.disableEditMode();
this._isDraggable = !this.isEmpty();
this.div.classList.remove("editing");
this.canvas.removeEventListener(
"pointerdown",
this.#boundCanvasPointerdown
);
this.#removePointerdownListener();
}
/** @inheritdoc */
@ -365,23 +356,33 @@ class InkEditor extends AnnotationEditor {
* @param {number} y
*/
#startDrawing(x, y) {
const signal = this._uiManager._signal;
this.canvas.addEventListener("contextmenu", noContextMenu, { signal });
this.canvas.addEventListener("contextmenu", noContextMenu, {
signal: this._uiManager._signal,
});
this.#removePointerdownListener();
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
assert(
!this.#drawingAC,
"No `this.#drawingAC` AbortController should exist."
);
}
this.#drawingAC = new AbortController();
const signal = this._uiManager.combinedSignal(this.#drawingAC);
this.canvas.addEventListener(
"pointerleave",
this.#boundCanvasPointerleave,
this.canvasPointerleave.bind(this),
{ signal }
);
this.canvas.addEventListener("pointermove", this.#boundCanvasPointermove, {
signal,
});
this.canvas.addEventListener("pointerup", this.#boundCanvasPointerup, {
signal,
});
this.canvas.removeEventListener(
"pointerdown",
this.#boundCanvasPointerdown
this.canvas.addEventListener(
"pointermove",
this.canvasPointermove.bind(this),
{ signal }
);
this.canvas.addEventListener("pointerup", this.canvasPointerup.bind(this), {
signal,
});
this.isEditing = true;
if (!this.#isCanvasInitialized) {
@ -653,6 +654,25 @@ class InkEditor extends AnnotationEditor {
this.enableEditMode();
}
#addPointerdownListener() {
if (this.#pointerdownAC) {
return;
}
this.#pointerdownAC = new AbortController();
const signal = this._uiManager.combinedSignal(this.#pointerdownAC);
this.canvas.addEventListener(
"pointerdown",
this.canvasPointerdown.bind(this),
{ signal }
);
}
#removePointerdownListener() {
this.pointerdownAC?.abort();
this.pointerdownAC = null;
}
/**
* onpointerdown callback for the canvas we're drawing on.
* @param {PointerEvent} event
@ -708,19 +728,10 @@ class InkEditor extends AnnotationEditor {
* @param {PointerEvent} event
*/
#endDrawing(event) {
this.canvas.removeEventListener(
"pointerleave",
this.#boundCanvasPointerleave
);
this.canvas.removeEventListener(
"pointermove",
this.#boundCanvasPointermove
);
this.canvas.removeEventListener("pointerup", this.#boundCanvasPointerup);
this.canvas.addEventListener("pointerdown", this.#boundCanvasPointerdown, {
signal: this._uiManager._signal,
});
this.#drawingAC?.abort();
this.#drawingAC = null;
this.#addPointerdownListener();
// Slight delay to avoid the context menu to appear (it can happen on a long
// tap with a pen).
if (this.#canvasContextMenuTimeoutId) {

View file

@ -550,6 +550,8 @@ class AnnotationEditorUIManager {
#commandManager = new CommandManager();
#copyPasteAC = null;
#currentPageIndex = 0;
#deletedAnnotationsElementIds = new Set();
@ -570,6 +572,8 @@ class AnnotationEditorUIManager {
#focusMainContainerTimeoutId = null;
#focusManagerAC = null;
#highlightColors = null;
#highlightWhenShiftUp = false;
@ -582,6 +586,8 @@ class AnnotationEditorUIManager {
#isWaiting = false;
#keyboardManagerAC = null;
#lastActiveElement = null;
#mainHighlightColorPicker = null;
@ -598,20 +604,6 @@ class AnnotationEditorUIManager {
#showAllStates = null;
#boundBlur = this.blur.bind(this);
#boundFocus = this.focus.bind(this);
#boundCopy = this.copy.bind(this);
#boundCut = this.cut.bind(this);
#boundPaste = this.paste.bind(this);
#boundKeydown = this.keydown.bind(this);
#boundKeyup = this.keyup.bind(this);
#previousStates = {
isEditing: false,
isEmpty: true,
@ -855,6 +847,10 @@ class AnnotationEditorUIManager {
}
}
combinedSignal(ac) {
return AbortSignal.any([this._signal, ac.signal]);
}
get mlManager() {
return this.#mlManager;
}
@ -1142,15 +1138,16 @@ class AnnotationEditorUIManager {
: null;
activeLayer?.toggleDrawing();
const signal = this._signal;
const ac = new AbortController();
const signal = this.combinedSignal(ac);
const pointerup = e => {
if (e.type === "pointerup" && e.button !== 0) {
// Do nothing on right click.
return;
}
ac.abort();
activeLayer?.toggleDrawing(true);
window.removeEventListener("pointerup", pointerup);
window.removeEventListener("blur", pointerup);
if (e.type === "pointerup") {
this.#onSelectEnd("main_toolbar");
}
@ -1172,21 +1169,24 @@ class AnnotationEditorUIManager {
document.addEventListener(
"selectionchange",
this.#selectionChange.bind(this),
{
signal: this._signal,
}
{ signal: this._signal }
);
}
#addFocusManager() {
const signal = this._signal;
window.addEventListener("focus", this.#boundFocus, { signal });
window.addEventListener("blur", this.#boundBlur, { signal });
if (this.#focusManagerAC) {
return;
}
this.#focusManagerAC = new AbortController();
const signal = this.combinedSignal(this.#focusManagerAC);
window.addEventListener("focus", this.focus.bind(this), { signal });
window.addEventListener("blur", this.blur.bind(this), { signal });
}
#removeFocusManager() {
window.removeEventListener("focus", this.#boundFocus);
window.removeEventListener("blur", this.#boundBlur);
this.#focusManagerAC?.abort();
this.#focusManagerAC = null;
}
blur() {
@ -1229,29 +1229,38 @@ class AnnotationEditorUIManager {
}
#addKeyboardManager() {
const signal = this._signal;
if (this.#keyboardManagerAC) {
return;
}
this.#keyboardManagerAC = new AbortController();
const signal = this.combinedSignal(this.#keyboardManagerAC);
// The keyboard events are caught at the container level in order to be able
// to execute some callbacks even if the current page doesn't have focus.
window.addEventListener("keydown", this.#boundKeydown, { signal });
window.addEventListener("keyup", this.#boundKeyup, { signal });
window.addEventListener("keydown", this.keydown.bind(this), { signal });
window.addEventListener("keyup", this.keyup.bind(this), { signal });
}
#removeKeyboardManager() {
window.removeEventListener("keydown", this.#boundKeydown);
window.removeEventListener("keyup", this.#boundKeyup);
this.#keyboardManagerAC?.abort();
this.#keyboardManagerAC = null;
}
#addCopyPasteListeners() {
const signal = this._signal;
document.addEventListener("copy", this.#boundCopy, { signal });
document.addEventListener("cut", this.#boundCut, { signal });
document.addEventListener("paste", this.#boundPaste, { signal });
if (this.#copyPasteAC) {
return;
}
this.#copyPasteAC = new AbortController();
const signal = this.combinedSignal(this.#copyPasteAC);
document.addEventListener("copy", this.copy.bind(this), { signal });
document.addEventListener("cut", this.cut.bind(this), { signal });
document.addEventListener("paste", this.paste.bind(this), { signal });
}
#removeCopyPasteListeners() {
document.removeEventListener("copy", this.#boundCopy);
document.removeEventListener("cut", this.#boundCut);
document.removeEventListener("paste", this.#boundPaste);
this.#copyPasteAC?.abort();
this.#copyPasteAC = null;
}
#addDragAndDropListeners() {