Skip to content

Commit

Permalink
Merge pull request #18284 from calixteman/editor_signal
Browse files Browse the repository at this point in the history 
[Editor] Remove the various listeners when destroying the editor manager
  • Loading branch information
@calixteman
calixteman committed Jun 19, 2024
2 parents 9dbe4c2 + 390afcb commit c53f71a
Show file tree
Hide file tree
Showing 10 changed files with 244 additions and 129 deletions.
70 changes: 47 additions & 23 deletions src/display/editor/alt_text.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,20 +49,27 @@ class AltText {
altText.textContent = msg;
altText.setAttribute("aria-label", msg);
altText.tabIndex = "0";
altText.addEventListener("contextmenu", noContextMenu);
altText.addEventListener("pointerdown", event => event.stopPropagation());
const signal = this.#editor._uiManager._signal;
altText.addEventListener("contextmenu", noContextMenu, { signal });
altText.addEventListener("pointerdown", event => event.stopPropagation(), {
signal,
});

const onClick = event => {
event.preventDefault();
this.#editor._uiManager.editAltText(this.#editor);
};
altText.addEventListener("click", onClick, { capture: true });
altText.addEventListener("keydown", event => {
if (event.target === altText && event.key === "Enter") {
this.#altTextWasFromKeyBoard = true;
onClick(event);
}
});
altText.addEventListener("click", onClick, { capture: true, signal });
altText.addEventListener(
"keydown",
event => {
if (event.target === altText && event.key === "Enter") {
this.#altTextWasFromKeyBoard = true;
onClick(event);
}
},
{ signal }
);
await this.#setState();

return altText;
Expand Down Expand Up @@ -142,22 +149,39 @@ class AltText {
button.setAttribute("aria-describedby", id);

const DELAY_TO_SHOW_TOOLTIP = 100;
button.addEventListener("mouseenter", () => {
this.#altTextTooltipTimeout = setTimeout(() => {
this.#altTextTooltipTimeout = null;
this.#altTextTooltip.classList.add("show");
this.#editor._reportTelemetry({
action: "alt_text_tooltip",
});
}, DELAY_TO_SHOW_TOOLTIP);
});
button.addEventListener("mouseleave", () => {
if (this.#altTextTooltipTimeout) {
const signal = this.#editor._uiManager._signal;
signal.addEventListener(
"abort",
() => {
clearTimeout(this.#altTextTooltipTimeout);
this.#altTextTooltipTimeout = null;
}
this.#altTextTooltip?.classList.remove("show");
});
},
{ once: true }
);
button.addEventListener(
"mouseenter",
() => {
this.#altTextTooltipTimeout = setTimeout(() => {
this.#altTextTooltipTimeout = null;
this.#altTextTooltip.classList.add("show");
this.#editor._reportTelemetry({
action: "alt_text_tooltip",
});
}, DELAY_TO_SHOW_TOOLTIP);
},
{ signal }
);
button.addEventListener(
"mouseleave",
() => {
if (this.#altTextTooltipTimeout) {
clearTimeout(this.#altTextTooltipTimeout);
this.#altTextTooltipTimeout = null;
}
this.#altTextTooltip?.classList.remove("show");
},
{ signal }
);
}
tooltip.innerText = this.#altTextDecorative
? await AltText._l10nPromise.get(
Expand Down
18 changes: 13 additions & 5 deletions src/display/editor/annotation_editor_layer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,8 @@ class AnnotationEditorLayer {
this.#boundTextLayerPointerDown = this.#textLayerPointerDown.bind(this);
this.#textLayer.div.addEventListener(
"pointerdown",
this.#boundTextLayerPointerDown
this.#boundTextLayerPointerDown,
{ signal: this.#uiManager._signal }
);
this.#textLayer.div.classList.add("highlighting");
}
Expand Down Expand Up @@ -409,7 +410,7 @@ class AnnotationEditorLayer {
() => {
this.#textLayer.div.classList.remove("free");
},
{ once: true }
{ once: true, signal: this.#uiManager._signal }
);
event.preventDefault();
}
Expand All @@ -419,10 +420,13 @@ class AnnotationEditorLayer {
if (this.#boundPointerdown) {
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.div.addEventListener("pointerup", this.#boundPointerup);
this.div.addEventListener("pointerdown", this.#boundPointerdown, {
signal,
});
this.div.addEventListener("pointerup", this.#boundPointerup, { signal });
}

disableClick() {
Expand Down Expand Up @@ -540,7 +544,7 @@ class AnnotationEditorLayer {
() => {
editor._focusEventsAllowed = true;
},
{ once: true }
{ once: true, signal: this.#uiManager._signal }
);
activeElement.focus();
} else {
Expand Down Expand Up @@ -596,6 +600,10 @@ class AnnotationEditorLayer {
return AnnotationEditorLayer.#editorTypes.get(this.#uiManager.getMode());
}

get _signal() {
return this.#uiManager._signal;
}

/**
* Create a new editor
* @param {Object} params
Expand Down
18 changes: 12 additions & 6 deletions src/display/editor/color_picker.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,9 @@ class ColorPicker {
button.tabIndex = "0";
button.setAttribute("data-l10n-id", "pdfjs-editor-colorpicker-button");
button.setAttribute("aria-haspopup", true);
button.addEventListener("click", this.#openDropdown.bind(this));
button.addEventListener("keydown", this.#boundKeyDown);
const signal = this.#uiManager._signal;
button.addEventListener("click", this.#openDropdown.bind(this), { signal });
button.addEventListener("keydown", this.#boundKeyDown, { signal });
const swatch = (this.#buttonSwatch = document.createElement("span"));
swatch.className = "swatch";
swatch.setAttribute("aria-hidden", true);
Expand All @@ -109,7 +110,8 @@ class ColorPicker {

#getDropdownRoot() {
const div = document.createElement("div");
div.addEventListener("contextmenu", noContextMenu);
const signal = this.#uiManager._signal;
div.addEventListener("contextmenu", noContextMenu, { signal });
div.className = "dropdown";
div.role = "listbox";
div.setAttribute("aria-multiselectable", false);
Expand All @@ -127,11 +129,13 @@ class ColorPicker {
swatch.className = "swatch";
swatch.style.backgroundColor = color;
button.setAttribute("aria-selected", color === this.#defaultColor);
button.addEventListener("click", this.#colorSelect.bind(this, color));
button.addEventListener("click", this.#colorSelect.bind(this, color), {
signal,
});
div.append(button);
}

div.addEventListener("keydown", this.#boundKeyDown);
div.addEventListener("keydown", this.#boundKeyDown, { signal });

return div;
}
Expand Down Expand Up @@ -211,7 +215,9 @@ class ColorPicker {
return;
}
this.#dropdownWasFromKeyboard = event.detail === 0;
window.addEventListener("pointerdown", this.#boundPointerDown);
window.addEventListener("pointerdown", this.#boundPointerDown, {
signal: this.#uiManager._signal,
});
if (this.#dropdown) {
this.#dropdown.classList.remove("hidden");
return;
Expand Down
41 changes: 25 additions & 16 deletions src/display/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -715,16 +715,18 @@ class AnnotationEditor {
"bottomLeft",
"middleLeft",
];
const signal = this._uiManager._signal;
for (const name of classes) {
const div = document.createElement("div");
this.#resizersDiv.append(div);
div.classList.add("resizer", name);
div.setAttribute("data-resizer-name", name);
div.addEventListener(
"pointerdown",
this.#resizerPointerdown.bind(this, name)
this.#resizerPointerdown.bind(this, name),
{ signal }
);
div.addEventListener("contextmenu", noContextMenu);
div.addEventListener("contextmenu", noContextMenu, { signal });
div.tabIndex = -1;
}
this.div.prepend(this.#resizersDiv);
Expand All @@ -742,14 +744,15 @@ class AnnotationEditor {
const boundResizerPointermove = this.#resizerPointermove.bind(this, name);
const savedDraggable = this._isDraggable;
this._isDraggable = false;
const pointerMoveOptions = { passive: true, capture: true };
const signal = this._uiManager._signal;
const pointerMoveOptions = { passive: true, capture: true, signal };
this.parent.togglePointerEvents(false);
window.addEventListener(
"pointermove",
boundResizerPointermove,
pointerMoveOptions
);
window.addEventListener("contextmenu", noContextMenu);
window.addEventListener("contextmenu", noContextMenu, { signal });
const savedX = this.x;
const savedY = this.y;
const savedWidth = this.width;
Expand All @@ -776,10 +779,10 @@ class AnnotationEditor {

this.#addResizeToUndoStack(savedX, savedY, savedWidth, savedHeight);
};
window.addEventListener("pointerup", pointerUpCallback);
window.addEventListener("pointerup", pointerUpCallback, { signal });
// If the user switches to another window (with alt+tab), then we end the
// resize session.
window.addEventListener("blur", pointerUpCallback);
window.addEventListener("blur", pointerUpCallback, { signal });
}

#addResizeToUndoStack(savedX, savedY, savedWidth, savedHeight) {
Expand Down Expand Up @@ -1027,8 +1030,9 @@ class AnnotationEditor {

this.setInForeground();

this.div.addEventListener("focusin", this.#boundFocusin);
this.div.addEventListener("focusout", this.#boundFocusout);
const signal = this._uiManager._signal;
this.div.addEventListener("focusin", this.#boundFocusin, { signal });
this.div.addEventListener("focusout", this.#boundFocusout, { signal });

const [parentWidth, parentHeight] = this.parentDimensions;
if (this.parentRotation % 180 !== 0) {
Expand Down Expand Up @@ -1089,9 +1093,10 @@ class AnnotationEditor {
this._uiManager.setUpDragSession();

let pointerMoveOptions, pointerMoveCallback;
const signal = this._uiManager._signal;
if (isSelected) {
this.div.classList.add("moving");
pointerMoveOptions = { passive: true, capture: true };
pointerMoveOptions = { passive: true, capture: true, signal };
this.#prevDragX = event.clientX;
this.#prevDragY = event.clientY;
pointerMoveCallback = e => {
Expand Down Expand Up @@ -1128,11 +1133,11 @@ class AnnotationEditor {
this.#selectOnPointerEvent(event);
}
};
window.addEventListener("pointerup", pointerUpCallback);
window.addEventListener("pointerup", pointerUpCallback, { signal });
// If the user is using alt+tab during the dragging session, the pointerup
// event could be not fired, but a blur event is fired so we can use it in
// order to interrupt the dragging session.
window.addEventListener("blur", pointerUpCallback);
window.addEventListener("blur", pointerUpCallback, { signal });
}

moveInDOM() {
Expand Down Expand Up @@ -1284,8 +1289,9 @@ class AnnotationEditor {
* To implement in subclasses.
*/
rebuild() {
this.div?.addEventListener("focusin", this.#boundFocusin);
this.div?.addEventListener("focusout", this.#boundFocusout);
const signal = this._uiManager._signal;
this.div?.addEventListener("focusin", this.#boundFocusin, { signal });
this.div?.addEventListener("focusout", this.#boundFocusout, { signal });
}

/**
Expand Down Expand Up @@ -1429,12 +1435,15 @@ class AnnotationEditor {
this.#allResizerDivs = Array.from(children);
const boundResizerKeydown = this.#resizerKeydown.bind(this);
const boundResizerBlur = this.#resizerBlur.bind(this);
const signal = this._uiManager._signal;
for (const div of this.#allResizerDivs) {
const name = div.getAttribute("data-resizer-name");
div.setAttribute("role", "spinbutton");
div.addEventListener("keydown", boundResizerKeydown);
div.addEventListener("blur", boundResizerBlur);
div.addEventListener("focus", this.#resizerFocus.bind(this, name));
div.addEventListener("keydown", boundResizerKeydown, { signal });
div.addEventListener("blur", boundResizerBlur, { signal });
div.addEventListener("focus", this.#resizerFocus.bind(this, name), {
signal,
});
AnnotationEditor._l10nPromise
.get(`pdfjs-editor-resizer-label-${name}`)
.then(msg => div.setAttribute("aria-label", msg));
Expand Down
21 changes: 16 additions & 5 deletions src/display/editor/freetext.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -307,11 +307,22 @@ class FreeTextEditor extends AnnotationEditor {
this.editorDiv.contentEditable = true;
this._isDraggable = false;
this.div.removeAttribute("aria-activedescendant");
this.editorDiv.addEventListener("keydown", this.#boundEditorDivKeydown);
this.editorDiv.addEventListener("focus", this.#boundEditorDivFocus);
this.editorDiv.addEventListener("blur", this.#boundEditorDivBlur);
this.editorDiv.addEventListener("input", this.#boundEditorDivInput);
this.editorDiv.addEventListener("paste", this.#boundEditorDivPaste);
const signal = this._uiManager._signal;
this.editorDiv.addEventListener("keydown", this.#boundEditorDivKeydown, {
signal,
});
this.editorDiv.addEventListener("focus", this.#boundEditorDivFocus, {
signal,
});
this.editorDiv.addEventListener("blur", this.#boundEditorDivBlur, {
signal,
});
this.editorDiv.addEventListener("input", this.#boundEditorDivInput, {
signal,
});
this.editorDiv.addEventListener("paste", this.#boundEditorDivPaste, {
signal,
});
}

/** @inheritdoc */
Expand Down
15 changes: 9 additions & 6 deletions src/display/editor/highlight.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,9 @@ class HighlightEditor extends AnnotationEditor {
if (this.#isFreeHighlight) {
div.classList.add("free");
} else {
this.div.addEventListener("keydown", this.#boundKeydown);
this.div.addEventListener("keydown", this.#boundKeydown, {
signal: this._uiManager._signal,
});
}
const highlightDiv = (this.#highlightDiv = document.createElement("div"));
div.append(highlightDiv);
Expand Down Expand Up @@ -702,7 +704,8 @@ class HighlightEditor extends AnnotationEditor {
const pointerMove = e => {
this.#highlightMove(parent, e);
};
const pointerDownOptions = { capture: true, passive: false };
const signal = parent._signal;
const pointerDownOptions = { capture: true, passive: false, signal };
const pointerDown = e => {
// Avoid to have undesired clicks during the drawing.
e.preventDefault();
Expand All @@ -720,12 +723,12 @@ class HighlightEditor extends AnnotationEditor {
window.removeEventListener("contextmenu", noContextMenu);
this.#endHighlight(parent, e);
};
window.addEventListener("blur", pointerUpCallback);
window.addEventListener("pointerup", pointerUpCallback);
window.addEventListener("blur", pointerUpCallback, { signal });
window.addEventListener("pointerup", pointerUpCallback, { signal });
window.addEventListener("pointerdown", pointerDown, pointerDownOptions);
window.addEventListener("contextmenu", noContextMenu);
window.addEventListener("contextmenu", noContextMenu, { signal });

textLayer.addEventListener("pointermove", pointerMove);
textLayer.addEventListener("pointermove", pointerMove, { signal });
this._freeHighlight = new FreeOutliner(
{ x, y },
[layerX, layerY, parentWidth, parentHeight],
Expand Down
Loading

0 comments on commit c53f71a

Please sign in to comment.