diff --git a/src/core/annotation.js b/src/core/annotation.js index c47b79ffb..0f15834a0 100644 --- a/src/core/annotation.js +++ b/src/core/annotation.js @@ -3553,7 +3553,7 @@ class FreeTextAnnotation extends MarkupAnnotation { constructor(params) { super(params); - this.data.hasOwnCanvas = this.data.noRotate; + this.data.hasOwnCanvas = true; const { xref } = params; this.data.annotationType = AnnotationType.FREETEXT; diff --git a/src/display/annotation_layer.js b/src/display/annotation_layer.js index 46c89e401..167310c48 100644 --- a/src/display/annotation_layer.js +++ b/src/display/annotation_layer.js @@ -2629,7 +2629,7 @@ class AnnotationLayer { #div = null; - #editableAnnotations = new Set(); + #editableAnnotations = new Map(); constructor({ div, accessibilityManager, annotationCanvasMap }) { this.#div = div; @@ -2696,7 +2696,7 @@ class AnnotationLayer { } if (element.annotationEditorType > 0) { - this.#editableAnnotations.add(element); + this.#editableAnnotations.set(element.data.id, element); } const rendered = element.render(); @@ -2767,7 +2767,11 @@ class AnnotationLayer { } getEditableAnnotations() { - return this.#editableAnnotations; + return Array.from(this.#editableAnnotations.values()); + } + + getEditableAnnotation(id) { + return this.#editableAnnotations.get(id); } } diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index a0be96cb0..b12012b1a 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -13,7 +13,6 @@ * limitations under the License. */ -/** @typedef {import("./editor.js").AnnotationEditor} AnnotationEditor */ // eslint-disable-next-line max-len /** @typedef {import("./tools.js").AnnotationEditorUIManager} AnnotationEditorUIManager */ /** @typedef {import("../display_utils.js").PageViewport} PageViewport */ @@ -24,6 +23,7 @@ /** @typedef {import("../src/display/annotation_layer.js").AnnotationLayer} AnnotationLayer */ import { AnnotationEditorType, FeatureTest } from "../../shared/util.js"; +import { AnnotationEditor } from "./editor.js"; import { bindEvents } from "./tools.js"; import { FreeTextEditor } from "./freetext.js"; import { InkEditor } from "./ink.js"; @@ -66,6 +66,8 @@ class AnnotationEditorLayer { #isCleaningUp = false; + #isDisabling = false; + #uiManager; static _initialized = false; @@ -86,6 +88,7 @@ class AnnotationEditorLayer { this.div = options.div; this.#accessibilityManager = options.accessibilityManager; this.#annotationLayer = options.annotationLayer; + this.viewport = options.viewport; this.#uiManager.addLayer(this); } @@ -175,18 +178,33 @@ class AnnotationEditorLayer { */ enable() { this.div.style.pointerEvents = "auto"; - if (this.#annotationLayer) { - const editables = this.#annotationLayer.getEditableAnnotations(); - for (const editable of editables) { - const editor = this.deserialize(editable); - if (!editor) { - continue; - } - editable.hide(); - this.addOrRebuild(editor); + const annotationElementIds = new Set(); + for (const editor of this.#editors.values()) { + editor.enableEditing(); + if (editor.annotationElementId) { + annotationElementIds.add(editor.annotationElementId); } } - for (const editor of this.#editors.values()) { + + if (!this.#annotationLayer) { + return; + } + + const editables = this.#annotationLayer.getEditableAnnotations(); + for (const editable of editables) { + // The element must be hidden whatever its state is. + editable.hide(); + if (this.#uiManager.isDeletedAnnotationElement(editable.data.id)) { + continue; + } + if (annotationElementIds.has(editable.data.id)) { + continue; + } + const editor = this.deserialize(editable); + if (!editor) { + continue; + } + this.addOrRebuild(editor); editor.enableEditing(); } } @@ -195,18 +213,25 @@ class AnnotationEditorLayer { * Disable editor creation. */ disable() { + this.#isDisabling = true; this.div.style.pointerEvents = "none"; for (const editor of this.#editors.values()) { editor.disableEditing(); - if (!editor.hasElementChanged()) { - editor.annotationElement.show(); - editor.remove(); + if (!editor.annotationElementId || editor.serialize() !== null) { + continue; } + this.getEditableAnnotation(editor.annotationElementId)?.show(); + editor.remove(); } this.#cleanup(); if (this.isEmpty) { this.div.hidden = true; } + this.#isDisabling = false; + } + + getEditableAnnotation(id) { + return this.#annotationLayer?.getEditableAnnotation(id) || null; } /** @@ -234,11 +259,22 @@ class AnnotationEditorLayer { attach(editor) { this.#editors.set(editor.id, editor); + const { annotationElementId } = editor; + if ( + annotationElementId && + this.#uiManager.isDeletedAnnotationElement(annotationElementId) + ) { + this.#uiManager.removeDeletedAnnotationElement(editor); + } } detach(editor) { this.#editors.delete(editor.id); this.#accessibilityManager?.removePointerInTextLayer(editor.contentDiv); + + if (!this.#isDisabling && editor.annotationElementId) { + this.#uiManager.addDeletedAnnotationElement(editor); + } } /** @@ -249,8 +285,8 @@ class AnnotationEditorLayer { // Since we can undo a removal we need to keep the // parent property as it is, so don't null it! - this.#uiManager.removeEditor(editor); this.detach(editor); + this.#uiManager.removeEditor(editor); editor.div.style.display = "none"; setTimeout(() => { // When the div is removed from DOM the focus can move on the @@ -280,6 +316,12 @@ class AnnotationEditorLayer { return; } + if (editor.annotationElementId) { + this.#uiManager.addDeletedAnnotationElement(editor.annotationElementId); + AnnotationEditor.deleteAnnotationElement(editor); + editor.annotationElementId = null; + } + this.attach(editor); editor.parent?.detach(editor); editor.setParent(this); diff --git a/src/display/editor/editor.js b/src/display/editor/editor.js index 224b5c56a..3052d6466 100644 --- a/src/display/editor/editor.js +++ b/src/display/editor/editor.js @@ -67,7 +67,7 @@ class AnnotationEditor { this.name = parameters.name; this.div = null; this._uiManager = parameters.uiManager; - this.annotationElement = null; + this.annotationElementId = null; const { rotation, @@ -85,6 +85,7 @@ class AnnotationEditor { this.y = parameters.y / height; this.isAttachedToDOM = false; + this.deleted = false; } static get _defaultLineColor() { @@ -95,6 +96,17 @@ class AnnotationEditor { ); } + static deleteAnnotationElement(editor) { + const fakeEditor = new FakeEditor({ + id: editor.parent.getNextId(), + parent: editor.parent, + uiManager: editor._uiManager, + }); + fakeEditor.annotationElementId = editor.annotationElementId; + fakeEditor.deleted = true; + fakeEditor._uiManager.addToAnnotationStorage(fakeEditor); + } + /** * Add some commands into the CommandManager (undo/redo stuff). * @param {Object} params @@ -601,14 +613,22 @@ class AnnotationEditor { this.parent.setActiveEditor(null); } } +} - /** - * Check if the editor has been changed. - * @param {Object} serialized - * @returns {boolean} - */ - hasElementChanged(serialized = null) { - return false; +// This class is used to fake an editor which has been deleted. +class FakeEditor extends AnnotationEditor { + constructor(params) { + super(params); + this.annotationElementId = params.annotationElementId; + this.deleted = true; + } + + serialize() { + return { + id: this.annotationElementId, + deleted: true, + pageIndex: this.pageIndex, + }; } } diff --git a/src/display/editor/freetext.js b/src/display/editor/freetext.js index 86b36845e..3e15e741a 100644 --- a/src/display/editor/freetext.js +++ b/src/display/editor/freetext.js @@ -48,6 +48,8 @@ class FreeTextEditor extends AnnotationEditor { #fontSize; + #initialData = null; + static _freeTextDefaultContent = ""; static _l10nPromise; @@ -285,6 +287,7 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ onceAdded() { if (this.width) { + this.#cheatInitialRect(); // The editor was created in using ctrl+c. return; } @@ -481,12 +484,17 @@ class FreeTextEditor extends AnnotationEditor { if (this.width) { // This editor was created in using copy (ctrl+c). const [parentWidth, parentHeight] = this.parentDimensions; - this.setAt( - baseX * parentWidth, - baseY * parentHeight, - this.width * parentWidth, - this.height * parentHeight - ); + if (this.annotationElementId) { + const [tx] = this.getInitialTranslation(); + this.setAt(baseX * parentWidth, baseY * parentHeight, tx, tx); + } else { + this.setAt( + baseX * parentWidth, + baseY * parentHeight, + this.width * parentWidth, + this.height * parentHeight + ); + } this.#setContent(); this.div.draggable = true; @@ -519,14 +527,37 @@ class FreeTextEditor extends AnnotationEditor { /** @inheritdoc */ static deserialize(data, parent, uiManager) { + let initialData = null; if (data instanceof FreeTextAnnotationElement) { - return null; + const { + data: { + defaultAppearanceData: { fontSize, fontColor }, + rect, + rotation, + id, + }, + textContent, + page: { pageNumber }, + } = data; + initialData = data = { + annotationType: AnnotationEditorType.FREETEXT, + color: Array.from(fontColor), + fontSize, + value: textContent.join("\n"), + pageIndex: pageNumber - 1, + rect, + rotation, + id, + deleted: false, + }; } const editor = super.deserialize(data, parent, uiManager); editor.#fontSize = data.fontSize; editor.#color = Util.makeHexColor(...data.color); editor.#content = data.value; + editor.annotationElementId = data.id || null; + editor.#initialData = initialData; return editor; } @@ -537,16 +568,23 @@ class FreeTextEditor extends AnnotationEditor { return null; } + if (this.deleted) { + return { + pageIndex: this.pageIndex, + id: this.annotationElementId, + deleted: true, + }; + } + const padding = FreeTextEditor._internalPadding * this.parentScale; const rect = this.getRect(padding, padding); - const color = AnnotationEditor._colorManager.convert( this.isAttachedToDOM ? getComputedStyle(this.editorDiv).color : this.#color ); - return { + const serialized = { annotationType: AnnotationEditorType.FREETEXT, color, fontSize: this.#fontSize, @@ -554,7 +592,45 @@ class FreeTextEditor extends AnnotationEditor { pageIndex: this.pageIndex, rect, rotation: this.rotation, + id: this.annotationElementId, }; + + if (this.annotationElementId && !this.#hasElementChanged(serialized)) { + return null; + } + + return serialized; + } + + #hasElementChanged(serialized) { + const { value, fontSize, color, rect, pageIndex } = this.#initialData; + + return ( + serialized.value !== value || + serialized.fontSize !== fontSize || + serialized.rect.some((x, i) => Math.abs(x - rect[i]) >= 1) || + serialized.color.some((c, i) => c !== color[i]) || + serialized.pageIndex !== pageIndex + ); + } + + #cheatInitialRect(delayed = false) { + // The annotation has a rect but the editor has an other one. + // When we want to know if the annotation has changed (e.g. has been moved) + // we must compare the editor initial rect with the current one. + // So this method is a hack to have a way to compare the real rects. + if (!this.annotationElementId) { + return; + } + + this.#setEditorDimensions(); + if (!delayed && (this.width === 0 || this.height === 0)) { + setTimeout(() => this.#cheatInitialRect(/* delayed = */ true), 0); + return; + } + + const padding = FreeTextEditor._internalPadding * this.parentScale; + this.#initialData.rect = this.getRect(padding, padding); } } diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index d50a37dc6..b9df8bb55 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -362,6 +362,8 @@ class AnnotationEditorUIManager { #currentPageIndex = 0; + #deletedAnnotationsElementIds = new Set(); + #editorTypes = null; #editorsToRescale = new Set(); @@ -554,7 +556,11 @@ class AnnotationEditorUIManager { const editors = []; for (const editor of this.#selectedEditors) { if (!editor.isEmpty()) { - editors.push(editor.serialize()); + const serialized = editor.serialize(); + // Remove the id from the serialized data because it mustn't be linked + // to an existing annotation. + delete serialized.id; + editors.push(serialized); } } if (editors.length === 0) { @@ -862,7 +868,39 @@ class AnnotationEditorUIManager { removeEditor(editor) { this.#allEditors.delete(editor.id); this.unselect(editor); - this.#annotationStorage?.remove(editor.id); + if ( + !editor.annotationElementId || + !this.#deletedAnnotationsElementIds.has(editor.annotationElementId) + ) { + this.#annotationStorage?.remove(editor.id); + } + } + + /** + * The annotation element with the given id has been deleted. + * @param {AnnotationEditor} editor + */ + addDeletedAnnotationElement(editor) { + this.#deletedAnnotationsElementIds.add(editor.annotationElementId); + editor.deleted = true; + } + + /** + * Check if the annotation element with the given id has been deleted. + * @param {string} annotationElementId + * @returns {boolean} + */ + isDeletedAnnotationElement(annotationElementId) { + return this.#deletedAnnotationsElementIds.has(annotationElementId); + } + + /** + * The annotation element with the given id have been restored. + * @param {AnnotationEditor} editor + */ + removeDeletedAnnotationElement(editor) { + this.#deletedAnnotationsElementIds.delete(editor.annotationElementId); + editor.deleted = false; } /** diff --git a/test/integration/freetext_editor_spec.js b/test/integration/freetext_editor_spec.js index 1227f58db..f4b0624f9 100644 --- a/test/integration/freetext_editor_spec.js +++ b/test/integration/freetext_editor_spec.js @@ -15,8 +15,10 @@ const { closePages, + getEditors, getEditorSelector, getSelectedEditors, + getSerialized, loadAndWait, waitForEvent, waitForSelectedEditor, @@ -39,7 +41,7 @@ const copyPaste = async page => { await promise; }; -describe("Editor", () => { +describe("FreeText Editor", () => { describe("FreeText", () => { let pages; @@ -837,4 +839,224 @@ describe("Editor", () => { ); }); }); + + describe("FreeText (move existing)", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("freetexts.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must move an annotation", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + if (browserName === "firefox") { + pending( + "Disabled in Firefox, because DnD isn't implemented yet (see bug 1838638)." + ); + } + + await page.setDragInterception(true); + await page.click("#editorFreeText"); + + const editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(6); + + // All the current annotations should be serialized as null objects + // because they haven't been edited yet. + let serialized = await getSerialized(page); + expect(serialized).withContext(`In ${browserName}`).toEqual([]); + + const editorRect = await page.$eval(getEditorSelector(0), el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { x, y, width, height }; + }); + + await page.mouse.dragAndDrop( + { + x: editorRect.x + editorRect.width / 2, + y: editorRect.y + editorRect.height / 2, + }, + { + x: editorRect.x + editorRect.width / 2 + 100, + y: editorRect.y + editorRect.height / 2 + 100, + }, + { delay: 100 } + ); + + serialized = await getSerialized(page); + expect(serialized.length).withContext(`In ${browserName}`).toEqual(1); + }) + ); + }); + }); + + describe("FreeText (update existing)", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("freetexts.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must update an existing annotation", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorFreeText"); + + let editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(6); + + const editorRect = await page.$eval(getEditorSelector(0), el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { x, y, width, height }; + }); + await page.mouse.click( + editorRect.x + editorRect.width / 2, + editorRect.y + editorRect.height / 2, + { clickCount: 2 } + ); + + await page.keyboard.down("Control"); + await page.keyboard.press("End"); + await page.keyboard.up("Control"); + await page.waitForTimeout(10); + + await page.type( + `${getEditorSelector(0)} .internal`, + " and edited in Firefox" + ); + + // Commit. + await page.mouse.click( + editorRect.x, + editorRect.y + 2 * editorRect.height + ); + + let serialized = await getSerialized(page); + expect(serialized.length).withContext(`In ${browserName}`).toEqual(1); + expect(serialized[0]).toEqual( + jasmine.objectContaining({ + color: [107, 217, 41], + fontSize: 14, + value: "Hello World from Acrobat and edited in Firefox", + id: "26R", + }) + ); + + // Disable editing mode. + await page.click("#editorFreeText"); + // We want to check that the editor is displayed but not the original + // annotation. + editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(1); + const hidden = await page.$eval( + "[data-annotation-id='26R']", + el => el.hidden + ); + expect(hidden).withContext(`In ${browserName}`).toBeTrue(); + + // Re-enable editing mode. + await page.click("#editorFreeText"); + await page.focus(".annotationEditorLayer"); + + // Undo. + await page.keyboard.down("Control"); + await page.keyboard.press("z"); + await page.keyboard.up("Control"); + await page.waitForTimeout(10); + + serialized = await getSerialized(page); + expect(serialized).withContext(`In ${browserName}`).toEqual([]); + + editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(6); + + // Undo again. + await page.keyboard.down("Control"); + await page.keyboard.press("z"); + await page.keyboard.up("Control"); + await page.waitForTimeout(10); + + // We check that the editor hasn't been removed. + editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(6); + }) + ); + }); + }); + + describe("FreeText (delete existing)", () => { + let pages; + + beforeAll(async () => { + pages = await loadAndWait("freetexts.pdf", ".annotationEditorLayer"); + }); + + afterAll(async () => { + await closePages(pages); + }); + + it("must delete an existing annotation", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await page.click("#editorFreeText"); + + let editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(6); + + const editorRect = await page.$eval(getEditorSelector(3), el => { + const { x, y, width, height } = el.getBoundingClientRect(); + return { x, y, width, height }; + }); + await page.mouse.click( + editorRect.x + editorRect.width / 2, + editorRect.y + editorRect.height / 2 + ); + + await page.keyboard.press("Backspace"); + await page.waitForTimeout(10); + + let serialized = await getSerialized(page); + expect(serialized).toEqual([ + { + pageIndex: 0, + id: "51R", + deleted: true, + }, + ]); + + await page.click("#editorFreeText"); + // We want to check that nothing is displayed. + editorIds = await getEditors(page, "freeText"); + expect(editorIds.length).withContext(`In ${browserName}`).toEqual(0); + const hidden = await page.$eval( + "[data-annotation-id='51R']", + el => el.hidden + ); + expect(hidden).withContext(`In ${browserName}`).toBeTrue(); + + // Re-enable editing mode. + await page.click("#editorFreeText"); + await page.focus(".annotationEditorLayer"); + + // Undo. + await page.keyboard.down("Control"); + await page.keyboard.press("z"); + await page.keyboard.up("Control"); + await page.waitForTimeout(10); + + serialized = await getSerialized(page); + expect(serialized).withContext(`In ${browserName}`).toEqual([]); + }) + ); + }); + }); }); diff --git a/test/integration/test_utils.js b/test/integration/test_utils.js index 0be72d8b0..69414a910 100644 --- a/test/integration/test_utils.js +++ b/test/integration/test_utils.js @@ -134,3 +134,21 @@ const mockClipboard = async pages => { ); }; exports.mockClipboard = mockClipboard; + +const getSerialized = page => + page.evaluate(() => [ + ...window.PDFViewerApplication.pdfDocument.annotationStorage.serializable.values(), + ]); +exports.getSerialized = getSerialized; + +function getEditors(page, kind) { + return page.evaluate(aKind => { + const elements = document.querySelectorAll(`.${aKind}Editor`); + const results = []; + for (const { id } of elements) { + results.push(id); + } + return results; + }, kind); +} +exports.getEditors = getEditors;