From cbc5241b53f7e364d2bb9201f7762a57140563ea Mon Sep 17 00:00:00 2001 From: Calixte Denizet Date: Thu, 18 Sep 2025 15:18:50 +0200 Subject: [PATCH] [Editor] Make sure to not add extra editors when showing again a destroyed page --- src/display/editor/annotation_editor_layer.js | 10 +- src/display/editor/tools.js | 8 +- test/integration/highlight_editor_spec.mjs | 92 +++++++++++++++++++ 3 files changed, 103 insertions(+), 7 deletions(-) diff --git a/src/display/editor/annotation_editor_layer.js b/src/display/editor/annotation_editor_layer.js index fe01b9156..493a2f75d 100644 --- a/src/display/editor/annotation_editor_layer.js +++ b/src/display/editor/annotation_editor_layer.js @@ -238,6 +238,12 @@ class AnnotationEditorLayer { this.#annotationLayer?.div.classList.toggle("disabled", !enabled); } + get #allEditorsIterator() { + return this.#editors.size !== 0 + ? this.#editors.values() + : this.#uiManager.getEditors(this.pageIndex); + } + /** * Enable pointer events on the main div in order to enable * editor creation. @@ -249,7 +255,7 @@ class AnnotationEditorLayer { this.#textLayerDblClickAC?.abort(); this.#textLayerDblClickAC = null; const annotationElementIds = new Set(); - for (const editor of this.#editors.values()) { + for (const editor of this.#allEditorsIterator) { editor.enableEditing(); editor.show(true); if (editor.annotationElementId) { @@ -342,7 +348,7 @@ class AnnotationEditorLayer { } const changedAnnotations = new Map(); const resetAnnotations = new Map(); - for (const editor of this.#editors.values()) { + for (const editor of this.#allEditorsIterator) { editor.disableEditing(); if (!editor.annotationElementId) { continue; diff --git a/src/display/editor/tools.js b/src/display/editor/tools.js index f9ac09946..7ff99eee7 100644 --- a/src/display/editor/tools.js +++ b/src/display/editor/tools.js @@ -2087,16 +2087,14 @@ class AnnotationEditorUIManager { /** * Get all the editors belonging to a given page. * @param {number} pageIndex - * @returns {Array} + * @yields {AnnotationEditor} */ - getEditors(pageIndex) { - const editors = []; + *getEditors(pageIndex) { for (const editor of this.#allEditors.values()) { if (editor.pageIndex === pageIndex) { - editors.push(editor); + yield editor; } } - return editors; } /** diff --git a/test/integration/highlight_editor_spec.mjs b/test/integration/highlight_editor_spec.mjs index 0e2a930b0..2b43bc58b 100644 --- a/test/integration/highlight_editor_spec.mjs +++ b/test/integration/highlight_editor_spec.mjs @@ -2861,4 +2861,96 @@ describe("Highlight Editor", () => { ); }); }); + + describe("Highlight (edit existing and scroll)", () => { + let pages; + + beforeEach(async () => { + pages = await loadAndWait( + "highlights.pdf", + ".annotationEditorLayer", + null, + null, + { + highlightEditorColors: + "yellow=#FFFF00,green=#00FF00,blue=#0000FF,pink=#FF00FF,red=#FF0102", + } + ); + }); + + afterEach(async () => { + await closePages(pages); + }); + + it("must check that no extra annotations are added while in editing mode", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + const editorSelector = getEditorSelector(7); + await page.waitForSelector(editorSelector); + + const oneToOne = Array.from(new Array(13).keys(), n => n + 2).concat( + Array.from(new Array(13).keys(), n => 13 - n) + ); + for (const pageNumber of oneToOne) { + await scrollIntoView( + page, + `.page[data-page-number = "${pageNumber}"]` + ); + } + + await page.waitForSelector(editorSelector); + + const count = await page.evaluate( + () => + document.querySelectorAll( + `.page[data-page-number = "1"] .annotationEditorLayer .highlightEditor` + ).length + ); + expect(count).withContext(`In ${browserName}`).toEqual(8); + }) + ); + }); + + it("must check that no extra annotations are added while in reading mode", async () => { + await Promise.all( + pages.map(async ([browserName, page]) => { + await switchToHighlight(page); + + const editorSelector = getEditorSelector(7); + await page.waitForSelector(editorSelector); + + const oneToThirteen = Array.from(new Array(13).keys(), n => n + 2); + const thirteenToOne = Array.from(new Array(13).keys(), n => 13 - n); + for (const pageNumber of oneToThirteen) { + await scrollIntoView( + page, + `.page[data-page-number = "${pageNumber}"]` + ); + } + + await switchToHighlight(page, /* disable */ true); + + for (const pageNumber of thirteenToOne) { + await scrollIntoView( + page, + `.page[data-page-number = "${pageNumber}"]` + ); + } + + await page.waitForSelector( + `.page[data-page-number = "1"] .annotationEditorLayer.disabled` + ); + + await page.waitForFunction( + () => + document.querySelectorAll( + `.page[data-page-number = "1"] .annotationEditorLayer .highlightEditor` + ).length === 0 + ); + }) + ); + }); + }); });