Skip to content

Commit b7198d3

Browse files
Merge pull request #18586 from Snuffleupagus/editor-AbortSignal-any
[Editor] Remove event listeners with `AbortSignal.any()`
2 parents 36dc666 + c0bf3d3 commit b7198d3

File tree

9 files changed

+201
-192
lines changed

9 files changed

+201
-192
lines changed

src/display/editor/annotation_editor_layer.js

Lines changed: 25 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,7 @@ class AnnotationEditorLayer {
6161

6262
#annotationLayer = null;
6363

64-
#boundPointerup = null;
65-
66-
#boundPointerdown = null;
67-
68-
#boundTextLayerPointerDown = null;
64+
#clickAC = null;
6965

7066
#editorFocusTimeoutId = null;
7167

@@ -79,6 +75,8 @@ class AnnotationEditorLayer {
7975

8076
#textLayer = null;
8177

78+
#textSelectionAC = null;
79+
8280
#uiManager;
8381

8482
static _initialized = false;
@@ -365,25 +363,25 @@ class AnnotationEditorLayer {
365363

366364
enableTextSelection() {
367365
this.div.tabIndex = -1;
368-
if (this.#textLayer?.div && !this.#boundTextLayerPointerDown) {
369-
this.#boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this);
366+
if (this.#textLayer?.div && !this.#textSelectionAC) {
367+
this.#textSelectionAC = new AbortController();
368+
const signal = this.#uiManager.combinedSignal(this.#textSelectionAC);
369+
370370
this.#textLayer.div.addEventListener(
371371
"pointerdown",
372-
this.#boundTextLayerPointerDown,
373-
{ signal: this.#uiManager._signal }
372+
this.#textLayerPointerDown.bind(this),
373+
{ signal }
374374
);
375375
this.#textLayer.div.classList.add("highlighting");
376376
}
377377
}
378378

379379
disableTextSelection() {
380380
this.div.tabIndex = 0;
381-
if (this.#textLayer?.div && this.#boundTextLayerPointerDown) {
382-
this.#textLayer.div.removeEventListener(
383-
"pointerdown",
384-
this.#boundTextLayerPointerDown
385-
);
386-
this.#boundTextLayerPointerDown = null;
381+
if (this.#textLayer?.div && this.#textSelectionAC) {
382+
this.#textSelectionAC.abort();
383+
this.#textSelectionAC = null;
384+
387385
this.#textLayer.div.classList.remove("highlighting");
388386
}
389387
}
@@ -428,26 +426,23 @@ class AnnotationEditorLayer {
428426
}
429427

430428
enableClick() {
431-
if (this.#boundPointerdown) {
429+
if (this.#clickAC) {
432430
return;
433431
}
434-
const signal = this.#uiManager._signal;
435-
this.#boundPointerdown = this.pointerdown.bind(this);
436-
this.#boundPointerup = this.pointerup.bind(this);
437-
this.div.addEventListener("pointerdown", this.#boundPointerdown, {
432+
this.#clickAC = new AbortController();
433+
const signal = this.#uiManager.combinedSignal(this.#clickAC);
434+
435+
this.div.addEventListener("pointerdown", this.pointerdown.bind(this), {
436+
signal,
437+
});
438+
this.div.addEventListener("pointerup", this.pointerup.bind(this), {
438439
signal,
439440
});
440-
this.div.addEventListener("pointerup", this.#boundPointerup, { signal });
441441
}
442442

443443
disableClick() {
444-
if (!this.#boundPointerdown) {
445-
return;
446-
}
447-
this.div.removeEventListener("pointerdown", this.#boundPointerdown);
448-
this.div.removeEventListener("pointerup", this.#boundPointerup);
449-
this.#boundPointerdown = null;
450-
this.#boundPointerup = null;
444+
this.#clickAC?.abort();
445+
this.#clickAC = null;
451446
}
452447

453448
attach(editor) {
@@ -611,8 +606,8 @@ class AnnotationEditorLayer {
611606
return AnnotationEditorLayer.#editorTypes.get(this.#uiManager.getMode());
612607
}
613608

614-
get _signal() {
615-
return this.#uiManager._signal;
609+
combinedSignal(ac) {
610+
return this.#uiManager.combinedSignal(ac);
616611
}
617612

618613
/**

src/display/editor/editor.js

Lines changed: 33 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -54,9 +54,7 @@ class AnnotationEditor {
5454

5555
#savedDimensions = null;
5656

57-
#boundFocusin = this.focusin.bind(this);
58-
59-
#boundFocusout = this.focusout.bind(this);
57+
#focusAC = null;
6058

6159
#focusedResizerName = "";
6260

@@ -758,16 +756,17 @@ class AnnotationEditor {
758756

759757
this.#altText?.toggle(false);
760758

761-
const boundResizerPointermove = this.#resizerPointermove.bind(this, name);
762759
const savedDraggable = this._isDraggable;
763760
this._isDraggable = false;
764-
const signal = this._uiManager._signal;
765-
const pointerMoveOptions = { passive: true, capture: true, signal };
761+
762+
const ac = new AbortController();
763+
const signal = this._uiManager.combinedSignal(ac);
764+
766765
this.parent.togglePointerEvents(false);
767766
window.addEventListener(
768767
"pointermove",
769-
boundResizerPointermove,
770-
pointerMoveOptions
768+
this.#resizerPointermove.bind(this, name),
769+
{ passive: true, capture: true, signal }
771770
);
772771
window.addEventListener("contextmenu", noContextMenu, { signal });
773772
const savedX = this.x;
@@ -780,17 +779,10 @@ class AnnotationEditor {
780779
window.getComputedStyle(event.target).cursor;
781780

782781
const pointerUpCallback = () => {
782+
ac.abort();
783783
this.parent.togglePointerEvents(true);
784784
this.#altText?.toggle(true);
785785
this._isDraggable = savedDraggable;
786-
window.removeEventListener("pointerup", pointerUpCallback);
787-
window.removeEventListener("blur", pointerUpCallback);
788-
window.removeEventListener(
789-
"pointermove",
790-
boundResizerPointermove,
791-
pointerMoveOptions
792-
);
793-
window.removeEventListener("contextmenu", noContextMenu);
794786
this.parent.div.style.cursor = savedParentCursor;
795787
this.div.style.cursor = savedCursor;
796788

@@ -1069,10 +1061,7 @@ class AnnotationEditor {
10691061
}
10701062

10711063
this.setInForeground();
1072-
1073-
const signal = this._uiManager._signal;
1074-
this.div.addEventListener("focusin", this.#boundFocusin, { signal });
1075-
this.div.addEventListener("focusout", this.#boundFocusout, { signal });
1064+
this.#addFocusListeners();
10761065

10771066
const [parentWidth, parentHeight] = this.parentDimensions;
10781067
if (this.parentRotation % 180 !== 0) {
@@ -1132,14 +1121,14 @@ class AnnotationEditor {
11321121
const isSelected = this._uiManager.isSelected(this);
11331122
this._uiManager.setUpDragSession();
11341123

1135-
let pointerMoveOptions, pointerMoveCallback;
1136-
const signal = this._uiManager._signal;
1124+
const ac = new AbortController();
1125+
const signal = this._uiManager.combinedSignal(ac);
1126+
11371127
if (isSelected) {
11381128
this.div.classList.add("moving");
1139-
pointerMoveOptions = { passive: true, capture: true, signal };
11401129
this.#prevDragX = event.clientX;
11411130
this.#prevDragY = event.clientY;
1142-
pointerMoveCallback = e => {
1131+
const pointerMoveCallback = e => {
11431132
const { clientX: x, clientY: y } = e;
11441133
const [tx, ty] = this.screenToPageTranslation(
11451134
x - this.#prevDragX,
@@ -1149,23 +1138,17 @@ class AnnotationEditor {
11491138
this.#prevDragY = y;
11501139
this._uiManager.dragSelectedEditors(tx, ty);
11511140
};
1152-
window.addEventListener(
1153-
"pointermove",
1154-
pointerMoveCallback,
1155-
pointerMoveOptions
1156-
);
1141+
window.addEventListener("pointermove", pointerMoveCallback, {
1142+
passive: true,
1143+
capture: true,
1144+
signal,
1145+
});
11571146
}
11581147

11591148
const pointerUpCallback = () => {
1160-
window.removeEventListener("pointerup", pointerUpCallback);
1161-
window.removeEventListener("blur", pointerUpCallback);
1149+
ac.abort();
11621150
if (isSelected) {
11631151
this.div.classList.remove("moving");
1164-
window.removeEventListener(
1165-
"pointermove",
1166-
pointerMoveCallback,
1167-
pointerMoveOptions
1168-
);
11691152
}
11701153

11711154
this.#hasBeenClicked = false;
@@ -1323,15 +1306,24 @@ class AnnotationEditor {
13231306
return this.div && !this.isAttachedToDOM;
13241307
}
13251308

1309+
#addFocusListeners() {
1310+
if (this.#focusAC || !this.div) {
1311+
return;
1312+
}
1313+
this.#focusAC = new AbortController();
1314+
const signal = this._uiManager.combinedSignal(this.#focusAC);
1315+
1316+
this.div.addEventListener("focusin", this.focusin.bind(this), { signal });
1317+
this.div.addEventListener("focusout", this.focusout.bind(this), { signal });
1318+
}
1319+
13261320
/**
13271321
* Rebuild the editor in case it has been removed on undo.
13281322
*
13291323
* To implement in subclasses.
13301324
*/
13311325
rebuild() {
1332-
const signal = this._uiManager._signal;
1333-
this.div?.addEventListener("focusin", this.#boundFocusin, { signal });
1334-
this.div?.addEventListener("focusout", this.#boundFocusout, { signal });
1326+
this.#addFocusListeners();
13351327
}
13361328

13371329
/**
@@ -1401,8 +1393,8 @@ class AnnotationEditor {
14011393
* It's used on ctrl+backspace action.
14021394
*/
14031395
remove() {
1404-
this.div.removeEventListener("focusin", this.#boundFocusin);
1405-
this.div.removeEventListener("focusout", this.#boundFocusout);
1396+
this.#focusAC?.abort();
1397+
this.#focusAC = null;
14061398

14071399
if (!this.isEmpty()) {
14081400
// The editor is removed but it can be back at some point thanks to

src/display/editor/freetext.js

Lines changed: 24 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,14 @@ const EOL_PATTERN = /\r\n?|\n/g;
3838
* Basic text editor in order to create a FreeTex annotation.
3939
*/
4040
class FreeTextEditor extends AnnotationEditor {
41-
#boundEditorDivBlur = this.editorDivBlur.bind(this);
42-
43-
#boundEditorDivFocus = this.editorDivFocus.bind(this);
44-
45-
#boundEditorDivInput = this.editorDivInput.bind(this);
46-
47-
#boundEditorDivKeydown = this.editorDivKeydown.bind(this);
48-
49-
#boundEditorDivPaste = this.editorDivPaste.bind(this);
50-
5141
#color;
5242

5343
#content = "";
5444

5545
#editorDivId = `${this.id}-editor`;
5646

47+
#editModeAC = null;
48+
5749
#fontSize;
5850

5951
#initialData = null;
@@ -307,20 +299,31 @@ class FreeTextEditor extends AnnotationEditor {
307299
this.editorDiv.contentEditable = true;
308300
this._isDraggable = false;
309301
this.div.removeAttribute("aria-activedescendant");
310-
const signal = this._uiManager._signal;
311-
this.editorDiv.addEventListener("keydown", this.#boundEditorDivKeydown, {
312-
signal,
313-
});
314-
this.editorDiv.addEventListener("focus", this.#boundEditorDivFocus, {
302+
303+
if (typeof PDFJSDev === "undefined" || PDFJSDev.test("TESTING")) {
304+
assert(
305+
!this.#editModeAC,
306+
"No `this.#editModeAC` AbortController should exist."
307+
);
308+
}
309+
this.#editModeAC = new AbortController();
310+
const signal = this._uiManager.combinedSignal(this.#editModeAC);
311+
312+
this.editorDiv.addEventListener(
313+
"keydown",
314+
this.editorDivKeydown.bind(this),
315+
{ signal }
316+
);
317+
this.editorDiv.addEventListener("focus", this.editorDivFocus.bind(this), {
315318
signal,
316319
});
317-
this.editorDiv.addEventListener("blur", this.#boundEditorDivBlur, {
320+
this.editorDiv.addEventListener("blur", this.editorDivBlur.bind(this), {
318321
signal,
319322
});
320-
this.editorDiv.addEventListener("input", this.#boundEditorDivInput, {
323+
this.editorDiv.addEventListener("input", this.editorDivInput.bind(this), {
321324
signal,
322325
});
323-
this.editorDiv.addEventListener("paste", this.#boundEditorDivPaste, {
326+
this.editorDiv.addEventListener("paste", this.editorDivPaste.bind(this), {
324327
signal,
325328
});
326329
}
@@ -337,11 +340,9 @@ class FreeTextEditor extends AnnotationEditor {
337340
this.editorDiv.contentEditable = false;
338341
this.div.setAttribute("aria-activedescendant", this.#editorDivId);
339342
this._isDraggable = true;
340-
this.editorDiv.removeEventListener("keydown", this.#boundEditorDivKeydown);
341-
this.editorDiv.removeEventListener("focus", this.#boundEditorDivFocus);
342-
this.editorDiv.removeEventListener("blur", this.#boundEditorDivBlur);
343-
this.editorDiv.removeEventListener("input", this.#boundEditorDivInput);
344-
this.editorDiv.removeEventListener("paste", this.#boundEditorDivPaste);
343+
344+
this.#editModeAC?.abort();
345+
this.#editModeAC = null;
345346

346347
// On Chrome, the focus is given to <body> when contentEditable is set to
347348
// false, hence we focus the div.

src/display/editor/highlight.js

Lines changed: 15 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -700,34 +700,33 @@ class HighlightEditor extends AnnotationEditor {
700700
width: parentWidth,
701701
height: parentHeight,
702702
} = textLayer.getBoundingClientRect();
703-
const pointerMove = e => {
704-
this.#highlightMove(parent, e);
705-
};
706-
const signal = parent._signal;
707-
const pointerDownOptions = { capture: true, passive: false, signal };
703+
704+
const ac = new AbortController();
705+
const signal = parent.combinedSignal(ac);
706+
708707
const pointerDown = e => {
709708
// Avoid to have undesired clicks during the drawing.
710709
e.preventDefault();
711710
e.stopPropagation();
712711
};
713712
const pointerUpCallback = e => {
714-
textLayer.removeEventListener("pointermove", pointerMove);
715-
window.removeEventListener("blur", pointerUpCallback);
716-
window.removeEventListener("pointerup", pointerUpCallback);
717-
window.removeEventListener(
718-
"pointerdown",
719-
pointerDown,
720-
pointerDownOptions
721-
);
722-
window.removeEventListener("contextmenu", noContextMenu);
713+
ac.abort();
723714
this.#endHighlight(parent, e);
724715
};
725716
window.addEventListener("blur", pointerUpCallback, { signal });
726717
window.addEventListener("pointerup", pointerUpCallback, { signal });
727-
window.addEventListener("pointerdown", pointerDown, pointerDownOptions);
718+
window.addEventListener("pointerdown", pointerDown, {
719+
capture: true,
720+
passive: false,
721+
signal,
722+
});
728723
window.addEventListener("contextmenu", noContextMenu, { signal });
729724

730-
textLayer.addEventListener("pointermove", pointerMove, { signal });
725+
textLayer.addEventListener(
726+
"pointermove",
727+
this.#highlightMove.bind(this, parent),
728+
{ signal }
729+
);
731730
this._freeHighlight = new FreeOutliner(
732731
{ x, y },
733732
[layerX, layerY, parentWidth, parentHeight],

0 commit comments

Comments
 (0)