Merge pull request #20282 from calixteman/fix_showing_invisible_editor

[Editor] Make sure to not add extra editors when showing again a destroyed page

Author: timvandermeij

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;

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<AnnotationEditor>}
+   * @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;
   }
 
   /**

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
+          );
+        })
+      );
+    });
+  });
 });