Skip to content

Commit

Permalink
[Editor] Don't use the editor parent which can be null.
Browse files Browse the repository at this point in the history 
An annotation editor layer can be destroyed when it's invisible, hence some
annotations can have a null parent but when printing/saving or when changing
font size, color, ... of all added annotations (when selected with ctrl+a) we
still need to have some parent properties especially the page dimensions, global
scale factor and global rotation angle.
This patch aims to remove all the references to the parent in the editor instances
except in some cases where an editor should obviously have one.
It fixes mozilla#15780.
  • Loading branch information
@calixteman
calixteman committed Dec 6, 2022
1 parent cdd39ec commit 43f4508
Show file tree
Hide file tree
Showing 10 changed files with 399 additions and 128 deletions.
40 changes: 6 additions & 34 deletions src/display/editor/annotation_editor_layer.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@
// eslint-disable-next-line max-len
/** @typedef {import("./tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */
// eslint-disable-next-line max-len
/** @typedef {import("../annotation_storage.js").AnnotationStorage} AnnotationStorage */
// eslint-disable-next-line max-len
/** @typedef {import("../../web/text_accessibility.js").TextAccessibilityManager} TextAccessibilityManager */
/** @typedef {import("../../web/interfaces").IL10n} IL10n */

Expand All @@ -33,7 +31,6 @@ import { InkEditor } from "./ink.js";
* @property {HTMLDivElement} div
* @property {AnnotationEditorUIManager} uiManager
* @property {boolean} enabled
* @property {AnnotationStorage} annotationStorage
* @property {TextAccessibilityManager} [accessibilityManager]
* @property {number} pageIndex
* @property {IL10n} l10n
Expand Down Expand Up @@ -73,7 +70,6 @@ class AnnotationEditorLayer {
options.uiManager.registerEditorTypes([FreeTextEditor, InkEditor]);

this.#uiManager = options.uiManager;
this.annotationStorage = options.annotationStorage;
this.pageIndex = options.pageIndex;
this.div = options.div;
this.#accessibilityManager = options.accessibilityManager;
Expand Down Expand Up @@ -213,7 +209,6 @@ class AnnotationEditorLayer {

this.#uiManager.removeEditor(editor);
this.detach(editor);
this.annotationStorage.remove(editor.id);
editor.div.style.display = "none";
setTimeout(() => {
// When the div is removed from DOM the focus can move on the
Expand Down Expand Up @@ -244,9 +239,8 @@ class AnnotationEditorLayer {
}

this.attach(editor);
editor.pageIndex = this.pageIndex;
editor.parent?.detach(editor);
editor.parent = this;
editor.setParent(this);
if (editor.div && editor.isAttachedToDOM) {
editor.div.remove();
this.div.append(editor.div);
Expand All @@ -270,7 +264,7 @@ class AnnotationEditorLayer {

this.moveEditorInDOM(editor);
editor.onceAdded();
this.addToAnnotationStorage(editor);
this.#uiManager.addToAnnotationStorage(editor);
}

moveEditorInDOM(editor) {
Expand All @@ -282,16 +276,6 @@ class AnnotationEditorLayer {
);
}

/**
* Add an editor in the annotation storage.
* @param {AnnotationEditor} editor
*/
addToAnnotationStorage(editor) {
if (!editor.isEmpty() && !this.annotationStorage.has(editor.id)) {
this.annotationStorage.setValue(editor.id, editor);
}
}

/**
* Add or rebuild depending if it has been removed or not.
* @param {AnnotationEditor} editor
Expand Down Expand Up @@ -365,9 +349,9 @@ class AnnotationEditorLayer {
deserialize(data) {
switch (data.annotationType) {
case AnnotationEditorType.FREETEXT:
return FreeTextEditor.deserialize(data, this);
return FreeTextEditor.deserialize(data, this, this.#uiManager);
case AnnotationEditorType.INK:
return InkEditor.deserialize(data, this);
return InkEditor.deserialize(data, this, this.#uiManager);
}
return null;
}
Expand All @@ -384,6 +368,7 @@ class AnnotationEditorLayer {
id,
x: event.offsetX,
y: event.offsetY,
uiManager: this.#uiManager,
});
if (editor) {
this.add(editor);
Expand Down Expand Up @@ -520,9 +505,9 @@ class AnnotationEditorLayer {

for (const editor of this.#editors.values()) {
this.#accessibilityManager?.removePointerInTextLayer(editor.contentDiv);
editor.setParent(null);
editor.isAttachedToDOM = false;
editor.div.remove();
editor.parent = null;
}
this.div = null;
this.#editors.clear();
Expand Down Expand Up @@ -571,14 +556,6 @@ class AnnotationEditorLayer {
this.updateMode();
}

/**
* Get the scale factor from the viewport.
* @returns {number}
*/
get scaleFactor() {
return this.viewport.scale;
}

/**
* Get page dimensions.
* @returns {Object} dimensions.
Expand All @@ -591,11 +568,6 @@ class AnnotationEditorLayer {
return [width, height];
}

get viewportBaseDimensions() {
const { width, height, rotation } = this.viewport;
return rotation % 180 === 0 ? [width, height] : [height, width];
}

/**
* Set the dimensions of the main div.
*/
Expand Down
78 changes: 63 additions & 15 deletions src/display/editor/editor.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@

// eslint-disable-next-line max-len
/** @typedef {import("./annotation_editor_layer.js").AnnotationEditorLayer} AnnotationEditorLayer */
// eslint-disable-next-line max-len
/** @typedef {import("./tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */

import { bindEvents, ColorManager } from "./tools.js";
import { FeatureTest, shadow, unreachable } from "../../shared/util.js";

/**
* @typedef {Object} AnnotationEditorParameters
* @property {AnnotationEditorUIManager} uiManager - the global manager
* @property {AnnotationEditorLayer} parent - the layer containing this editor
* @property {string} id - editor id
* @property {number} x - x-coordinate
Expand All @@ -41,6 +44,8 @@ class AnnotationEditor {

#isInEditMode = false;

_uiManager = null;

#zIndex = AnnotationEditor._zIndex++;

static _colorManager = new ColorManager();
Expand All @@ -61,11 +66,13 @@ class AnnotationEditor {
this.pageIndex = parameters.parent.pageIndex;
this.name = parameters.name;
this.div = null;
this._uiManager = parameters.uiManager;

const [width, height] = this.parent.viewportBaseDimensions;
this.rotation = this.parent.viewport.rotation;
this.pageDimensions = this.parent.pageDimensions;
const [width, height] = this.parentDimensions;
this.x = parameters.x / width;
this.y = parameters.y / height;
this.rotation = this.parent.viewport.rotation;

this.isAttachedToDOM = false;
}
Expand All @@ -78,6 +85,18 @@ class AnnotationEditor {
);
}

/**
* Add some commands into the CommandManager (undo/redo stuff).
* @param {Object} params
*/
addCommands(params) {
this._uiManager.addCommands(params);
}

get currentLayer() {
return this._uiManager.currentLayer;
}

/**
* This editor will be behind the others.
*/
Expand All @@ -92,6 +111,14 @@ class AnnotationEditor {
this.div.style.zIndex = this.#zIndex;
}

setParent(parent) {
if (parent !== null) {
this.pageIndex = parent.pageIndex;
this.pageDimensions = parent.pageDimensions;
}
this.parent = parent;
}

/**
* onfocus callback.
*/
Expand Down Expand Up @@ -123,7 +150,7 @@ class AnnotationEditor {

event.preventDefault();

if (!this.parent.isMultipleSelection) {
if (!this.parent?.isMultipleSelection) {
this.commitOrRemove();
}
}
Expand All @@ -140,7 +167,11 @@ class AnnotationEditor {
* Commit the data contained in this editor.
*/
commit() {
this.parent.addToAnnotationStorage(this);
this.addToAnnotationStorage();
}

addToAnnotationStorage() {
this._uiManager.addToAnnotationStorage(this);
}

/**
Expand All @@ -163,7 +194,7 @@ class AnnotationEditor {
* @param {number} ty - y-translation in screen coordinates.
*/
setAt(x, y, tx, ty) {
const [width, height] = this.parent.viewportBaseDimensions;
const [width, height] = this.parentDimensions;
[tx, ty] = this.screenToPageTranslation(tx, ty);

this.x = (x + tx) / width;
Expand All @@ -179,7 +210,7 @@ class AnnotationEditor {
* @param {number} y - y-translation in screen coordinates.
*/
translate(x, y) {
const [width, height] = this.parent.viewportBaseDimensions;
const [width, height] = this.parentDimensions;
[x, y] = this.screenToPageTranslation(x, y);

this.x += x / width;
Expand All @@ -195,8 +226,7 @@ class AnnotationEditor {
* @param {number} y
*/
screenToPageTranslation(x, y) {
const { rotation } = this.parent.viewport;
switch (rotation) {
switch (this.parentRotation) {
case 90:
return [y, -x];
case 180:
Expand All @@ -208,13 +238,29 @@ class AnnotationEditor {
}
}

get parentScale() {
return this._uiManager.viewParameters.realScale;
}

get parentRotation() {
return this._uiManager.viewParameters.rotation;
}

get parentDimensions() {
const { realScale, rotation } = this._uiManager.viewParameters;
const [pageWidth, pageHeight] = this.pageDimensions;
return rotation % 180 === 0
? [pageWidth * realScale, pageHeight * realScale]
: [pageHeight * realScale, pageWidth * realScale];
}

/**
* Set the dimensions of this editor.
* @param {number} width
* @param {number} height
*/
setDims(width, height) {
const [parentWidth, parentHeight] = this.parent.viewportBaseDimensions;
const [parentWidth, parentHeight] = this.parentDimensions;
this.div.style.width = `${(100 * width) / parentWidth}%`;
this.div.style.height = `${(100 * height) / parentHeight}%`;
}
Expand All @@ -228,7 +274,7 @@ class AnnotationEditor {
return;
}

const [parentWidth, parentHeight] = this.parent.viewportBaseDimensions;
const [parentWidth, parentHeight] = this.parentDimensions;
if (!widthPercent) {
style.width = `${(100 * parseFloat(width)) / parentWidth}%`;
}
Expand Down Expand Up @@ -295,10 +341,10 @@ class AnnotationEditor {
}

getRect(tx, ty) {
const [parentWidth, parentHeight] = this.parent.viewportBaseDimensions;
const [pageWidth, pageHeight] = this.parent.pageDimensions;
const shiftX = (pageWidth * tx) / parentWidth;
const shiftY = (pageHeight * ty) / parentHeight;
const scale = this.parentScale;
const [pageWidth, pageHeight] = this.pageDimensions;
const shiftX = tx / scale;
const shiftY = ty / scale;
const x = this.x * pageWidth;
const y = this.y * pageHeight;
const width = this.width * pageWidth;
Expand Down Expand Up @@ -436,12 +482,14 @@ class AnnotationEditor {
*
* @param {Object} data
* @param {AnnotationEditorLayer} parent
* @param {AnnotationEditorUIManager} uiManager
* @returns {AnnotationEditor}
*/
static deserialize(data, parent) {
static deserialize(data, parent, uiManager) {
const editor = new this.prototype.constructor({
parent,
id: parent.getNextId(),
uiManager,
});
editor.rotation = data.rotation;

Expand Down
Loading

0 comments on commit 43f4508

Please sign in to comment.