Skip to content

Commit

Permalink
Merge pull request #18586 from Snuffleupagus/editor-AbortSignal-any
Browse files Browse the repository at this point in the history 
[Editor] Remove event listeners with `AbortSignal.any()`
  • Loading branch information
@Snuffleupagus
Snuffleupagus authored Aug 9, 2024
2 parents 36dc666 + c0bf3d3 commit b7198d3
Show file tree
Hide file tree
Showing 9 changed files with 201 additions and 192 deletions.
55 changes: 25 additions & 30 deletions src/display/editor/annotation_editor_layer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,7 @@ class AnnotationEditorLayer {

#annotationLayer = null;

#boundPointerup = null;

#boundPointerdown = null;

#boundTextLayerPointerDown = null;
#clickAC = null;

#editorFocusTimeoutId = null;

Expand All @@ -79,6 +75,8 @@ class AnnotationEditorLayer {

#textLayer = null;

#textSelectionAC = null;

#uiManager;

static _initialized = false;
Expand Down Expand Up @@ -365,25 +363,25 @@ 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");
}
}

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");
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}

/**
Expand Down
74 changes: 33 additions & 41 deletions src/display/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,7 @@ class AnnotationEditor {

#savedDimensions = null;

#boundFocusin = this.focusin.bind(this);

#boundFocusout = this.focusout.bind(this);
#focusAC = null;

#focusedResizerName = "";

Expand Down Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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();
}

/**
Expand Down Expand Up @@ -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
Expand Down
47 changes: 24 additions & 23 deletions src/display/editor/freetext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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, {
signal,
});
this.editorDiv.addEventListener("focus", this.#boundEditorDivFocus, {

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("blur", this.#boundEditorDivBlur, {
this.editorDiv.addEventListener("blur", this.editorDivBlur.bind(this), {
signal,
});
this.editorDiv.addEventListener("input", this.#boundEditorDivInput, {
this.editorDiv.addEventListener("input", this.editorDivInput.bind(this), {
signal,
});
this.editorDiv.addEventListener("paste", this.#boundEditorDivPaste, {
this.editorDiv.addEventListener("paste", this.editorDivPaste.bind(this), {
signal,
});
}
Expand All @@ -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.
Expand Down
31 changes: 15 additions & 16 deletions src/display/editor/highlight.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down
Loading

0 comments on commit b7198d3

Please sign in to comment.